[prev in list] [next in list] [prev in thread] [next in thread] 

List:       strace
Subject:    [PATCH 1/2] remove TCB_SUSPENDED
From:       Denys Vlasenko <dvlasenk () redhat ! com>
Date:       2011-06-25 9:34:32
Message-ID: 1308994473.1791.4.camel () dhcp-25-63 ! brq ! redhat ! com
[Download RAW message or body]

This patch is on top of "do not detach when we think tracee is going to die".

Since we no longer suspend waitpid'ing tracees, we have only one case when
we suspend tracee: when we pick up a new tracee created by clone/fork/vfork.

Background: on some other OSes, attach to child is done this way:
get fork's result (pid), loop ptrace(PTRACE_ATTACH) until you hook up
new process/thread. This is ugly and not safe, but what matters for us
is that it doesn't require suspending. 

Only on Linux, it does. On Linux, we use two methods of catching
new tracee: adding CLONE_THREAD bit to syscall (if needed, we change
[v]fork into clone before that), or using ptrace options.
In both cases, it may be so that new tracee appears before one which
created it returns from syscall. In this case, current code
suspends new tracee until its creator returns. Only then
strace can determine who is its parent (it needs child's pid for this).
This is inherently racy. For example, what if SIGKILL kills
creator after it succeeded creating child, but before it returns?
Looks like we will have child suspended forever.

But after "do not detach when we think tracee is going to die" patch,
we DO NOT NEED parent<->child link for anything. Therefore
we do not need suspending too. Bingo!

This patch removes suspending code. Now new tracees will be continued
right away. Next patch will remove tcp->parent member.

Please review.

-- 
vda




diff -d -urpN strace.6/defs.h strace.7/defs.h
--- strace.6/defs.h	2011-06-24 22:44:51.642954549 +0200
+++ strace.7/defs.h	2011-06-24 22:45:02.910919655 +0200
@@ -392,7 +392,6 @@ struct tcb {
 #define TCB_INUSE	00002	/* This table entry is in use */
 #define TCB_INSYSCALL	00004	/* A system call is in progress */
 #define TCB_ATTACHED	00010	/* Process is not our own child */
-#define TCB_SUSPENDED	00040	/* Process can not be allowed to resume just now */
 #define TCB_BPTSET	00100	/* "Breakpoint" set after fork(2) */
 #define TCB_SIGTRAPPED	00200	/* Process wanted to block SIGTRAP */
 #define TCB_REPRINT	01000	/* We should reprint this syscall on exit */
diff -d -urpN strace.6/process.c strace.7/process.c
--- strace.6/process.c	2011-06-24 00:05:03.614985099 +0200
+++ strace.7/process.c	2011-06-24 02:16:01.492073781 +0200
@@ -762,94 +762,6 @@ change_syscall(struct tcb *tcp, int new)
 }
 
 #ifdef LINUX
-int
-handle_new_child(struct tcb *tcp, int pid, int bpt)
-{
-	struct tcb *tcpchild;
-
-	tcpchild = pid2tcb(pid);
-	if (tcpchild != NULL) {
-		/* The child already reported its startup trap
-		   before the parent reported its syscall return.  */
-		if ((tcpchild->flags
-		     & (TCB_STARTUP|TCB_ATTACHED|TCB_SUSPENDED))
-		    != (TCB_STARTUP|TCB_ATTACHED|TCB_SUSPENDED))
-			fprintf(stderr, "\
-[preattached child %d of %d in weird state!]\n",
-				pid, tcp->pid);
-	}
-	else {
-		tcpchild = alloctcb(pid);
-	}
-
-	tcpchild->flags |= TCB_ATTACHED;
-
-	if (bpt) {
-		clearbpt(tcp);
-		/* Child has BPT too, must be removed on first occasion.  */
-		tcpchild->flags |= TCB_BPTSET;
-		tcpchild->baddr = tcp->baddr;
-		memcpy(tcpchild->inst, tcp->inst,
-			sizeof tcpchild->inst);
-	}
-	tcpchild->parent = tcp;
-	if (tcpchild->flags & TCB_SUSPENDED) {
-		/* The child was born suspended, due to our having
-		   forced CLONE_PTRACE.  */
-		if (bpt)
-			clearbpt(tcpchild);
-
-		tcpchild->flags &= ~(TCB_SUSPENDED|TCB_STARTUP);
-		if (ptrace_restart(PTRACE_SYSCALL, tcpchild, 0) < 0)
-			return -1;
-
-		if (!qflag)
-			fprintf(stderr, "\
-Process %u resumed (parent %d ready)\n",
-				pid, tcp->pid);
-	}
-	else {
-		if (!qflag)
-			fprintf(stderr, "Process %d attached\n", pid);
-	}
-
-#ifdef TCB_CLONE_THREAD
-	if (sysent[tcp->scno].sys_func == sys_clone) {
-		/*
-		 * Save the flags used in this call,
-		 * in case we point TCP to our parent below.
-		 */
-		int call_flags = tcp->u_arg[ARG_FLAGS];
-		if ((tcp->flags & TCB_CLONE_THREAD) &&
-		    tcp->parent != NULL) {
-			/* The parent in this clone is itself a
-			   thread belonging to another process.
-			   There is no meaning to the parentage
-			   relationship of the new child with the
-			   thread, only with the process.  We
-			   associate the new thread with our
-			   parent.  Since this is done for every
-			   new thread, there will never be a
-			   TCB_CLONE_THREAD process that has
-			   children.  */
-			tcp = tcp->parent;
-			tcpchild->parent = tcp;
-		}
-		if (call_flags & CLONE_THREAD) {
-			tcpchild->flags |= TCB_CLONE_THREAD;
-		}
-		if ((call_flags & CLONE_PARENT) &&
-		    !(call_flags & CLONE_THREAD)) {
-			tcpchild->parent = NULL;
-			if (tcp->parent != NULL) {
-				tcp = tcp->parent;
-				tcpchild->parent = tcp;
-			}
-		}
-	}
-#endif /* TCB_CLONE_THREAD */
-	return 0;
-}
 
 int
 internal_fork(struct tcb *tcp)
@@ -864,29 +776,17 @@ internal_fork(struct tcb *tcp)
 
 	if (entering(tcp)) {
 		/*
-		 * In occasion of using PTRACE_O_TRACECLONE, we won't see the
-		 * new child if clone is called with flag CLONE_UNTRACED, so
-		 * we keep the same logic with that option and don't trace it.
+		 * We won't see the new child if clone is called with
+		 * CLONE_UNTRACED, so we keep the same logic with that option
+		 * and don't trace it.
 		 */
 		if ((sysent[tcp->scno].sys_func == sys_clone) &&
 		    (tcp->u_arg[ARG_FLAGS] & CLONE_UNTRACED))
 			return 0;
 		setbpt(tcp);
 	} else {
-		int pid;
-		int bpt;
-
-		bpt = tcp->flags & TCB_BPTSET;
-
-		if (syserror(tcp)) {
-			if (bpt)
-				clearbpt(tcp);
-			return 0;
-		}
-
-		pid = tcp->u_rval;
-
-		return handle_new_child(tcp, pid, bpt);
+		if (tcp->flags & TCB_BPTSET)
+			clearbpt(tcp);
 	}
 	return 0;
 }
diff -d -urpN strace.6/strace.c strace.7/strace.c
--- strace.6/strace.c	2011-06-24 22:44:51.643954446 +0200
+++ strace.7/strace.c	2011-06-24 22:45:02.911919723 +0200
@@ -2216,32 +2216,6 @@ trace(void)
 
 #else /* !USE_PROCFS */
 
-#ifdef LINUX
-static int
-handle_ptrace_event(int status, struct tcb *tcp)
-{
-	if (status >> 16 == PTRACE_EVENT_VFORK ||
-	    status >> 16 == PTRACE_EVENT_CLONE ||
-	    status >> 16 == PTRACE_EVENT_FORK) {
-		long childpid;
-
-		if (do_ptrace(PTRACE_GETEVENTMSG, tcp, NULL, &childpid) < 0) {
-			if (errno != ESRCH) {
-				error_msg_and_die("Cannot get new child's pid");
-			}
-			return -1;
-		}
-		return handle_new_child(tcp, childpid, 0);
-	}
-	if (status >> 16 == PTRACE_EVENT_EXEC) {
-		return 0;
-	}
-	/* Some PTRACE_EVENT_foo we didn't ask for?! */
-	error_msg("Unexpected status %x on pid %d", status, tcp->pid);
-	return 1;
-}
-#endif
-
 static int
 trace()
 {
@@ -2359,10 +2333,9 @@ trace()
 				   child so that we know how to do clearbpt
 				   in the child.  */
 				tcp = alloctcb(pid);
-				tcp->flags |= TCB_ATTACHED | TCB_SUSPENDED;
+				tcp->flags |= TCB_ATTACHED;
 				if (!qflag)
-					fprintf(stderr, "\
-Process %d attached (waiting for parent)\n",
+					fprintf(stderr, "Process %d attached\n",
 						pid);
 			}
 			else
@@ -2385,17 +2358,6 @@ Process %d attached (waiting for parent)
 		}
 #endif
 
-		if (tcp->flags & TCB_SUSPENDED) {
-			/*
-			 * Apparently, doing any ptrace() call on a stopped
-			 * process, provokes the kernel to report the process
-			 * status again on a subsequent wait(), even if the
-			 * process has not been actually restarted.
-			 * Since we have inspected the arguments of suspended
-			 * processes we end up here testing for this case.
-			 */
-			continue;
-		}
 		if (WIFSIGNALED(status)) {
 			if (pid == strace_child)
 				exit_code = 0x100 | WTERMSIG(status);
@@ -2439,8 +2401,8 @@ Process %d attached (waiting for parent)
 		}
 
 		if (status >> 16) {
-			if (handle_ptrace_event(status, tcp) != 1)
-				goto tracing;
+			/* Ptrace event (we ignore all of them for now) */
+			goto tracing;
 		}
 
 		/*
@@ -2537,7 +2499,6 @@ Process %d attached (waiting for parent)
 				cleanup();
 				return -1;
 			}
-			tcp->flags &= ~TCB_SUSPENDED;
 			continue;
 		}
 		/* we handled the STATUS, we are permitted to interrupt now. */
@@ -2568,11 +2529,6 @@ Process %d attached (waiting for parent)
 			}
 			continue;
 		}
-		if (tcp->flags & TCB_SUSPENDED) {
-			if (!qflag)
-				fprintf(stderr, "Process %u suspended\n", pid);
-			continue;
-		}
 	tracing:
 		/* Remember current print column before continuing. */
 		tcp->curcol = curcol;




------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a 
definitive record of customers, application performance, security 
threats, fraudulent activity and more. Splunk takes this data and makes 
sense of it. Business sense. IT sense. Common sense.. 
http://p.sf.net/sfu/splunk-d2d-c1
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic