Re: [ptrace] please review follow fork/exec changes

On 01/28/2012 11:48 PM, Kostik Belousov wrote:
> On Fri, Jan 27, 2012 at 10:12:13AM -0800, Dmitry Mikulin wrote:
>> Attached are 4 separate patches for each somewhat independent  portion of
>> the kernel work related to the follow-fork implementation.
> Ok, as I said, I think that vfork-fork.patch is just wrong.

Gdb needs to be able to read/write process memory between the time the child is forked and exec is called (in the vfork case). Without the change it causes a kernel panic when gdb tries to read/write process memory. Since my understanding of the kernel is a bit limited, it was the best I could do at the time. I will send more details about the panic once I get a working fbsd system again. Maybe there's a better way to deal with the panic.

> Lets postpone discussion of the orphan.patch for later.

OK.

>
> The follow-fork.patch and follow-exec.patch make me wonder, and I
> already expressed my doubts. IMO, all features, except one bug, are
> already presented in the stock src.
>
> More, suggested follow-{fork,exec} patches break the SCE/SCX tracers
> notification of fork and exec events, since TDB_FORK and TBD_EXEC flags
> are consumed before syscall returns (I also said this previously).




>
> Namely, if the process is being debugged, the successfull [f]execve()
> causes unconditional stop even. This makes PT_FOLLOW_EXEC unneccessary.
>
> Existing PT_FOLLOW_FORK implementation indeed has a bug, which was not
> revealed by my testing during the development, because I only tested
> SCE/SCX scenario. Namely, if PT_FOLLOW_FORK is requested, but the next
> stop is not SCX, then follow-fork notification is not sent. After this
> nit is fixed, PT_FOLLOW_FORK caller gets stops for the child creation.
> Child is put into stop state as needed to not loose it.

I think this will fix only a part of the problem, the one that relates to PT_CONTINUE.

I still need the change that forces a stop in both child and parent on fork(). Without my changes the notification is generated in the child but not in the parent. I need to be able to have both processes stopped in gdb in order to clean up and detach from the parent, and initialize and attach to the child. The main reason I need both processes stopped is that gdb has to be able to read/write into both processes address space and registers.  Ideally I would like to have a single event generated for fork() at a point where both child and parent are stopped and available for ptrace read/write requests.  Do you think it's possible?

>
> I updated the test program I use to test this functionality, see
> http://people.freebsd.org/~kib/misc/scescx.c
> The default or -s flag causes it to use SCE/SCX tracing, while -c flag
> switches it to use PT_CONTINUE tracing, which should be the kind of loop
> performed by normal debuggers. You can see the exec/fork events and
> child auto-attach illustrated with this test.
>
> Can you, please, provide the test-case which illustrates the omissions
> in the existing API (with the patch below applied), if any ?
>
> diff --git a/sys/kern/subr_syscall.c b/sys/kern/subr_syscall.c
> index bba4479..75328f6 100644
> --- a/sys/kern/subr_syscall.c
> +++ b/sys/kern/subr_syscall.c
> _at__at_ -212,7 +212,8 _at__at_ syscallret(struct thread *td, int error, struct syscall_args *sa __unused)
>   		 * executes.  If debugger requested tracing of syscall
>   		 * returns, do it now too.
>   		 */
> -		if (traced&&  ((td->td_dbgflags&  TDB_EXEC) != 0 ||
> +		if (traced&&
> +		    ((td->td_dbgflags&  (TDB_FORK | TDB_EXEC)) != 0 ||
>   		    (p->p_stops&  S_PT_SCX) != 0))
>   			ptracestop(td, SIGTRAP);
>   		td->td_dbgflags&= ~(TDB_SCX | TDB_EXEC | TDB_FORK);