Fix a case of broken output if last seen syscall was exit

* defs.h: Rename tcp_last to printing_tcp. Explain what it means.
Remove printtrailer() function.
* process.c (sys_exit): Convert printtrailer() call to "printing_tcp = NULL".
* strace.c: Add new variable printing_tcp.
(cleanup): Convert printtrailer() call to "printing_tcp = NULL".
(trace): Likewise.
(trace): Fix checks for incomplete line - it was working wrongly if last syscall was exit.
(printleader): Set printing_tcp.
(printtrailer): Remove this function.
* syscall.c: Remove tcp_last variable.
(trace_syscall_entering): Don't set printing_tcp, printleader call now does it.
(trace_syscall_exiting): Convert printtrailer() call to "printing_tcp = NULL".

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

diff --git a/defs.h b/defs.h
index 59a8a096a7033de18eb6ca8bf668cfc8d3f3b059..108b5321112bfa4e73f9bcbc2df933c577c6cbfc 100644 (file)
--- a/defs.h
+++ b/defs.h
@@ -584,7 +584,12 @@ extern unsigned int ptrace_setoptions;
 extern int dtime, xflag, qflag;
 extern cflag_t cflag;
 extern int max_strlen;
-extern struct tcb *tcp_last;
+/*
+ * Which tcb has incomplete line being printed right now?
+ * NULL if last line has been completed ('\n'-terminated).
+ * printleader(tcp) sets it. Clearing is open-coded.
+ */
+extern struct tcb *printing_tcp;
 
 enum bitness_t { BITNESS_CURRENT = 0, BITNESS_32 };
 
@@ -658,7 +663,6 @@ extern const char *signame(int);
 extern void print_sigset(struct tcb *, long, int);
 extern void printsignal(int);
 extern void printleader(struct tcb *);
-extern void printtrailer(void);
 extern void tabto(void);
 extern void call_summary(FILE *);
 extern void tprint_iov(struct tcb *, unsigned long, unsigned long, int decode_iov);

diff --git a/process.c b/process.c
index 33033a4360e7802b547db37dfdb380fa2cd0f3bb..d93a19034942a82f24729f4a6e0b91a47d68ba52 100644 (file)
--- a/process.c
+++ b/process.c
@@ -430,8 +430,8 @@ sys_exit(struct tcb *tcp)
        /* special case: we stop tracing this process, finish line now */
        tprintf("%ld) ", tcp->u_arg[0]);
        tabto();
-       tprints("= ?");
-       printtrailer();
+       tprints("= ?\n");
+       printing_tcp = NULL;
        return 0;
 }
 

diff --git a/strace.c b/strace.c
index 7cbcb0cde38d97b299914c4cf8e585c6dc628efb..84b35c8b0de2ba9a8e24ac260669c52fe7565513 100644 (file)
--- a/strace.c
+++ b/strace.c
@@ -125,6 +125,7 @@ static int acolumn = DEFAULT_ACOLUMN;
 static char *acolumn_spaces;
 static char *outfname = NULL;
 static FILE *outf;
+struct tcb *printing_tcp = NULL;
 static int curcol;
 static struct tcb **tcbtab;
 static unsigned int nprocs, tcbtabsize;
@@ -1844,10 +1845,10 @@ cleanup(void)
                if (debug)
                        fprintf(stderr,
                                "cleanup: looking at pid %u\n", tcp->pid);
-               if (tcp_last &&
-                   (!outfname || followfork < 2 || tcp_last == tcp)) {
-                       tprints(" <unfinished ...>");
-                       printtrailer();
+               if (printing_tcp &&
+                   (!outfname || followfork < 2 || printing_tcp == tcp)) {
+                       tprints(" <unfinished ...>\n");
+                       printing_tcp = NULL;
                }
                if (tcp->flags & TCB_ATTACHED)
                        detach(tcp);
@@ -2292,15 +2293,16 @@ trace(void)
                        if (cflag != CFLAG_ONLY_STATS
                            && (qual_flags[what] & QUAL_SIGNAL)) {
                                printleader(tcp);
-                               tprintf("--- %s (%s) ---",
+                               tprintf("--- %s (%s) ---\n",
                                        signame(what), strsignal(what));
-                               printtrailer();
+                               printing_tcp = NULL;
 #ifdef PR_INFO
                                if (tcp->status.PR_INFO.si_signo == what) {
                                        printleader(tcp);
                                        tprints("    siginfo=");
                                        printsiginfo(&tcp->status.PR_INFO, 1);
-                                       printtrailer();
+                                       tprints("\n");
+                                       printing_tcp = NULL;
                                }
 #endif
                        }
@@ -2309,8 +2311,8 @@ trace(void)
                        if (cflag != CFLAGS_ONLY_STATS
                            && (qual_flags[what] & QUAL_FAULT)) {
                                printleader(tcp);
-                               tprintf("=== FAULT %d ===", what);
-                               printtrailer();
+                               tprintf("=== FAULT %d ===\n", what);
+                               printing_tcp = NULL;
                        }
                        break;
 #ifdef FREEBSD
@@ -2483,15 +2485,11 @@ trace()
                                        outf = tcp->outf;
                                        curcol = tcp->curcol;
                                        if (!cflag) {
-                                               if ((tcp->flags & (TCB_INSYSCALL|TCB_REPRINT)) == TCB_INSYSCALL) {
-                                                       /* We printed "syscall(some params"
-                                                        * but didn't print "\n" yet.
-                                                        */
+                                               if (printing_tcp)
                                                        tprints(" <unfinished ...>\n");
-                                               }
                                                printleader(tcp);
-                                               tprintf("+++ superseded by execve in pid %lu +++", old_pid);
-                                               printtrailer();
+                                               tprintf("+++ superseded by execve in pid %lu +++\n", old_pid);
+                                               printing_tcp = NULL;
                                                fflush(outf);
                                        }
                                        if (execve_thread) {
@@ -2555,14 +2553,14 @@ trace()
                            && (qual_flags[WTERMSIG(status)] & QUAL_SIGNAL)) {
                                printleader(tcp);
 #ifdef WCOREDUMP
-                               tprintf("+++ killed by %s %s+++",
+                               tprintf("+++ killed by %s %s+++\n",
                                        signame(WTERMSIG(status)),
                                        WCOREDUMP(status) ? "(core dumped) " : "");
 #else
-                               tprintf("+++ killed by %s +++",
+                               tprintf("+++ killed by %s +++\n",
                                        signame(WTERMSIG(status)));
 #endif
-                               printtrailer();
+                               printing_tcp = NULL;
                        }
                        fflush(tcp->outf);
                        droptcb(tcp);
@@ -2571,16 +2569,14 @@ trace()
                if (WIFEXITED(status)) {
                        if (pid == strace_child)
                                exit_code = WEXITSTATUS(status);
-                       if (tcp == tcp_last) {
-                               if ((tcp->flags & (TCB_INSYSCALL|TCB_REPRINT)) == TCB_INSYSCALL)
-                                       tprintf(" <unfinished ... exit status %d>\n",
-                                               WEXITSTATUS(status));
-                               tcp_last = NULL;
+                       if (tcp == printing_tcp) {
+                               tprints(" <unfinished ...>\n");
+                               printing_tcp = NULL;
                        }
                        if (!cflag /* && (qual_flags[WTERMSIG(status)] & QUAL_SIGNAL) */ ) {
                                printleader(tcp);
-                               tprintf("+++ exited with %d +++", WEXITSTATUS(status));
-                               printtrailer();
+                               tprintf("+++ exited with %d +++\n", WEXITSTATUS(status));
+                               printing_tcp = NULL;
                        }
                        fflush(tcp->outf);
                        droptcb(tcp);
@@ -2666,15 +2662,15 @@ trace()
                                if (ptrace(PTRACE_GETSIGINFO, pid, 0, (long) &si) == 0) {
                                        tprints("--- ");
                                        printsiginfo(&si, verbose(tcp));
-                                       tprintf(" (%s)" PC_FORMAT_STR " ---",
+                                       tprintf(" (%s)" PC_FORMAT_STR " ---\n",
                                                strsignal(sig)
                                                PC_FORMAT_ARG);
                                } else
-                                       tprintf("--- %s by %s" PC_FORMAT_STR " ---",
+                                       tprintf("--- %s by %s" PC_FORMAT_STR " ---\n",
                                                strsignal(sig),
                                                signame(sig)
                                                PC_FORMAT_ARG);
-                               printtrailer();
+                               printing_tcp = NULL;
                                fflush(tcp->outf);
                        }
                        goto restart_tracee;
@@ -2695,13 +2691,13 @@ trace()
                         * all processes in thread group.
                         */
                        if (tcp->flags & TCB_ATTACHED) {
-                               if (tcp_last) {
+                               if (printing_tcp) {
                                        /* Do we have dangling line "syscall(param, param"?
                                         * Finish the line then.
                                         */
-                                       tcp_last->flags |= TCB_REPRINT;
-                                       tprints(" <unfinished ...>");
-                                       printtrailer();
+                                       printing_tcp->flags |= TCB_REPRINT;
+                                       tprints(" <unfinished ...>\n");
+                                       printing_tcp = NULL;
                                        fflush(tcp->outf);
                                }
                                /* We assume that ptrace error was caused by process death.
@@ -2768,19 +2764,21 @@ tprints(const char *str)
 void
 printleader(struct tcb *tcp)
 {
-       if (tcp_last) {
-               if (tcp_last->ptrace_errno) {
-                       if (tcp_last->flags & TCB_INSYSCALL) {
+       if (printing_tcp) {
+               if (printing_tcp->ptrace_errno) {
+                       if (printing_tcp->flags & TCB_INSYSCALL) {
                                tprints(" <unavailable>) ");
                                tabto();
                        }
                        tprints("= ? <unavailable>\n");
-                       tcp_last->ptrace_errno = 0;
-               } else if (!outfname || followfork < 2 || tcp_last == tcp) {
-                       tcp_last->flags |= TCB_REPRINT;
+                       printing_tcp->ptrace_errno = 0;
+               } else if (!outfname || followfork < 2 || printing_tcp == tcp) {
+                       printing_tcp->flags |= TCB_REPRINT;
                        tprints(" <unfinished ...>\n");
                }
        }
+
+       printing_tcp = tcp;
        curcol = 0;
        if ((followfork == 1 || pflag_seen > 1) && outfname)
                tprintf("%-5d ", tcp->pid);
@@ -2824,13 +2822,6 @@ tabto(void)
                tprints(acolumn_spaces + curcol);
 }
 
-void
-printtrailer(void)
-{
-       tprints("\n");
-       tcp_last = NULL;
-}
-
 #ifdef HAVE_MP_PROCFS
 
 int

diff --git a/syscall.c b/syscall.c
index 9c04cf90fe0080b907b5bcc2429ce03397148849..88c976629957d9052e5cca2f424dbd54f9931429 100644 (file)
--- a/syscall.c
+++ b/syscall.c
@@ -731,8 +731,6 @@ is_restart_error(struct tcb *tcp)
        return 0;
 }
 
-struct tcb *tcp_last = NULL;
-
 #ifdef LINUX
 # if defined(I386)
 struct pt_regs i386_regs;
@@ -1724,7 +1722,6 @@ trace_syscall_entering(struct tcb *tcp)
        if (res != 1) {
                printleader(tcp);
                tcp->flags &= ~TCB_REPRINT;
-               tcp_last = tcp;
                if (scno_good != 1)
                        tprintf("????" /* anti-trigraph gap */ "(");
                else if (!SCNO_IN_RANGE(tcp->scno))
@@ -1844,7 +1841,6 @@ trace_syscall_entering(struct tcb *tcp)
 
        printleader(tcp);
        tcp->flags &= ~TCB_REPRINT;
-       tcp_last = tcp;
        if (!SCNO_IN_RANGE(tcp->scno))
                tprintf("syscall_%lu(", tcp->scno);
        else
@@ -2382,8 +2378,8 @@ trace_syscall_exiting(struct tcb *tcp)
        if (res != 1) {
                tprints(") ");
                tabto();
-               tprints("= ? <unavailable>");
-               printtrailer();
+               tprints("= ? <unavailable>\n");
+               printing_tcp = NULL;
                tcp->flags &= ~TCB_INSYSCALL;
                return res;
        }
@@ -2530,7 +2526,8 @@ trace_syscall_exiting(struct tcb *tcp)
                tprintf(" <%ld.%06ld>",
                        (long) tv.tv_sec, (long) tv.tv_usec);
        }
-       printtrailer();
+       tprints("\n");
+       printing_tcp = NULL;
 
        dumpio(tcp);
        if (fflush(tcp->outf) == EOF)

diff --git a/test/threaded_execve.c b/test/threaded_execve.c
index 51a43606a08a289efc89e95cd18be90b4f9a03ed..9cf3dae2ddec64c58b8ff42aa1d4a4150f14aaa6 100644 (file)
--- a/test/threaded_execve.c
+++ b/test/threaded_execve.c
@@ -11,7 +11,6 @@
  * # Should not be confused by traced execve-ing thread
  * # replacing traced leader:
  * [LEADER_EXIT=1] strace -oLOG -f ./threaded_execve
- * ^^^ so far slightly bad output with LEADER_EXIT=1
  *
  * # Same, but different output mode. Output after execve
  * # should go into leader's LOG.<pid> file, not into execve'ed
@@ -24,7 +23,7 @@
  * ^^^^^^^^^^^^^^^^^^^^^
  * In Linux 3.2, non-traced execve-ing thread does not
  * become traced after execve, even though it has pid == leader's pid
- * after execve.
+ * after execve. And yet, strace's waitpid doesn't return ECHILD.
  *
  * # Run for NUM seconds, not just one second.
  * # Watch top to check for memory leaks in strace: