From 0da91dbac33cdb9d70825da05ab30515c101513a Mon Sep 17 00:00:00 2001
From: Peter Barker <pbarker@barker.dropbear.id.au>
Date: Sun, 16 Aug 2026 09:43:25 +1000
Subject: [PATCH] AP_HAL_SITL: do not truncate the stack dump on a short write

dump_stack_trace() and dump_core_file() both run a script on our own pid
and copy its output to stderr a block at a time.  The copy ended on any
write() which did not place the whole block:

    if (write(2, buf, ret) != ret) {
        // *sigh*
        break;
    }

write() is entitled to do that.  stderr is a pipe when SITL runs under
autotest, so a full pipe shortens a write, and a timer signal can cut one
short with EINTR.  Either way the copy stopped silently, part-way
through, with no "end dumpstack.sh output" line to show that anything was
missing.

Seen in CI: a backtrace ended at frame #8, in the middle of the frame
which would have named the caller - the one thing the dump exists to
provide.  A truncated core dump goes the same way and is even easier to
miss.

Keep writing until the block is out, retry EINTR on both the read and the
write, and say so if a write really does fail.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 libraries/AP_HAL_SITL/system.cpp | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/libraries/AP_HAL_SITL/system.cpp b/libraries/AP_HAL_SITL/system.cpp
index 5b8edd7a947..046f62d5b6e 100644
--- a/libraries/AP_HAL_SITL/system.cpp
+++ b/libraries/AP_HAL_SITL/system.cpp
@@ -1,4 +1,5 @@
 #include <stdarg.h>
+#include <errno.h>
 #include <stdio.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -133,18 +134,40 @@ static void run_command_on_ownpid(const char *commandname)
         fprintf(stderr, "Failed to open stack dump filepath: %m");
         return;
     }
+    fflush(stderr);  // we are about to write(2) around it
     char buf[1024]; // let's hope we're not here because we ran out of stack
     while (true) {
         const ssize_t ret = read(fd, buf, ARRAY_SIZE(buf));
         if (ret == -1) {
+            if (errno == EINTR) {
+                // one of our timers, not a real error
+                continue;
+            }
             fprintf(stderr, "Read error: %m");
             break;
         }
         if (ret == 0) {
             break;
         }
-        if (write(2, buf, ret) != ret) {
-            // *sigh*
+        // write() is allowed to write less than it was asked to - stderr
+        // is a pipe when run from autotest, and a signal can cut a write
+        // short - so keep going until the whole block is out.  Treating a
+        // short write as fatal silently truncated the output, which is
+        // the one thing we came here to collect.
+        ssize_t written = 0;
+        while (written < ret) {
+            const ssize_t wrote = write(2, &buf[written], ret - written);
+            if (wrote == -1) {
+                if (errno == EINTR) {
+                    continue;
+                }
+                break;
+            }
+            written += wrote;
+        }
+        if (written != ret) {
+            fprintf(stderr, "Write error after %d of %d bytes: %m\n",
+                    (int)written, (int)ret);
             break;
         }
     }