]> granicus.if.org Git - strace/commitdiff
Remove tcp->parent and TCB_CLONE_THREAD.
authorDenys Vlasenko <dvlasenk@redhat.com>
Wed, 17 Aug 2011 13:18:21 +0000 (15:18 +0200)
committerDenys Vlasenko <dvlasenk@redhat.com>
Wed, 17 Aug 2011 13:18:21 +0000 (15:18 +0200)
tcp->parent is used for only two things:
(1) to send signal on detach via tgkill (need to know tgid).
Solution: use tkill, it needs only tid.
(2) to optimize out ptrace options setting for new tracees.
Not a big deal if we drop this optimization: "set options" op is fast,
doing it just one extra time once per each tracee is hardly measurable.

TCB_CLONE_THREAD is a misnomer. It used only to flag sibling we attached to
in startup_attach. This is used to prevent infinite recursive rescanning
of /proc/PID/task.
Despite the name, there is no guarantee it is set only on non-leader:
if one would run "strace -f -p THREAD_ID" and THREAD_ID is *not*
a thread leader, strace will happily attach to it and all siblings
and will think that THREAD_ID is the leader! Which is a bug, but
since we no longer detach when we think tracee is going to die,
this bug no longer matters, because we do not use the knowledge
about thread group leaders for anything. (We used it to delay
leader's exit).

IOW: after this patch strace has no need to know about threads, parents
and children, and so on. Therefore it does not track that information.
It treats all tracees as independent entities. Overall,
this simplifies code a lot.

* defs.h: Add TCB_ATTACH_DONE flag, remove TCB_CLONE_THREAD flag
and struct tcb::parent field.
* process.c (internal_fork): Don't set tcpchild->parent.
* strace.c (startup_attach): Use TCB_ATTACH_DONE flag instead of
TCB_CLONE_THREAD to avoid attach attempts on already-attached threads.
Unlike TCB_CLONE_THREAD, TCB_ATTACH_DONE bit is used only temporarily,
and only in this function. We clear it on every tcb before we return.
(detach): Use tkill instead of tgkill.
(trace): Set ptrace options on new tracees unconditionally,
not only when tcp->parent == NULL.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
defs.h
process.c
strace.c

diff --git a/defs.h b/defs.h
index 7356fe57369ed4b96ff7b4a49859a38e6acf634e..aaa290e0aa6e24d39777ab2b745d23ac59e1020b 100644 (file)
--- a/defs.h
+++ b/defs.h
@@ -385,7 +385,6 @@ struct tcb {
        struct timeval dtime;   /* Delta for system time usage */
        struct timeval etime;   /* Syscall entry time */
                                /* Support for tracing forked processes */
-       struct tcb *parent;     /* Parent of this process */
        long baddr;             /* `Breakpoint' address */
        long inst[2];           /* Instructions on above */
        int pfd;                /* proc file descriptor */
@@ -411,6 +410,9 @@ 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 */
+#ifdef LINUX
+#define TCB_ATTACH_DONE        00020   /* PTRACE_ATTACH was done on this tcb->pid */
+#endif
 #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 */
@@ -424,9 +426,8 @@ struct tcb {
   || 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 exceve */
+#  define TCB_WAITEXECVE 04000 /* ignore SIGTRAP after execve */
 # endif
-# define TCB_CLONE_THREAD  010000 /* CLONE_THREAD set in creating syscall */
 # include <sys/syscall.h>
 # ifndef __NR_exit_group
 # /* Hack: Most headers around are too old to have __NR_exit_group.  */
index 2d218fc8cf871dbb59e01e7e3ea285d7877fb9ff..12b872098e929adb44d15456a862bf8f40c46c9c 100644 (file)
--- a/process.c
+++ b/process.c
@@ -866,7 +866,6 @@ internal_fork(struct tcb *tcp)
                        memcpy(tcpchild->inst, tcp->inst,
                                sizeof tcpchild->inst);
                }
-               tcpchild->parent = tcp;
                if (!qflag)
                        fprintf(stderr, "Process %d attached\n", pid);
        }
index b2ebb54915cff97a782de819549a3d93b69c5a72..1d79cc448177a6ce13f1f2ce5477fba51b16bd63 100644 (file)
--- a/strace.c
+++ b/strace.c
 
 #ifdef LINUX
 # include <asm/unistd.h>
-# if defined __NR_tgkill
-#  define my_tgkill(pid, tid, sig) syscall(__NR_tgkill, (pid), (tid), (sig))
-# elif defined __NR_tkill
-#  define my_tgkill(pid, tid, sig) syscall(__NR_tkill, (tid), (sig))
+# if defined __NR_tkill
+#  define my_tkill(tid, sig) syscall(__NR_tkill, (tid), (sig))
 # else
    /* kill() may choose arbitrarily the target task of the process group
       while we later wait on a that specific TID.  PID process waits become
       TID task specific waits for a process under ptrace(2).  */
 #  warning "Neither tkill(2) nor tgkill(2) available, risk of strace hangs!"
-#  define my_tgkill(pid, tid, sig) kill((tid), (sig))
+#  define my_tkill(tid, sig) kill((tid), (sig))
 # endif
 #endif
 
@@ -436,10 +434,11 @@ startup_attach(void)
 
        for (tcbi = 0; tcbi < tcbtabsize; tcbi++) {
                tcp = tcbtab[tcbi];
+
                if (!(tcp->flags & TCB_INUSE) || !(tcp->flags & TCB_ATTACHED))
                        continue;
 #ifdef LINUX
-               if (tcp->flags & TCB_CLONE_THREAD)
+               if (tcp->flags & TCB_ATTACH_DONE)
                        continue;
 #endif
                /* Reinitialize the output since it may have changed. */
@@ -479,16 +478,15 @@ startup_attach(void)
                                        else {
                                                if (debug)
                                                        fprintf(stderr, "attach to pid %d succeeded\n", tid);
-                                               if (tid != tcbtab[tcbi]->pid) {
-                                                       tcp = alloctcb(tid);
-                                                       tcp->flags |= TCB_ATTACHED|TCB_CLONE_THREAD;
-                                                       tcp->parent = tcbtab[tcbi];
+                                               if (tid != tcp->pid) {
+                                                       struct tcb *new_tcp = alloctcb(tid);
+                                                       new_tcp->flags |= TCB_ATTACHED|TCB_ATTACH_DONE;
                                                }
                                        }
                                        if (interactive) {
                                                sigprocmask(SIG_SETMASK, &empty_set, NULL);
                                                if (interrupted)
-                                                       return;
+                                                       goto ret;
                                                sigprocmask(SIG_BLOCK, &blocked_set, NULL);
                                        }
                                }
@@ -503,12 +501,12 @@ startup_attach(void)
                                        fprintf(stderr, ntid > 1
 ? "Process %u attached with %u threads - interrupt to quit\n"
 : "Process %u attached - interrupt to quit\n",
-                                               tcbtab[tcbi]->pid, ntid);
+                                               tcp->pid, ntid);
                                }
                                continue;
                        } /* if (opendir worked) */
                } /* if (-f) */
-# endif
+# endif /* LINUX */
                if (ptrace(PTRACE_ATTACH, tcp->pid, (char *) 1, 0) < 0) {
                        perror("attach: ptrace(PTRACE_ATTACH, ...)");
                        droptcb(tcp);
@@ -537,6 +535,15 @@ startup_attach(void)
                                tcp->pid);
        } /* for each tcbtab[] */
 
+ ret:
+#ifdef LINUX
+       /* TCB_ATTACH_DONE flag is used only in this function */
+       for (tcbi = 0; tcbi < tcbtabsize; tcbi++) {
+               tcp = tcbtab[tcbi];
+               tcp->flags &= ~TCB_ATTACH_DONE;
+       }
+#endif
+
        if (interactive)
                sigprocmask(SIG_SETMASK, &empty_set, NULL);
 }
@@ -1621,15 +1628,11 @@ detach(struct tcb *tcp, int sig)
                /* Shouldn't happen. */
                perror("detach: ptrace(PTRACE_DETACH, ...)");
        }
-       else if (my_tgkill((tcp->flags & TCB_CLONE_THREAD ? tcp->parent->pid
-                                                         : tcp->pid),
-                          tcp->pid, 0) < 0) {
+       else if (my_tkill(tcp->pid, 0) < 0) {
                if (errno != ESRCH)
                        perror("detach: checking sanity");
        }
-       else if (!catch_sigstop && my_tgkill((tcp->flags & TCB_CLONE_THREAD
-                                             ? tcp->parent->pid : tcp->pid),
-                                            tcp->pid, SIGSTOP) < 0) {
+       else if (!catch_sigstop && my_tkill(tcp->pid, SIGSTOP) < 0) {
                if (errno != ESRCH)
                        perror("detach: stopping child");
        }
@@ -2443,16 +2446,13 @@ trace()
                                }
                        }
 #ifdef LINUX
-                       /* If options were not set for this tracee yet */
-                       if (tcp->parent == NULL) {
-                               if (ptrace_setoptions) {
-                                       if (debug)
-                                               fprintf(stderr, "setting opts %x on pid %d\n", ptrace_setoptions, tcp->pid);
-                                       if (ptrace(PTRACE_SETOPTIONS, tcp->pid, NULL, ptrace_setoptions) < 0) {
-                                               if (errno != ESRCH) {
-                                                       /* Should never happen, really */
-                                                       perror_msg_and_die("PTRACE_SETOPTIONS");
-                                               }
+                       if (ptrace_setoptions) {
+                               if (debug)
+                                       fprintf(stderr, "setting opts %x on pid %d\n", ptrace_setoptions, tcp->pid);
+                               if (ptrace(PTRACE_SETOPTIONS, tcp->pid, NULL, ptrace_setoptions) < 0) {
+                                       if (errno != ESRCH) {
+                                               /* Should never happen, really */
+                                               perror_msg_and_die("PTRACE_SETOPTIONS");
                                        }
                                }
                        }