From: Denys Vlasenko <dvlasenk@redhat.com>
Date: Sun, 21 Aug 2011 16:03:23 +0000 (+0200)
Subject: Straighten up confused comments/messages about post-execve SIGTRAP handling
X-Git-Tag: v4.7~306
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b88f96129f8ebc064d490c8c161bcbc925d938c8;p=strace

Straighten up confused comments/messages about post-execve SIGTRAP handling

* defs.h: Explain TCB_INSYSCALL and TCB_WAITEXECVE bits in detail.
* strace.c (choose_pfd): Use entering/exiting macros instead of direct check
for TCB_INSYSCALL.
* syscall.c (get_scno): Use entering/exiting macros instead of direct check
for TCB_INSYSCALL. Fix comments about post-execve SIGTRAP.
(syscall_fixup): Use entering/exiting instead of direct check
for TCB_INSYSCALL. Add a comment what "not a syscall entry" message
usually means. Change wrong "stray syscall exit" messages into
"not a syscall entry" ones.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
---

diff --git a/defs.h b/defs.h
index c4ea5fab..97dbf6aa 100644
--- a/defs.h
+++ b/defs.h
@@ -432,7 +432,25 @@ struct tcb {
 /* TCB flags */
 #define TCB_STARTUP	00001	/* We have just begun ptracing this process */
 #define TCB_INUSE	00002	/* This table entry is in use */
-#define TCB_INSYSCALL	00004	/* A system call is in progress */
+/*
+ * Are we in system call entry or in syscall exit?
+ *
+ * This bit is set after all syscall entry processing is done.
+ * Therefore, this bit will be set when next ptrace stop occurs,
+ * which should be syscall exit stop. Other stops which are possible
+ * directly after syscall entry (death, ptrace event stop)
+ * are simpler and handled without calling trace_syscall(), therefore
+ * the places where TCB_INSYSCALL can be set but we aren't in syscall stop
+ * are limited to trace(), this condition is never observed in trace_syscall()
+ * and below.
+ * The bit is cleared after all syscall exit processing is done.
+ * User-generated SIGTRAPs and post-execve SIGTRAP make it necessary
+ * to be very careful and NOT set TCB_INSYSCALL bit when they are encountered.
+ * TCB_WAITEXECVE bit is used for this purpose (see below).
+ *
+ * Use entering(tcp) / exiting(tcp) to check this bit to make code more readable.
+ */
+#define TCB_INSYSCALL	00004
 #define TCB_ATTACHED	00010	/* Process is not our own child */
 #ifdef LINUX
 #define TCB_ATTACH_DONE	00020	/* PTRACE_ATTACH was done on this tcb->pid */
@@ -443,14 +461,17 @@ struct tcb {
 #define TCB_FILTERED	02000	/* This system call has been filtered out */
 #ifdef LINUX
 /* x86 does not need TCB_WAITEXECVE.
- * It can detect execve's SIGTRAP by looking at eax/rax.
- * See "stray syscall exit: eax = " message in syscall_fixup().
+ * It can detect SIGTRAP by looking at eax/rax.
+ * See "not a syscall entry (eax = %ld)\n" message in syscall_fixup().
  */
 # if defined(ALPHA) || defined(AVR32) || defined(SPARC) || defined(SPARC64) \
   || defined(POWERPC) || defined(IA64) || defined(HPPA) \
   || defined(SH) || defined(SH64) || defined(S390) || defined(S390X) \
   || defined(ARM) || defined(MIPS) || defined(BFIN) || defined(TILE)
-#  define TCB_WAITEXECVE 04000	/* ignore SIGTRAP after execve */
+/* This tracee has entered into execve syscall. Expect post-execve SIGTRAP
+ * to happen. (When it is detected, tracee is continued and this bit is cleared.)
+ */
+#  define TCB_WAITEXECVE 04000
 # endif
 # include <sys/syscall.h>
 # ifndef __NR_exit_group
diff --git a/strace.c b/strace.c
index c018abf3..04af6b56 100644
--- a/strace.c
+++ b/strace.c
@@ -2012,7 +2012,7 @@ choose_pfd()
 		 * heuristic improves the readability of the trace.
 		 */
 		tcp = pfd2tcb(pollv[last].fd);
-		if (tcp && (tcp->flags & TCB_INSYSCALL))
+		if (tcp && exiting(tcp))
 			return pollv[last].fd;
 	}
 
diff --git a/syscall.c b/syscall.c
index 200d8d64..a2338049 100644
--- a/syscall.c
+++ b/syscall.c
@@ -743,6 +743,12 @@ static long r3;
 struct reg regs; /* TODO: make static? */
 #endif /* FREEBSD */
 
+/* Returns:
+ * 0: "ignore this ptrace stop", bail out of trace_syscall() silently.
+ * 1: ok, continue in trace_syscall().
+ * other: error, trace_syscall() should print error indicator
+ *    ("????" etc) and bail out.
+ */
 int
 get_scno(struct tcb *tcp)
 {
@@ -760,10 +766,10 @@ get_scno(struct tcb *tcp)
 		 * leave the flag set so that syscall_fixup can fake the
 		 * result.
 		 */
-		if (tcp->flags & TCB_INSYSCALL)
+		if (exiting(tcp))
 			return 1;
 		/*
-		 * This is the SIGTRAP after execve.  We cannot try to read
+		 * This is the post-execve SIGTRAP.  We cannot try to read
 		 * the system call here either.
 		 */
 		tcp->flags &= ~TCB_WAITEXECVE;
@@ -855,8 +861,8 @@ get_scno(struct tcb *tcp)
 # elif defined (POWERPC)
 	if (upeek(tcp, sizeof(unsigned long)*PT_R0, &scno) < 0)
 		return -1;
-	if (!(tcp->flags & TCB_INSYSCALL)) {
-		/* Check if we return from execve. */
+	if (entering(tcp)) {
+		/* Check if this is the post-execve SIGTRAP. */
 		if (scno == 0 && (tcp->flags & TCB_WAITEXECVE)) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
@@ -864,7 +870,7 @@ get_scno(struct tcb *tcp)
 	}
 
 #  ifdef POWERPC64
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		/* TODO: speed up strace by not doing this at every syscall.
 		 * We only need to do it after execve.
 		 */
@@ -898,10 +904,10 @@ get_scno(struct tcb *tcp)
 	/*
 	 * We only need to grab the syscall number on syscall entry.
 	 */
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		scno = regs.r8;
 
-		/* Check if we return from execve. */
+		/* Check if this is the post-execve SIGTRAP. */
 		if (tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
@@ -917,7 +923,7 @@ get_scno(struct tcb *tcp)
 	if (upeek(tcp, 8*ORIG_RAX, &scno) < 0)
 		return -1;
 
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		/* TODO: speed up strace by not doing this at every syscall.
 		 * We only need to do it after execve.
 		 */
@@ -986,7 +992,7 @@ get_scno(struct tcb *tcp)
 #	define IA64_PSR_IS	((long)1 << 34)
 	if (upeek(tcp, PT_CR_IPSR, &psr) >= 0)
 		ia32 = (psr & IA64_PSR_IS) != 0;
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		if (ia32) {
 			if (upeek(tcp, PT_R1, &scno) < 0)	/* orig eax */
 				return -1;
@@ -994,13 +1000,13 @@ get_scno(struct tcb *tcp)
 			if (upeek(tcp, PT_R15, &scno) < 0)
 				return -1;
 		}
-		/* Check if we return from execve. */
+		/* Check if this is the post-execve SIGTRAP. */
 		if (tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
 		}
 	} else {
-		/* syscall in progress */
+		/* Syscall exit */
 		if (upeek(tcp, PT_R8, &r8) < 0)
 			return -1;
 		if (upeek(tcp, PT_R10, &r10) < 0)
@@ -1017,8 +1023,8 @@ get_scno(struct tcb *tcp)
 	 * We only need to grab the syscall number on syscall entry.
 	 */
 	if (regs.ARM_ip == 0) {
-		if (!(tcp->flags & TCB_INSYSCALL)) {
-			/* Check if we return from execve. */
+		if (entering(tcp)) {
+			/* Check if this is the post-execve SIGTRAP. */
 			if (tcp->flags & TCB_WAITEXECVE) {
 				tcp->flags &= ~TCB_WAITEXECVE;
 				return 0;
@@ -1042,6 +1048,9 @@ get_scno(struct tcb *tcp)
 			if (errno)
 				return -1;
 
+		/* FIXME: bogus check? it is already done on entering before,
+		 * so we never can see it here?
+		 */
 			if (scno == 0 && (tcp->flags & TCB_WAITEXECVE)) {
 				tcp->flags &= ~TCB_WAITEXECVE;
 				return 0;
@@ -1076,13 +1085,13 @@ get_scno(struct tcb *tcp)
 		} else
 			set_personality(0);
 
-		if (tcp->flags & TCB_INSYSCALL) {
-			fprintf(stderr, "pid %d stray syscall entry\n", tcp->pid);
+		if (exiting(tcp)) {
+			fprintf(stderr, "pid %d stray syscall exit\n", tcp->pid);
 			tcp->flags &= ~TCB_INSYSCALL;
 		}
 	} else {
-		if (!(tcp->flags & TCB_INSYSCALL)) {
-			fprintf(stderr, "pid %d stray syscall exit\n", tcp->pid);
+		if (entering(tcp)) {
+			fprintf(stderr, "pid %d stray syscall entry\n", tcp->pid);
 			tcp->flags |= TCB_INSYSCALL;
 		}
 	}
@@ -1097,10 +1106,10 @@ get_scno(struct tcb *tcp)
 	a3 = regs[REG_A3];
 	r2 = regs[REG_V0];
 
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		scno = r2;
 
-		/* Check if we return from execve. */
+		/* Check if this is the post-execve SIGTRAP. */
 		if (scno == 0 && tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
@@ -1117,11 +1126,11 @@ get_scno(struct tcb *tcp)
 # elif defined (MIPS)
 	if (upeek(tcp, REG_A3, &a3) < 0)
 		return -1;
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		if (upeek(tcp, REG_V0, &scno) < 0)
 			return -1;
 
-		/* Check if we return from execve. */
+		/* Check if this is the post-execve SIGTRAP. */
 		if (scno == 0 && tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
@@ -1142,11 +1151,11 @@ get_scno(struct tcb *tcp)
 	if (upeek(tcp, REG_A3, &a3) < 0)
 		return -1;
 
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		if (upeek(tcp, REG_R0, &scno) < 0)
 			return -1;
 
-		/* Check if we return from execve. */
+		/* Check if this is the post-execve SIGTRAP. */
 		if (scno == 0 && tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
@@ -1174,7 +1183,7 @@ get_scno(struct tcb *tcp)
 		return -1;
 
 	/* If we are entering, then disassemble the syscall trap. */
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		/* Retrieve the syscall trap instruction. */
 		errno = 0;
 #  if defined(SPARC64)
@@ -1213,7 +1222,7 @@ get_scno(struct tcb *tcp)
 			set_personality(1);
 			break;
 		default:
-			/* Unknown syscall trap. */
+			/* Check if this is the post-execve SIGTRAP. */
 			if (tcp->flags & TCB_WAITEXECVE) {
 				tcp->flags &= ~TCB_WAITEXECVE;
 				return 0;
@@ -1239,9 +1248,9 @@ get_scno(struct tcb *tcp)
 # elif defined(HPPA)
 	if (upeek(tcp, PT_GR20, &scno) < 0)
 		return -1;
-	if (!(tcp->flags & TCB_INSYSCALL)) {
-		/* Check if we return from execve. */
-		if ((tcp->flags & TCB_WAITEXECVE)) {
+	if (entering(tcp)) {
+		/* Check if this is the post-execve SIGTRAP. */
+		if (tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
 		}
@@ -1267,8 +1276,8 @@ get_scno(struct tcb *tcp)
 		scno = correct_scno;
 	}
 
-	if (!(tcp->flags & TCB_INSYSCALL)) {
-		/* Check if we return from execve. */
+	if (entering(tcp)) {
+		/* Check if this is the post-execve SIGTRAP. */
 		if (scno == 0 && tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
@@ -1279,8 +1288,8 @@ get_scno(struct tcb *tcp)
 		return -1;
 	scno &= 0xFFFF;
 
-	if (!(tcp->flags & TCB_INSYSCALL)) {
-		/* Check if we return from execve. */
+	if (entering(tcp)) {
+		/* Check if this is the post-execve SIGTRAP. */
 		if (tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
@@ -1293,8 +1302,8 @@ get_scno(struct tcb *tcp)
 	if (upeek(tcp, PTREGS_OFFSET_REG(10), &scno) < 0)
 		return -1;
 
-	if (!(tcp->flags & TCB_INSYSCALL)) {
-		/* Check if we return from execve. */
+	if (entering(tcp)) {
+		/* Check if this is the post-execve SIGTRAP. */
 		if (tcp->flags & TCB_WAITEXECVE) {
 			tcp->flags &= ~TCB_WAITEXECVE;
 			return 0;
@@ -1343,7 +1352,7 @@ get_scno(struct tcb *tcp)
 # endif /* !HAVE_PR_SYSCALL */
 #endif /* USE_PROCFS */
 
-	if (!(tcp->flags & TCB_INSYSCALL))
+	if (entering(tcp))
 		tcp->scno = scno;
 	return 1;
 }
@@ -1364,7 +1373,7 @@ known_scno(struct tcb *tcp)
 
 /* Called in trace_syscall() at each syscall entry and exit.
  * Returns:
- * 0: "ignore this syscall", bail out of trace_syscall() silently.
+ * 0: "ignore this ptrace stop", bail out of trace_syscall() silently.
  * 1: ok, continue in trace_syscall().
  * other: error, trace_syscall() should print error indicator
  *    ("????" etc) and bail out.
@@ -1375,7 +1384,7 @@ syscall_fixup(struct tcb *tcp)
 #ifdef USE_PROCFS
 	int scno = known_scno(tcp);
 
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		if (tcp->status.PR_WHY != PR_SYSENTRY) {
 			if (
 			    scno == SYS_fork
@@ -1413,8 +1422,9 @@ syscall_fixup(struct tcb *tcp)
 		}
 	}
 #endif /* USE_PROCFS */
+
 #ifdef SUNOS4
-	if (!(tcp->flags & TCB_INSYSCALL)) {
+	if (entering(tcp)) {
 		if (scno == 0) {
 			fprintf(stderr, "syscall: missing entry\n");
 			tcp->flags |= TCB_INSYSCALL;
@@ -1434,13 +1444,15 @@ syscall_fixup(struct tcb *tcp)
 		}
 	}
 #endif /* SUNOS4 */
+
 #ifdef LINUX
+	/* A common case of "not a syscall entry" is post-execve SIGTRAP */
 #if defined (I386)
 	if (upeek(tcp, 4*EAX, &eax) < 0)
 		return -1;
-	if (eax != -ENOSYS && !(tcp->flags & TCB_INSYSCALL)) {
+	if (eax != -ENOSYS && entering(tcp)) {
 		if (debug)
-			fprintf(stderr, "stray syscall exit: eax = %ld\n", eax);
+			fprintf(stderr, "not a syscall entry (eax = %ld)\n", eax);
 		return 0;
 	}
 #elif defined (X86_64)
@@ -1448,9 +1460,9 @@ syscall_fixup(struct tcb *tcp)
 		return -1;
 	if (current_personality == 1)
 		rax = (long int)(int)rax; /* sign extend from 32 bits */
-	if (rax != -ENOSYS && !(tcp->flags & TCB_INSYSCALL)) {
+	if (rax != -ENOSYS && entering(tcp)) {
 		if (debug)
-			fprintf(stderr, "stray syscall exit: rax = %ld\n", rax);
+			fprintf(stderr, "not a syscall entry (rax = %ld)\n", rax);
 		return 0;
 	}
 #elif defined (S390) || defined (S390X)
@@ -1458,9 +1470,9 @@ syscall_fixup(struct tcb *tcp)
 		return -1;
 	if (syscall_mode != -ENOSYS)
 		syscall_mode = tcp->scno;
-	if (gpr2 != syscall_mode && !(tcp->flags & TCB_INSYSCALL)) {
+	if (gpr2 != syscall_mode && entering(tcp)) {
 		if (debug)
-			fprintf(stderr, "stray syscall exit: gpr2 = %ld\n", gpr2);
+			fprintf(stderr, "not a syscall entry (gpr2 = %ld)\n", gpr2);
 		return 0;
 	}
 	else if (((tcp->flags & (TCB_INSYSCALL|TCB_WAITEXECVE))
@@ -1483,9 +1495,9 @@ syscall_fixup(struct tcb *tcp)
 #elif defined (M68K)
 	if (upeek(tcp, 4*PT_D0, &d0) < 0)
 		return -1;
-	if (d0 != -ENOSYS && !(tcp->flags & TCB_INSYSCALL)) {
+	if (d0 != -ENOSYS && entering(tcp)) {
 		if (debug)
-			fprintf(stderr, "stray syscall exit: d0 = %ld\n", d0);
+			fprintf(stderr, "not a syscall entry (d0 = %ld)\n", d0);
 		return 0;
 	}
 #elif defined (ARM)
@@ -1503,25 +1515,25 @@ syscall_fixup(struct tcb *tcp)
 		return -1;
 	if (upeek(tcp, PT_R8, &r8) < 0)
 		return -1;
-	if (ia32 && r8 != -ENOSYS && !(tcp->flags & TCB_INSYSCALL)) {
+	if (ia32 && r8 != -ENOSYS && entering(tcp)) {
 		if (debug)
-			fprintf(stderr, "stray syscall exit: r8 = %ld\n", r8);
+			fprintf(stderr, "not a syscall entry (r8 = %ld)\n", r8);
 		return 0;
 	}
 #elif defined(CRISV10) || defined(CRISV32)
 	if (upeek(tcp, 4*PT_R10, &r10) < 0)
 		return -1;
-	if (r10 != -ENOSYS && !(tcp->flags & TCB_INSYSCALL)) {
+	if (r10 != -ENOSYS && entering(tcp)) {
 		if (debug)
-			fprintf(stderr, "stray syscall exit: r10 = %ld\n", r10);
+			fprintf(stderr, "not a syscall entry (r10 = %ld)\n", r10);
 		return 0;
 	}
 #elif defined(MICROBLAZE)
 	if (upeek(tcp, 3 * 4, &r3) < 0)
 		return -1;
-	if (r3 != -ENOSYS && !(tcp->flags & TCB_INSYSCALL)) {
+	if (r3 != -ENOSYS && entering(tcp)) {
 		if (debug)
-			fprintf(stderr, "stray syscall exit: r3 = %ld\n", r3);
+			fprintf(stderr, "not a syscall entry (r3 = %ld)\n", r3);
 		return 0;
 	}
 #endif