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

On Thu, Feb 09, 2012 at 12:48:26PM -0800, Dmitry Mikulin wrote:
> 
> >The semantic of PL_FLAG_EXEC up until now is very simple: it indicates
> >that current stop occured during the first return to usermode after
> >successful exec. The proposed patch breaks the semantic, because now
> >some stops which satisfy the stated condition are no longer marked with
> >the flag.
> >
> >That said, I am lost. You stated that you still need some stops at
> >exec even when not PT_FOLLOW_EXEC is requested. Why usermode cannot
> >remember whether the PT_FOLLOW_EXEC was set for the process, and ignore
> >PL_FLAG_EXEC if not requested ?
> 
> I was trying to avoid making ugly changes in gdb if it was possible not to 
> make ugly changes in the kernel. I changed gdb to work without 
> PT_FOLLOW_EXEC.
So, does the patch below helps you, or did I missed something again ?

> 
> >I just gave up and added PL_FLAG_EXECF, which is set when PT_FOLLOW_EXEC
> >was set and exec is active. Would this work for your purposes ?
> >PL_FLAG_EXECF has the same semantic as PL_FLAG_EXEC had in your
> >follow-exec.patch. But the stop set is not changed comparing with the
> >stock src.
> >
> >Are you fine with PL_FLAG_CHILD part of the changes ? If yes, I will
> >commit it to make some progress.
> 
> yes, the PL_FLAG_CHILD part works for me.
> Please commit it and we can move on to the next part of the review.

Committed as r231320. Below is what left for PT_FOLLOWEXEC.

diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
index 2060efe..4f93a79 100644
--- a/sys/kern/sys_process.c
+++ b/sys/kern/sys_process.c
_at__at_ -660,6 +660,7 _at__at_ 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;
_at__at_ -873,6 +874,12 _at__at_ 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_FOLLOWEXEC;
+		else
+			p->p_flag &= ~P_FOLLOWEXEC;
+		break;
 
 	case PT_STEP:
 	case PT_CONTINUE:
_at__at_ -936,7 +943,8 _at__at_ 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_FOLLOWEXEC);
 
 			/* should we send SIGCHLD? */
 			/* childproc_continued(p); */
_at__at_ -1139,8 +1147,11 _at__at_ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
 			pl->pl_flags |= PL_FLAG_SCE;
 		else if (td2->td_dbgflags & TDB_SCX)
 			pl->pl_flags |= PL_FLAG_SCX;
-		if (td2->td_dbgflags & TDB_EXEC)
+		if (td2->td_dbgflags & TDB_EXEC) {
 			pl->pl_flags |= PL_FLAG_EXEC;
+			if (p->p_flag & P_FOLLOWEXEC)
+				pl->pl_flags |= PL_FLAG_EXECF;
+		}
 		if (td2->td_dbgflags & TDB_FORK) {
 			pl->pl_flags |= PL_FLAG_FORKED;
 			pl->pl_child_pid = td2->td_dbg_forked;
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 0245e88..bec7223 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
_at__at_ -614,6 +614,7 _at__at_ struct proc {
 #define	P_HWPMC		0x800000 /* Process is using HWPMCs */
 
 #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 8a02495..81cebfc 100644
--- a/sys/sys/ptrace.h
+++ b/sys/sys/ptrace.h
_at__at_ -64,6 +64,7 _at__at_
 #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 */
_at__at_ -100,14 +101,15 _at__at_ struct ptrace_lwpinfo {
 #define	PL_EVENT_NONE	0
 #define	PL_EVENT_SIGNAL	1
 	int	pl_flags;	/* LWP flags. */
-#define	PL_FLAG_SA	0x01	/* M:N thread */
-#define	PL_FLAG_BOUND	0x02	/* M:N bound thread */
-#define	PL_FLAG_SCE	0x04	/* syscall enter point */
-#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_CHILD	0x80	/* I am from child */
+#define	PL_FLAG_SA	0x0001	/* M:N thread */
+#define	PL_FLAG_BOUND	0x0002	/* M:N bound thread */
+#define	PL_FLAG_SCE	0x0004	/* syscall enter point */
+#define	PL_FLAG_SCX	0x0008	/* syscall leave point */
+#define	PL_FLAG_EXEC	0x0010	/* exec(2) succeeded */
+#define	PL_FLAG_SI	0x0020	/* siginfo is valid */
+#define	PL_FLAG_FORKED	0x0040	/* child born */
+#define	PL_FLAG_CHILD	0x0080	/* I am from child */
+#define	PL_FLAG_EXECF	0x0100	/* exec and PT_FOLLOW_EXEC was set */
 	sigset_t	pl_sigmask;	/* LWP signal mask */
 	sigset_t	pl_siglist;	/* LWP pending signal */
 	struct __siginfo pl_siginfo;	/* siginfo for signal */