Fix regression introduced by "Properly handle real SIGTRAPs" change

Commit 3454e4b463e6c22c7ea8c5461ef5a077f4650a54
introduced a bug: sometimes, TRACECLONE/TRACE[V]FORK opts were not set.
The check (tcp->parent == NULL) in old code was meant to check
"if we are not a child created by auto-attach" - in this case,
options need to be set on the child; otherwise they are inherited
and do not need to be set.
I misunderstood the check and if tcp->parent is not NULL, I was
setting only ptrace_setoptions_for_all bits.
This change fixes the problem. Since the fixed logic makes it
unnecessary to keep two sets of options in separate variables,
I merge them back into one variable, ptrace_setoptions.

* defs.h: Merge ptrace_setoptions_followfork and ptrace_setoptions_for_all
  into one variable, ptrace_setoptions.
* strace.c: Likewise.
  (test_ptrace_setoptions_followfork): Use ptrace_setoptions variable.
  (test_ptrace_setoptions_for_all): Likewise.
  (main): Likewise.
* process.c (internal_fork): Likewise.
  (internal_exec): Likewise.
* strace.c (trace): Fix the bug where different options were set
  depending on "tcp->parent == NULL" condition. Add a comment
  which makes it more clear why this condition is checked.

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

diff --git a/defs.h b/defs.h
index b2ca92332f82039e3483daa974a3dbfa9ab8d6c2..75b002dc7157d410cc0bc7e43420493e62ea8c79 100644 (file)
--- a/defs.h
+++ b/defs.h
@@ -508,8 +508,7 @@ typedef enum {
 extern struct tcb **tcbtab;
 extern int *qual_flags;
 extern int debug, followfork;
-extern unsigned int ptrace_setoptions_followfork;
-extern unsigned int ptrace_setoptions_for_all;
+extern unsigned int ptrace_setoptions;
 extern int dtime, xflag, qflag;
 extern cflag_t cflag;
 extern int acolumn;

diff --git a/process.c b/process.c
index fc5fa90c0237f5a294e4d09ef9146942c09e2bef..0584f54773f3e2c63b2234965738c6dfc35f14ab 100644 (file)
--- a/process.c
+++ b/process.c
@@ -904,7 +904,7 @@ Process %u resumed (parent %d ready)\n",
 int
 internal_fork(struct tcb *tcp)
 {
-       if ((ptrace_setoptions_followfork
+       if ((ptrace_setoptions
            & (PTRACE_O_TRACECLONE | PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK))
           == (PTRACE_O_TRACECLONE | PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK))
                return 0;
@@ -1751,7 +1751,7 @@ internal_exec(struct tcb *tcp)
                tcp->flags &= ~TCB_WAITEXECVE;
        else {
                /* Maybe we have post-execve SIGTRAP suppressed? */
-               if (!(ptrace_setoptions_for_all & PTRACE_O_TRACEEXEC))
+               if (!(ptrace_setoptions & PTRACE_O_TRACEEXEC))
                        tcp->flags |= TCB_WAITEXECVE; /* no */
        }
 #endif /* LINUX && TCB_WAITEXECVE */

diff --git a/strace.c b/strace.c
index 3298795a19147cb163287fa574c742eaf17af2e7..93e6d29fb90446bf1371247d26baf917a2c99100 100644 (file)
--- a/strace.c
+++ b/strace.c
@@ -84,8 +84,7 @@ extern char *optarg;
 
 
 int debug = 0, followfork = 0;
-unsigned int ptrace_setoptions_followfork = 0;
-unsigned int ptrace_setoptions_for_all = 0;
+unsigned int ptrace_setoptions = 0;
 /* Which WSTOPSIG(status) value marks syscall traps? */
 static unsigned int syscall_trap_sig = SIGTRAP;
 int dtime = 0, xflag = 0, qflag = 0;
@@ -826,7 +825,7 @@ test_ptrace_setoptions_followfork(void)
                }
        }
        if (expected_grandchild && expected_grandchild == found_grandchild)
-               ptrace_setoptions_followfork |= test_options;
+               ptrace_setoptions |= test_options;
        return 0;
 }
 
@@ -907,10 +906,10 @@ test_ptrace_setoptions_for_all(void)
 
        if (it_worked) {
                syscall_trap_sig = (SIGTRAP | 0x80);
-               ptrace_setoptions_for_all = test_options;
+               ptrace_setoptions |= test_options;
                if (debug)
-                       fprintf(stderr, "ptrace_setoptions_for_all = %#x\n",
-                               ptrace_setoptions_for_all);
+                       fprintf(stderr, "ptrace_setoptions = %#x\n",
+                               ptrace_setoptions);
                return;
        }
 
@@ -1136,11 +1135,11 @@ main(int argc, char *argv[])
                        fprintf(stderr,
                                "Test for options supported by PTRACE_SETOPTIONS "
                                "failed, giving up using this feature.\n");
-                       ptrace_setoptions_followfork = 0;
+                       ptrace_setoptions = 0;
                }
                if (debug)
-                       fprintf(stderr, "ptrace_setoptions_followfork = %#x\n",
-                               ptrace_setoptions_followfork);
+                       fprintf(stderr, "ptrace_setoptions = %#x\n",
+                               ptrace_setoptions);
        }
        test_ptrace_setoptions_for_all();
 #endif
@@ -2679,16 +2678,16 @@ Process %d attached (waiting for parent)\n",
                                }
                        }
 #ifdef LINUX
-                       int options = ptrace_setoptions_for_all;
-                       if (followfork && (tcp->parent == NULL))
-                               options |= ptrace_setoptions_followfork;
-                       if (options) {
-                               if (debug)
-                                       fprintf(stderr, "setting opts %x on pid %d\n", options, tcp->pid);
-                               if (ptrace(PTRACE_SETOPTIONS, tcp->pid, NULL, options) < 0) {
-                                       if (errno != ESRCH) {
-                                               /* Should never happen, really */
-                                               perror_msg_and_die("PTRACE_SETOPTIONS");
+                       /* 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");
+                                               }
                                        }
                                }
                        }