On Fri, Feb 03, 2012 at 04:01:46PM -0800, Dmitry Mikulin wrote: > > >Please provide more details, I am looking forward for the panic > >message and backtrace. > > I can't seem to get the panic with the latest source base, but tracing > doesn't appear to work with vfork(). I attached a modified test case to > closer model what gdb is doing. If you change fork() to vfork() in simple.c > the parent doesn't get the events the same way it does under fork(). I see what is going on. The wait loop for P_PPWAIT in do_fork() simply do not allow the ptracestop() in the syscall return path to be reached. There seems to be more problems. In particular, I do not see anything which would prevent the child from being reapped while the loop is executing (assume that the parent is multithreaded and other thread processed SIGCHLD and called wait).
Lets deal with these bugs after your proposal for interface changes is
dealt with.
>
> >The lack of the notification for parent is exactly what the patch I
> >posted fixes. More exactly, it is the lack of notification for parent
> >with PT_CONTINUE loop. I will commit this today.
> >
> >Regarding a single notification. Currently, parent arrives at the
> >syscall return code only after the child is attached to the debugger.
> >See the cv_wait() in kern_fork.c:739. In other words, if you get the
> >PL_FLAG_FORK, the child is already attached (at last it shall be). My
> >scescx.c code illustrates this well, IMO.
> >
> >You still get a separate stop from the child, but I do not see how is it
> >harmful.
>
> There a couple of tweaks to the interface that I'd like to have:
> - set PL_FLAG_FORKED in the child so gdb can identify the stop as a fork()
> event.
> - add a switch similar to PT_FOLLOW_FORK to enable/disable exec()
> notifications. Gdb gets confused by the events it hasn't explicitly asked
> for and having a switch makes it easier to filter out unwanted stops.
>
> Do you think it's possible/makes sense?
Yes, I agree with the proposal to add flag to the child lwp info.
I think it will be easier if the flag is different from PL_FLAG_FORKED.
I named it PL_FLAG_CHILD.
PT_FOLLOW_EXEC is easy to implement, but my question is, how can debugger
operate (correctly) if it ignores exec events ? After exec, the whole
cached state of the debuggee must be invalidated, and since debugger
ignores the notification when the invalidation shall be done, it probably
gets very confused.
Anyway, below is the combined patch that adds PL_FLAG_CHILD and
PT_FOLLOW_EXEC. If you agree with this, I will commit them (separately).
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 135f798..c756777 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -889,7 +889,8 @@ exec_fail_dealloc:
if (error == 0) {
PROC_LOCK(p);
- td->td_dbgflags |= TDB_EXEC;
+ if ((p->p_flag & P_NOFOLLOWEXEC) == 0)
+ td->td_dbgflags |= TDB_EXEC;
PROC_UNLOCK(p);
/*
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 60639c9..e447c93 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -1035,7 +1035,9 @@ fork_return(struct thread *td, struct trapframe *frame)
p->p_oppid = p->p_pptr->p_pid;
proc_reparent(p, dbg);
sx_xunlock(&proctree_lock);
+ td->td_dbgflags |= TDB_CHILD;
ptracestop(td, SIGSTOP);
+ td->td_dbgflags &= ~TDB_CHILD;
} else {
/*
* ... otherwise clear the request.
diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
index 4510380..f58039a 100644
--- a/sys/kern/sys_process.c
+++ b/sys/kern/sys_process.c
@@ -660,6 +660,7 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void
*addr, int data)
case PT_TO_SCX:
case PT_SYSCALL:
case PT_FOLLOW_FORK:
+ case PT_FOLLOW_EXEC:
case PT_DETACH:
sx_xlock(&proctree_lock);
proctree_locked = 1;
@@ -873,6 +874,12 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void
*addr, int data)
else
p->p_flag &= ~P_FOLLOWFORK;
break;
+ case PT_FOLLOW_EXEC:
+ if (data)
+ p->p_flag &= ~P_NOFOLLOWEXEC;
+ else
+ p->p_flag |= P_NOFOLLOWEXEC;
+ break;
case PT_STEP:
case PT_CONTINUE:
@@ -936,7 +943,8 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void
*addr, int data)
p->p_sigparent = SIGCHLD;
}
p->p_oppid = 0;
- p->p_flag &= ~(P_TRACED | P_WAITED | P_FOLLOWFORK);
+ p->p_flag &= ~(P_TRACED | P_WAITED | P_FOLLOWFORK |
+ P_NOFOLLOWEXEC);
/* should we send SIGCHLD? */
/* childproc_continued(p); */
@@ -1145,6 +1153,8 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void
*addr, int data)
pl->pl_flags |= PL_FLAG_FORKED;
pl->pl_child_pid = td2->td_dbg_forked;
}
+ if (td2->td_dbgflags & TDB_CHILD)
+ pl->pl_flags |= PL_FLAG_CHILD;
pl->pl_sigmask = td2->td_sigmask;
pl->pl_siglist = td2->td_siglist;
strcpy(pl->pl_tdname, td2->td_name);
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 76f3355..91bc86e 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -384,6 +384,7 @@ do {
\
process */
#define TDB_STOPATFORK 0x00000080 /* Stop at the return from fork
(child
only) */
+#define TDB_CHILD 0x00000100 /* New child indicator for ptrace()
*/
/*
* "Private" flags kept in td_pflags:
@@ -613,6 +614,7 @@ struct proc {
#define P_HWPMC 0x800000 /* Process is using HWPMCs */
#define P_JAILED 0x1000000 /* Process is in jail. */
+#define P_NOFOLLOWEXEC 0x2000000 /* Do not report execs with ptrace. */
#define P_INEXEC 0x4000000 /* Process is in execve(). */
#define P_STATCHILD 0x8000000 /* Child process stopped or exited. */
#define P_INMEM 0x10000000 /* Loaded into memory. */
diff --git a/sys/sys/ptrace.h b/sys/sys/ptrace.h
index 2583d59..05c758c 100644
--- a/sys/sys/ptrace.h
+++ b/sys/sys/ptrace.h
@@ -64,6 +64,7 @@
#define PT_SYSCALL 22
#define PT_FOLLOW_FORK 23
+#define PT_FOLLOW_EXEC 24
#define PT_GETREGS 33 /* get general-purpose registers */
#define PT_SETREGS 34 /* set general-purpose registers */
@@ -106,7 +107,8 @@ struct ptrace_lwpinfo {
#define PL_FLAG_SCX 0x08 /* syscall leave point */
#define PL_FLAG_EXEC 0x10 /* exec(2) succeeded */
#define PL_FLAG_SI 0x20 /* siginfo is valid */
-#define PL_FLAG_FORKED 0x40 /* new child */
+#define PL_FLAG_FORKED 0x40 /* child born */
+#define PL_FLAG_CHILD 0x80 /* I am from child */
sigset_t pl_sigmask; /* LWP signal mask */
sigset_t pl_siglist; /* LWP pending signal */
struct __siginfo pl_siginfo; /* siginfo for signal */
pgp861I3IL67O.pgp
Description: PGP signature
