Date: Tue, 7 Feb 2012 14:10:22 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Dmitry Mikulin <dmitrym@juniper.net> Cc: freebsd-current Current <freebsd-current@freebsd.org>, Marcel Moolenaar <marcelm@juniper.net> Subject: Re: [ptrace] please review follow fork/exec changes Message-ID: <20120207121022.GC3283@deviant.kiev.zoral.com.ua> In-Reply-To: <4F3043E2.6090607@juniper.net> References: <20120125074824.GD2726@deviant.kiev.zoral.com.ua> <4F2094B4.70707@juniper.net> <20120126122326.GT2726@deviant.kiev.zoral.com.ua> <4F22E8FD.6010201@juniper.net> <20120129074843.GL2726@deviant.kiev.zoral.com.ua> <4F26E0D1.8040100@juniper.net> <20120130192727.GZ2726@deviant.kiev.zoral.com.ua> <4F2C756A.80900@juniper.net> <20120204204218.GC3283@deviant.kiev.zoral.com.ua> <4F3043E2.6090607@juniper.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--lmWPTIvm4M7zUP/T
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Mon, Feb 06, 2012 at 01:19:30PM -0800, Dmitry Mikulin wrote:
>=20
> >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.
>=20
> OK.
>=20
> >
> >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.
>=20
> You're right, the debugger needs to handle exec() events implicitly when =
it=20
> starts up executables. The problem is that there is OS-independent=20
> machinery in gdb which handles statup fork-exec sequence differently from=
=20
> when the debuggee itself does an exec(). Basically in the event handling=
=20
> code I need to be able to distinguish app startup by gdb from an exec don=
e=20
> by the app. Other OS-es have flags like PL_FLAG_EXEC set on demand: they=
=20
> have an equivalent of PT_FOLLOW_EXEC. I attached a modified patch that=20
> solves the problem. It tries to separate the always-on TDB_EXEC from the=
=20
> on-demand TDB_FOLLOWEXEC without changing existing functionality. Let me=
=20
> know if it's acceptable.
So, do you in fact need to distinguish exec stops from syscall exit
against exec stops from PT_FOLLOW_EXEC, or do you need to only get stops
at exec returns from PT_CONTINUE when explicitely requested them ?
I would prefer to not introduce another PL_FLAG_<something>EXEC with the
same semantic as PL_FLAG_EXEC. Instead, would the following patch be fine
for your purposes ? With it, stop on exec should only occur if PT_SCX
is requested, or PT_CONTINUE and PT_FOLLOW_EXEC.
[I am unable to fully test this until tomorrow].
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 135f798..67cb1b2 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -56,6 +56,7 @@ __FBSDID("$FreeBSD$");
#include <sys/proc.h>
#include <sys/pioctl.h>
#include <sys/namei.h>
+#include <sys/ptrace.h>
#include <sys/resourcevar.h>
#include <sys/sdt.h>
#include <sys/sf_buf.h>
@@ -889,7 +890,9 @@ exec_fail_dealloc:
=20
if (error =3D=3D 0) {
PROC_LOCK(p);
- td->td_dbgflags |=3D TDB_EXEC;
+ if ((p->p_flag & P_TRACED) !=3D 0 &&
+ ((P_FOLLOWEXEC) !=3D 0 || (p->p_stops & S_PT_SCX) !=3D 0))
+ td->td_dbgflags |=3D TDB_EXEC;
PROC_UNLOCK(p);
=20
/*
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 *fram=
e)
p->p_oppid =3D p->p_pptr->p_pid;
proc_reparent(p, dbg);
sx_xunlock(&proctree_lock);
+ td->td_dbgflags |=3D TDB_CHILD;
ptracestop(td, SIGSTOP);
+ td->td_dbgflags &=3D ~TDB_CHILD;
} else {
/*
* ... otherwise clear the request.
diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
index 4510380..79bbaed 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 =3D 1;
@@ -873,6 +874,12 @@ kern_ptrace(struct thread *td, int req, pid_t pid, voi=
d *addr, int data)
else
p->p_flag &=3D ~P_FOLLOWFORK;
break;
+ case PT_FOLLOW_EXEC:
+ if (data)
+ p->p_flag |=3D P_FOLLOWEXEC;
+ else
+ p->p_flag &=3D ~P_FOLLOWEXEC;
+ break;
=20
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 =3D SIGCHLD;
}
p->p_oppid =3D 0;
- p->p_flag &=3D ~(P_TRACED | P_WAITED | P_FOLLOWFORK);
+ p->p_flag &=3D ~(P_TRACED | P_WAITED | P_FOLLOWFORK |
+ P_FOLLOWEXEC);
=20
/* should we send SIGCHLD? */
/* childproc_continued(p); */
@@ -1145,6 +1153,8 @@ kern_ptrace(struct thread *td, int req, pid_t pid, vo=
id *addr, int data)
pl->pl_flags |=3D PL_FLAG_FORKED;
pl->pl_child_pid =3D td2->td_dbg_forked;
}
+ if (td2->td_dbgflags & TDB_CHILD)
+ pl->pl_flags |=3D PL_FLAG_CHILD;
pl->pl_sigmask =3D td2->td_sigmask;
pl->pl_siglist =3D td2->td_siglist;
strcpy(pl->pl_tdname, td2->td_name);
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 9ebfe83..bec7223 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() */
=20
/*
* "Private" flags kept in td_pflags:
@@ -613,6 +614,7 @@ struct proc {
#define P_HWPMC 0x800000 /* Process is using HWPMCs */
=20
#define P_JAILED 0x1000000 /* Process is in jail. */
+#define P_FOLLOWEXEC 0x2000000 /* 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
=20
#define PT_FOLLOW_FORK 23
+#define PT_FOLLOW_EXEC 24
=20
#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 */
--lmWPTIvm4M7zUP/T
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)
iEYEARECAAYFAk8xFK4ACgkQC3+MBN1Mb4gpWwCbB1kzJxlazKuClYS12oyoeggT
gz8AoKZMLnE/W9E/eIzEzGJK7kP7I9fu
=EncL
-----END PGP SIGNATURE-----
--lmWPTIvm4M7zUP/T--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120207121022.GC3283>