Child mode: report the child's exit status, and say why it failed

phpspy.c

@@ -454,7 +454,12 @@ int main_pid(pid_t pid) {
         if (opt_pause) rv |= unpause_pid(pid);
 
         /* bail if pid died */
-        if ((rv & PHPSPY_ERR_PID_DEAD) != 0) break;
+        if ((rv & PHPSPY_ERR_PID_DEAD) != 0) {
+            if (!in_pgrep_mode) {
+                log_error("phpspy: pid %d exited\n", (int)pid);
+            }
+            break;
+        }
 
         /* maybe apply trace limit */
         if (opt_trace_limit > 0 && rv == PHPSPY_OK) {
@@ -508,13 +513,40 @@ static int main_fork(int argc, char **argv) {
         log_perror("fork");
         exit(1);
     }
-    waitpid(fork_pid, &status, 0);
+    if (waitpid(fork_pid, &status, 0) < 0) {
+        log_perror("main_fork: waitpid");
+        return PHPSPY_ERR;
+    }
+
+    /* The child exits before exec if it could not redirect its stdio, so say
+       so rather than pressing on and probing a zombie. */
+    if (WIFEXITED(status)) {
+        log_error("main_fork: Child exited with status %d before exec; see errors above\n", WEXITSTATUS(status));
+        return WEXITSTATUS(status) != 0 ? WEXITSTATUS(status) : 1;
+    }
+    if (WIFSIGNALED(status)) {
+        log_error("main_fork: Child killed by signal %d before exec\n", WTERMSIG(status));
+        return 128 + WTERMSIG(status);
+    }
     if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGTRAP) {
         log_error("main_fork: Expected SIGTRAP from child\n");
     }
+
     ptrace(PTRACE_DETACH, fork_pid, NULL, NULL);
     rv = main_pid(fork_pid);
-    waitpid(fork_pid, NULL, 0);
+    waitpid(fork_pid, &status, 0);
+
+    /* Act as a wrapper and report the child's status, the way strace(1) and
+       time(1) do. A profiling failure is reported on stderr; it must not mask
+       the exit code of the command being profiled, not least because a
+       short-lived child routinely exits before phpspy finishes attaching. */
+    if (WIFEXITED(status)) {
+        return WEXITSTATUS(status);
+    }
+    if (WIFSIGNALED(status)) {
+        return 128 + WTERMSIG(status);
+    }
+
     return rv;
 }
 
@@ -584,7 +616,12 @@ static void redirect_child_stdio(int proc_fd, char *opt_path) {
         }
     }
     if ((redir_file = fopen(redir_path, "w")) == NULL) {
-        log_perror("fopen");
+        log_error(
+            "redirect_child_stdio: Failed to open '%s' for child %s: %s\n",
+            redir_path,
+            proc_fd == STDOUT_FILENO ? "stdout (-O)" : "stderr (-E)",
+            strerror(errno)
+        );
         free(redir_path);
         exit(1);
     }
@@ -738,6 +775,7 @@ static void glopeek_add(char *glospec) {
 static int copy_proc_mem(pid_t pid, const char *what, void *raddr, void *laddr, size_t size) {
     struct iovec local[1];
     struct iovec remote[1];
+    ssize_t nread;
 
     if (raddr == NULL) {
         log_error("copy_proc_mem: Not copying %s; raddr is NULL\n", what);
@@ -749,15 +787,27 @@ static int copy_proc_mem(pid_t pid, const char *what, void *raddr, void *laddr,
     remote[0].iov_base = raddr;
     remote[0].iov_len = size;
 
-    if (process_vm_readv(pid, local, 1, remote, 1, 0) == -1) {
-        if (errno == ESRCH) { /* No such process */
-            log_perror("process_vm_readv");
+    nread = process_vm_readv(pid, local, 1, remote, 1, 0);
+
+    if (nread == -1) {
+        if (errno == ESRCH) {
+            /* The target is gone. This is the ordinary end of a child-mode
+               run, so it is reported once by the sampling loop rather than
+               looking like a failure here. */
             return PHPSPY_ERR | PHPSPY_ERR_PID_DEAD;
         }
         log_error("copy_proc_mem: Failed to copy %s; err=%s raddr=%p size=%lu\n", what, strerror(errno), raddr, size);
         return PHPSPY_ERR;
     }
 
+    if ((size_t)nread != size) {
+        /* A partial read (e.g. straddling a mapping boundary) would leave the
+           tail of the destination as-is; callers treat those bytes as pointers
+           into the target, so refuse the sample instead. */
+        log_error("copy_proc_mem: Short read of %s; got %ld of %lu bytes; raddr=%p\n", what, (long)nread, size, raddr);
+        return PHPSPY_ERR;
+    }
+
     return PHPSPY_OK;
 }
 

tests/test_child_exit_status.sh

@@ -0,0 +1,49 @@
+#!/bin/bash
+
+# In child mode phpspy is a wrapper, so `$?` must be the command's own status.
+# Written standalone rather than through test.sh, which has no exit-code
+# assertion of its own.
+
+check_exit() { # <label> <expected> <php args...>
+    local label="$1" want="$2"; shift 2
+    $PHPSPY --limit=0 --quiet -O/dev/null -E/dev/null -o /dev/null -- "$@" >/dev/null 2>&1
+    local got=$?
+    if [ "$got" -eq "$want" ]; then
+        echo -e "  \x1b[32mOK  \x1b[0m $label (exit=$got)"
+    else
+        echo -e "  \x1b[31mERR \x1b[0m $label\nexpected=$want\n\nactual=$got"
+        exit 1
+    fi
+}
+
+check_exit exit_0   0   $PHP -r 'exit(0);'
+check_exit exit_3   3   $PHP -r 'exit(3);'
+check_exit exit_42  42  $PHP -r 'exit(42);'
+
+# a child killed by a signal reports 128+signum
+if $PHP -r 'exit(function_exists("posix_kill") ? 0 : 1);' 2>/dev/null; then
+    check_exit signalled 143 $PHP -r 'posix_kill(posix_getpid(), SIGTERM); usleep(5000000);'
+else
+    echo -e "  \x1b[33mSKIP\x1b[0m signalled (ext/posix not available)"
+fi
+
+# a target that outlives sampling still reports its own status
+$PHPSPY --limit=0 --time-limit-ms=500 --quiet -O/dev/null -E/dev/null -o /dev/null \
+    -- $PHP -r 'usleep(1500000); exit(7);' >/dev/null 2>&1
+got=$?
+if [ "$got" -eq 7 ]; then
+    echo -e "  \x1b[32mOK  \x1b[0m outlives_sampling (exit=$got)"
+else
+    echo -e "  \x1b[31mERR \x1b[0m outlives_sampling\nexpected=7\n\nactual=$got"
+    exit 1
+fi
+
+# an unwritable -E must name the path, and must not then probe a zombie
+err=$($PHPSPY --limit=0 -O/dev/null -E/nonexistent-dir/x.err -o /dev/null \
+    -- $PHP -r 'usleep(100000);' 2>&1 >/dev/null)
+if grep -q "nonexistent-dir/x.err" <<<"$err" && ! grep -q 'get_php_bin_path' <<<"$err"; then
+    echo -e "  \x1b[32mOK  \x1b[0m names_unwritable_path"
+else
+    echo -e "  \x1b[31mERR \x1b[0m names_unwritable_path\n$err"
+    exit 1
+fi