On 01/26/2012 04:23 AM, Kostik Belousov wrote:
On Wed, Jan 25, 2012 at 03:48:04PM -0800, Dmitry Mikulin wrote:
Thanks for taking a look at it. I'll try to explain the changes the best I
can: the work was done nearly 6 months ago...
I would certainly appreciate some more words describing the changes.
What is the goal of introducing the PT_FOLLOW_EXEC ? To not force
the debugger to filter all syscall exits to see exec events ?
PT_FOLLOW_EXEC was added to trace the exec() family of calls. When it's
enabled, the kernel will generate a trap to give debugger a chance to clean
up old process info and initialize its state with the new binary.
It was more a question, why PT_FLAG_EXEC is not enough.
I don't see it in the code (sp?) If you mean PL_FLAG_FORKED, than it's used to
return lwpinfo, and PT_FOLLOW_EXEC is a ptrace request.
Why did you moved the stopevent/ptracestop for exec events from
syscallret() to kern_execve() ? If moving, why the handling of TDB_EXEC
is not removed from syscallret() ? I do not think that TDB_EXEC can be
seen there after the patch. The same question about TDB_FORK.
The reason I moved the event notifications from syscallret() is because the
debugger is interested successful completion of either fork or exec, not
just the fact that they have been called. If, say, a call to fork() failed,
as far as debugger is concerned, fork() might as well had never been
called. Having a ptracestop in syscallret triggered a trap on every return
from fork without telling the debugger whether a new process had been
created or not. Same goes for exec().
PT_FLAG_EXEC is only set if an exec-kind of syscall succeeded. The same
is true for PT_FLAG_FORKED, the flag is set only if a new child was
successfully created.
I found the test cases you sent me for your fork/exec tracing changes and now I
see why I needed to make some of the changes. Before my patches tracing of
fork/exec is done via PT_TO_SCE/PT_TO_SCX ptrace calls. Their semantics did not
fit well into what gdb needs to do. They operate as a variant of PT_CONTINUE.
What gdb needs is a process-wide setting that instructs the kernel to generate
SIGTRAPs in fork/exec when the process is being controlled via the regular
ptrace(PT_CONTINUE)/wait() sequence.
Looking at the code now I think I should have removed TDB_EXEC/TDB_FORK
processing from syscallret(). I'll do that and verify that it works as
expected.
If possible, I would greatly prefer to have fork changes separated.
You mean separate fork changes into a separate patch from exec changes?
Yes. Even more, it seems that fork changes should be separated into
bug fixes and new functionality.
OK, that's doable.
I doubt that disallowing RFMEM while tracing is the right change. It may
be to fix some (undescribed) bug, but RFMEM is documented behaviour used
not only for vfork(2), and the change just breaks rfork(2) for debugged
processes.
Even vfork() should not be broken, since I believe there are programs
that rely on the vfork() model, in particular, C shell. It will be
broken if vfork() is substituted by fork(). The fact that it breaks only
under debugger will make it esp. confusing.
I need to dig up the details since I can't recall the exact reason for
forcing fork() in cases of user calls to vfork() under gdb. I believe it
had to do with when child process memory was available for writing in case
of vfork() and it wasn't early enough to complete the switch over from
parent to child in gdb. After consulting with our internal kernel experts
we decided that doing fork() instead of vfork() under gdb is a low impact
change.
Why do we need to have TDB_FORK set for td2 ?
The debugger needs to intercept fork() in both parent and child so it can
detach from the old process and attach to the new one. Maybe it'll make
more sense in the context of gdb changes. Should I send them too? Don't
think Marcel included that patch...
No, I think the kernel changes are enough for now.
Does the orphan list change intended to not lost the child after fork ?
But the child shall be traced, so debugger would get the SIGTRAP on
the attach on fork returning to usermode. I remember that I explicitely
tested this when adding followfork changes.
Yes, the debugger gets SIGTRAPs. The problem arises when the real parent of
the forked process has the code to collect termination status. Since
attaching to a process changes the parent/child relationships, we need to
keep track of the children lost due to re-parenting so we can properly
attribute their exit status to the "real" parent.
I do not quite understand it. From the description it sounds as the problem
that is not related to follow fork changes at all, and shall be presented
regardless of the follow fork. If yes, I think this shall be separated
into a standalone patch.
You're right, it's not related. It just happened to prevent me from finishing
the work.
I'll break up the changes into multiple patches and re-send.
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
