Export current system call code and argument count for SCE and SCX events.

[jhb]

Summary

Export current system call code and argument count for system call entry
and exit events. procfs stop events for system call tracing report these
values (argument count for system call entry and code for system call exit),
but ptrace() does not provide this information. (Note that while the system
call code can be determined in an ABI-specific manner during system call
entry, it is not generally available during system call exit.)

The values are exported via new fields at the end of struct ptrace_lwpinfo
exported via PT_LWPINFO.

Test Plan

• I have updated truss to use these values, in particular the argument count on entry restores behavior lost when truss was converted from procfs to ptrace.

[kib]

IMO you cannot keep the pointer to the syscall_args in struct thread. I would add td_syscall_code and _narg to the struct thread, for direct use.

That said, at least syscall_code is available in registers. although in the MD way.

sys/kern/subr_syscall.c

71

You cannot do it this way. struct syscall_args is typically allocated on the thread stack, which means that it could be swapped out. I do not remember any reason why stopped thread cannot be swapped.

166

Why not keeping the (analog of) td_dbgsa for the whole syscall processing duration ? Setting pl_syscall_* should be directed by TDB_SCE/TDB_SCX, then you do not need to temporary NULLify the data.

sys/kern/sys_process.c

1218

As I said above, I would change the condition to td2->td_dbgflags & (TDB_SCE|TDB_SCX) != 0

[jhb]

As noted in the commit log, while you can obtain the system call code for system call entry events, you cannot obtain it for system call exit (and newborn processes and threads return from a system call that a tracer would not have a matching system call entry event for. I suspect this is why procfs returned the code for SCX stops.)

[jhb]

• Store the system call code and narg directly in struct thread.

[jhb]

sys/kern/subr_syscall.c

71

So one slightly annoying thing here is that sa->code and sa->narg aren't valid yet when we set TDB_SCE. For now I've deferred setting the td_dbg_sc_* fields until we call ptracestop(). If we want these fields to always be valid when TDB_SCE is set, then we either need to defer setting TDB_SCE or move this section after the call to sv_fetch_syscall_args so that sa->code and sa->narg are valid. I would probably prefer the latter, but I'm not sure of the implications of deferring the set of TDB_SCE until after the call to fetch_syscall_args, especially in regards to TDB_USERWR.

[kib]

BTW, why not take a step further and not provide the syscall arguments as well ?

sys/kern/subr_syscall.c

71

But TDB_ flags are not really usable until the debugee is at the stop point. That is, I do not see much difference between having TDB_SCE set where it is now, and moving it to the ptracestop() location. Except possibly one less PROC_LOCK().

sys/sys/ptrace.h

117

Make this unsigned as well ?

[jhb]

In D3536#72717, @kib wrote:

BTW, why not take a step further and not provide the syscall arguments as well ?

When I had asked you about that previously via e-mail I found your argument convincing that it would be putting too much into the kernel. It would mean embedding syscall_args directly in struct thread so that the full register array is present as well.

sys/kern/subr_syscall.c

71

After talking more on IRC, we realized that moving TDB_SCE down would be a bug because it would not properly report PL_FLAG_SCE if a stop inside of a system call triggered but the debugger had not enabled system call stops, so I will leave TDB_SCE where it is.

sys/kern/sys_process.c

99–101

Unrelated: should this be pid_t instead of int? Right now that would be a no-op since pid_t is int32_t?

sys/sys/ptrace.h

117

Done. Note that it is signed in 'struct syscall_args' so I was just copying from there. However, unsigned is probably better.

[jhb]

• Leave TDB_SCE where it is.
• Make narg unsigned.

[kib]

What about the fix for TDB_USERWR bug ? Is it planned for other change ?

sys/kern/subr_syscall.c

173

Is this now stray change ?

sys/kern/sys_process.c

99–102

int is explicit type, this is probably a reason why I used it there (and in ptrace_lwpinfo).
I do not object against changing it.

[jhb]

In D3536#72841, @kib wrote:

What about the fix for TDB_USERWR bug ? Is it planned for other change ?

Yes, I will handle TDB_USERWR separately. I ran into that trying to debug why I'm getting a seemingly spurious SIGHUP testing fork following in gdb (per bug 201149) and noticed the odd output in the kdump.

sys/kern/subr_syscall.c

173

Yes, though it is actually used, so it is more correct. I can do it separately.

[jhb]

• Cleanup.

[kib]

sys/kern/subr_syscall.c

173

I am fine with either. It is more logical to make a separate commit.