Replace reprinting decision logic

After this change, we no longer need to decide when we need
to set TCB_REPRINT, and when we don't: it's never needed :)

Well, almost. That pesky pid-changing execve needs special treatment.
If not it, it'd be possible to nuke TCB_REPRINT...

While at it, fix a case of mishandled -C.

* strace.c (printleader): Do not set TCB_REPRINT.
(trace): Set TCB_REPRINT only for execve with changing pid.
Fix mishandling of -C.
* syscall.c (trace_syscall_entering): Do not clear TCB_REPRINT.
(trace_syscall_exiting): Replace reprinting decision logic.
Remove call to printargs(): it is known to just return 0 here.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

diff --git a/strace.c b/strace.c
index ad29bb40003bfe9355032820ed0d16235da7f4bc..789ea92586d27e1fb70506c80bee100dd1ffec26 100644 (file)
--- a/strace.c
+++ b/strace.c
@@ -560,7 +560,6 @@ printleader(struct tcb *tcp)
                         * didn't finish ("SIGKILL nuked us after syscall entry" etc).
                         */
                        tprints(" <unfinished ...>\n");
-                       printing_tcp->flags |= TCB_REPRINT;
                        printing_tcp->curcol = 0;
                }
        }
@@ -1943,11 +1942,11 @@ trace(void)
                                tcp = execve_thread;
                                if (tcp) {
                                        tcp->pid = pid;
-                                       tcp->flags |= TCB_REPRINT;
-                                       if (!cflag) {
+                                       if (cflag != CFLAG_ONLY_STATS) {
                                                printleader(tcp);
                                                tprintf("+++ superseded by execve in pid %lu +++\n", old_pid);
                                                line_ended();
+                                               tcp->flags |= TCB_REPRINT;
                                        }
                                }
                        }
@@ -2103,6 +2102,8 @@ trace(void)
 
                        /* Nonzero (true) if tracee is stopped by signal
                         * (as opposed to "tracee received signal").
+                        * TODO: shouldn't we check for errno == EINVAL too?
+                        * We can get ESRCH instead, you know...
                         */
                        stopped = (ptrace(PTRACE_GETSIGINFO, pid, 0, (long) &si) < 0);
 #ifdef USE_SEIZE

diff --git a/syscall.c b/syscall.c
index ac0f76999443cfa628217d3168ad598868a58078..663d678b841608cfb6085fad8e750f8d38c675fb 100644 (file)
--- a/syscall.c
+++ b/syscall.c
@@ -1469,7 +1469,6 @@ trace_syscall_entering(struct tcb *tcp)
 
        if (res != 1) {
                printleader(tcp);
-               tcp->flags &= ~TCB_REPRINT;
                if (scno_good != 1)
                        tprints("????" /* anti-trigraph gap */ "(");
                else if (!SCNO_IN_RANGE(tcp->scno))
@@ -1518,7 +1517,6 @@ trace_syscall_entering(struct tcb *tcp)
        }
 
        printleader(tcp);
-       tcp->flags &= ~TCB_REPRINT;
        if (!SCNO_IN_RANGE(tcp->scno))
                tprintf("syscall_%lu(", tcp->scno);
        else
@@ -1918,25 +1916,6 @@ trace_syscall_exiting(struct tcb *tcp)
                }
        }
 
-       /* TODO: TCB_REPRINT is probably not necessary:
-        * we can determine whether reprinting is needed
-        * by examining printing_tcp. Something like:
-        * if not in -ff mode, and printing_tcp != tcp,
-        * then the log is not currently ends with *our*
-        * syscall entry output, but with something else,
-        * and we need to reprint.
-        * If we'd implement this, printing_tcp = tcp
-        * assignments in code below can be made more logical.
-        */
-
-       if (tcp->flags & TCB_REPRINT) {
-               printleader(tcp);
-               if (!SCNO_IN_RANGE(tcp->scno))
-                       tprintf("<... syscall_%lu resumed> ", tcp->scno);
-               else
-                       tprintf("<... %s resumed> ", sysent[tcp->scno].sys_name);
-       }
-
        if (cflag) {
                struct timeval t = tv;
                count_syscall(tcp, &t);
@@ -1945,8 +1924,27 @@ trace_syscall_exiting(struct tcb *tcp)
                }
        }
 
+       /* If not in -ff mode, and printing_tcp != tcp,
+        * then the log currently does not end with output
+        * of _our syscall entry_, but with something else.
+        * We need to say which syscall's return is this.
+        *
+        * Forced reprinting via TCB_REPRINT is used only by
+        * "strace -ff -oLOG test/threaded_execve" corner case.
+        * It's the only case when -ff mode needs reprinting.
+        */
+       if ((followfork < 2 && printing_tcp != tcp) || (tcp->flags & TCB_REPRINT)) {
+               tcp->flags &= ~TCB_REPRINT;
+               printleader(tcp);
+               if (!SCNO_IN_RANGE(tcp->scno))
+                       tprintf("<... syscall_%lu resumed> ", tcp->scno);
+               else
+                       tprintf("<... %s resumed> ", sysent[tcp->scno].sys_name);
+       }
+       printing_tcp = tcp;
+
        if (res != 1) {
-               printing_tcp = tcp;
+               /* There was error in one of prior ptrace ops */
                tprints(") ");
                tabto();
                tprints("= ? <unavailable>\n");
@@ -1955,10 +1953,10 @@ trace_syscall_exiting(struct tcb *tcp)
                return res;
        }
 
+       sys_res = 0;
        if (!SCNO_IN_RANGE(tcp->scno)
            || (qual_flags[tcp->scno] & QUAL_RAW)) {
-               printing_tcp = tcp;
-               sys_res = printargs(tcp);
+               /* sys_res = printargs(tcp); - but it's nop on sysexit */
        } else {
        /* FIXME: not_failing_only (IOW, option -z) is broken:
         * failure of syscall is known only after syscall return.
@@ -1970,7 +1968,6 @@ trace_syscall_exiting(struct tcb *tcp)
         */
                if (not_failing_only && tcp->u_error)
                        goto ret;       /* ignore failed syscalls */
-               printing_tcp = tcp;
                sys_res = (*sysent[tcp->scno].sys_func)(tcp);
        }