host: launch watchdog sampling without shell parsing

dm/host/crash.c

@@ -11,6 +11,12 @@
 #include <mach/mach.h>
 #include <stdatomic.h>
 #include <time.h>
+#if TARGET_OS_OSX
+#include <crt_externs.h>
+#include <fcntl.h>
+#include <spawn.h>
+#include <sys/wait.h>
+#endif
 
 static _Atomic uint64_t g_watch_last_ns;
 static _Atomic uint64_t g_watch_count;
@@ -39,6 +45,28 @@ void crash_watch_frame(void) { if (atomic_load(&g_watch_pregame)) crash_watch_pr
 void crash_watch_pregame_done(void) { atomic_store(&g_watch_pregame, 0); }
 void crash_watch_pregame_again(void) { atomic_store(&g_watch_pregame, 1); }   // Back from a run to the front end: count progress by presented frames again (diag.c)
 void crash_dump_now(const char *why);
+#if TARGET_OS_OSX
+// Keep the output path as one argument: shell quoting breaks apostrophes and can
+// execute path contents, and a command buffer can truncate a long path.
+static int watch_sample(const char *path) {
+    char pid[32];
+    snprintf(pid, sizeof pid, "%d", (int)getpid());
+    char *argv[] = {"/usr/bin/sample", pid, "1", "-f", (char *)path, NULL};
+    posix_spawn_file_actions_t actions;
+    int error = posix_spawn_file_actions_init(&actions);
+    if (error) return -1;
+    error = posix_spawn_file_actions_addopen(&actions, STDOUT_FILENO, "/dev/null", O_WRONLY, 0);
+    if (!error) error = posix_spawn_file_actions_adddup2(&actions, STDOUT_FILENO, STDERR_FILENO);
+    pid_t child;
+    if (!error) error = posix_spawn(&child, argv[0], &actions, NULL, argv, *_NSGetEnviron());
+    posix_spawn_file_actions_destroy(&actions);
+    if (error) return -1;
+    int status;
+    pid_t waited;
+    do { waited = waitpid(child, &status, 0); } while (waited < 0 && errno == EINTR);
+    return waited == child && WIFEXITED(status) && WEXITSTATUS(status) == 0 ? 0 : -1;
+}
+#endif
 static void *watchdog_main(void *arg) {
     (void)arg;
     for (;;) {
@@ -61,7 +89,7 @@ static void *watchdog_main(void *arg) {
             if (g_wait_dump) g_wait_dump();   // State of all blocking wait points on the graphics side (registered when nvn_mtl.m initializes)
 #if TARGET_OS_OSX
             { const char *d = getenv("DM_WATCH_SAMPLE");   // Mac diagnostics: when stuck, sample itself for 1 second and write the thread stacks to this path
-              if (d) { char c[1200]; snprintf(c, sizeof c, "/usr/bin/sample %d 1 -f '%s' >/dev/null 2>&1", (int)getpid(), d); if (system(c)) LOG("watchdog: sampling failed"); else LOG("watchdog: thread stacks written to %s", d); } }
+              if (d) { if (watch_sample(d)) LOG("watchdog: sampling failed"); else LOG("watchdog: thread stacks written to %s", d); } }
 #endif
             _exit(128 + SIGALRM);
         }

dm/host/tests/Makefile

@@ -169,6 +169,12 @@ $(B)/t_swkbd: t_swkbd.c ../hle_swkbd.c ../hle.h ../dm.h $(GAME_H) | $(B)
 run: $(B)/t_bigstack
 $(B)/t_bigstack: t_bigstack.c ../hle_bigstack.c ../hle.h ../dm.h $(GAME_H) | $(B)
 	$(CLANG) -O1 -Wall -Wno-unused-function -I.. $(GINC) -o $@ t_bigstack.c ../hle_bigstack.c
+run: watch-sample
+$(B)/t_watch_sample: t_watch_sample.c ../crash.c ../dm.h $(GAME_H) | $(B)
+	$(CLANG) -O1 -g -Wall -Werror -Wno-unused-function -I.. $(GINC) -o $@ t_watch_sample.c
+.PHONY: watch-sample
+watch-sample: $(B)/t_watch_sample
+	$(B)/t_watch_sample
 $(B)/t_altstack: t_altstack.c t_altstack_stub.c ../crash.c ../dm.h $(GAME_H) | $(B)
 	$(CLANG) -O1 -Wno-unused-function -I.. $(GINC) -o $@ t_altstack.c t_altstack_stub.c ../crash.c
 $(B)/t_pctl: t_pctl.c ../hle_misc.c ../hle.h ../dm.h $(GAME_H) | $(B)

dm/host/tests/t_watch_sample.c

@@ -0,0 +1,170 @@
+// Run the real watchdog in a child; replace only the external sampler with this
+// executable in recorder mode. No sample process attaches to another application.
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <signal.h>
+#include <spawn.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+enum fault { NONE, INIT, OPEN, DUP, SPAWN, INTERRUPT, WAIT, EXIT_BAD, SIGNAL_BAD };
+static enum fault fault;
+static int actions_live, interrupted;
+
+static int sample_init(posix_spawn_file_actions_t *a) {
+    if (fault == INIT) return ENOMEM;
+    int rc = posix_spawn_file_actions_init(a);
+    if (!rc) actions_live++;
+    return rc;
+}
+static int sample_open(posix_spawn_file_actions_t *a, int fd, const char *p, int flags, mode_t mode) {
+    if (fault == OPEN) return ENOMEM;
+    return posix_spawn_file_actions_addopen(a, fd, p, flags, mode);
+}
+static int sample_dup(posix_spawn_file_actions_t *a, int from, int to) {
+    if (fault == DUP) return ENOMEM;
+    return posix_spawn_file_actions_adddup2(a, from, to);
+}
+static int sample_destroy(posix_spawn_file_actions_t *a) {
+    assert(actions_live == 1);
+    actions_live--;
+    return posix_spawn_file_actions_destroy(a);
+}
+static int sample_spawn(pid_t *pid, const char *file, const posix_spawn_file_actions_t *a,
+                        const posix_spawnattr_t *attr, char *const argv[], char *const env[]) {
+    assert(!strcmp(file, "/usr/bin/sample"));
+    assert(!strcmp(argv[0], "/usr/bin/sample"));
+    if (fault == SPAWN) return ENOENT;
+    return posix_spawn(pid, "./sampler", a, attr, argv, env);
+}
+static pid_t sample_wait(pid_t pid, int *status, int flags) {
+    if (fault == INTERRUPT && !interrupted++) { errno = EINTR; return -1; }
+    pid_t rc = waitpid(pid, status, flags);
+    // Reap the real fixture before simulating a failed wait, so no child is left.
+    if (fault == WAIT) { errno = ECHILD; return -1; }
+    return rc;
+}
+static _Noreturn void sample_exit(int status) {
+    assert(actions_live == 0);
+    _exit(status);
+}
+#define posix_spawn_file_actions_init sample_init
+#define posix_spawn_file_actions_addopen sample_open
+#define posix_spawn_file_actions_adddup2 sample_dup
+#define posix_spawn_file_actions_destroy sample_destroy
+#define posix_spawn sample_spawn
+#define waitpid sample_wait
+#define _exit sample_exit
+// A regression to a shell must fail to compile, not execute the fixture's paths.
+#pragma GCC poison system
+#include "../crash.c"
+#undef posix_spawn_file_actions_init
+#undef posix_spawn_file_actions_addopen
+#undef posix_spawn_file_actions_adddup2
+#undef posix_spawn_file_actions_destroy
+#undef posix_spawn
+#undef waitpid
+#undef _exit
+
+module_t *g_mod;
+int g_nmod;
+module_t *module_of(const void *p) { (void)p; return NULL; }
+
+static int recorder(int argc, char **argv) {
+    FILE *f = fopen("arguments.bin", "wb");
+    assert(f);
+    for (int i = 0; i < argc; i++) assert(fwrite(argv[i], 1, strlen(argv[i]) + 1, f) == strlen(argv[i]) + 1);
+    assert(!fclose(f));
+    puts("SAMPLER_STDOUT");
+    fputs("SAMPLER_STDERR\n", stderr);
+    const char *injected = getenv("WATCH_TEST_FAULT");
+    if (injected && !strcmp(injected, "exit")) return 7;
+    if (injected && !strcmp(injected, "signal")) raise(SIGTERM);
+    return 0;
+}
+static int checks, failures;
+#define CHECK(cond, name) do { checks++; if (!(cond)) { failures++; fprintf(stderr, "FAIL: %s\n", name); } } while (0)
+
+static void run_case(const char *path, enum fault inject, const char *name) {
+    assert(unlink("arguments.bin") == 0 || errno == ENOENT);
+    assert(unlink("injected-marker") == 0 || errno == ENOENT);
+    int logs[2]; assert(!pipe(logs));
+    pid_t child = fork(); assert(child >= 0);
+    if (!child) {
+        close(logs[0]);
+        assert(dup2(logs[1], STDOUT_FILENO) == STDOUT_FILENO);
+        assert(dup2(logs[1], STDERR_FILENO) == STDERR_FILENO);
+        close(logs[1]);
+        fault = inject;
+        if (path) assert(!setenv("DM_WATCH_SAMPLE", path, 1));
+        else assert(!unsetenv("DM_WATCH_SAMPLE"));
+        // Normal sampling also covers a recorder launched without fault controls.
+        if (inject == NONE) assert(!unsetenv("WATCH_TEST_FAULT"));
+        else assert(!setenv("WATCH_TEST_FAULT", inject == EXIT_BAD ? "exit" : inject == SIGNAL_BAD ? "signal" : "none", 1));
+        g_watch_stall = 1;
+        atomic_store(&g_watch_last_ns, 0);
+        watchdog_main(NULL);
+        _exit(99);
+    }
+    close(logs[1]);
+    char output[16384]; size_t used = 0; ssize_t n;
+    while ((n = read(logs[0], output + used, sizeof output - 1 - used)) > 0) used += (size_t)n;
+    assert(n == 0); output[used] = 0; close(logs[0]);
+    int status; assert(waitpid(child, &status, 0) == child);
+    CHECK(WIFEXITED(status) && WEXITSTATUS(status) == 128 + SIGALRM, "watchdog retains its exit status and frees spawn actions");
+    CHECK(!strstr(output, "SAMPLER_STDOUT") && !strstr(output, "SAMPLER_STDERR"), "sampler stdout and stderr stay redirected");
+    int succeeds = path && (inject == NONE || inject == INTERRUPT);
+    CHECK((strstr(output, "thread stacks written to") != NULL) == succeeds, name);
+    CHECK((strstr(output, "sampling failed") != NULL) == (path && !succeeds), "sampling failures are reported");
+    CHECK(access("injected-marker", F_OK) == -1 && errno == ENOENT, "output path cannot execute shell syntax");
+    FILE *f = fopen("arguments.bin", "rb");
+    int launched = path && (inject == NONE || inject >= INTERRUPT);
+    CHECK((f != NULL) == launched, "sampler launches only after setup succeeds");
+    if (f) {
+        char args[8192]; size_t size = fread(args, 1, sizeof args, f); assert(!ferror(f)); fclose(f);
+        char pid[32]; snprintf(pid, sizeof pid, "%d", child);
+        const char *expected[] = {NULL, pid, "1", "-f", path};
+        size_t offset = 0;
+        for (int i = 0; i < 5; i++) {
+            size_t end = offset;
+            while (end < size && args[end]) end++;
+            CHECK(end < size && (!expected[i] || !strcmp(args + offset, expected[i])), "sampler receives the exact PID, duration, flag and path");
+            offset = end + 1;
+        }
+        CHECK(offset == size, "sampler receives exactly five arguments");
+    }
+    printf("%s\n", name);
+}
+int main(int argc, char **argv) {
+    if (argc > 1) return recorder(argc, argv);
+    char executable[PATH_MAX]; assert(realpath(argv[0], executable));
+    char directory[] = "/tmp/dm-watch-sample-XXXXXX";
+    assert(mkdtemp(directory)); assert(!chdir(directory));
+    assert(!symlink(executable, "sampler"));
+    run_case(NULL, NONE, "unset output path skips sampling");
+    run_case("", NONE, "empty output path is still passed");
+    run_case("/tmp/ordinary sample.txt", NONE, "spaces in an output path");
+    run_case("/tmp/John's Mac/hang.txt", NONE, "apostrophe in an output path");
+    run_case("sample'; printf injected > injected-marker; #", NONE, "shell syntax is a literal output path");
+    run_case("/tmp/line\n\"$(`echo bad`)\\*?;\xc3\xa9.txt", NONE, "newlines, metacharacters and UTF-8 path bytes");
+    char long_path[1601]; memset(long_path, 'x', sizeof long_path - 1); long_path[sizeof long_path - 1] = 0;
+    run_case(long_path, NONE, "output arguments are never truncated");
+    run_case("result", INIT, "spawn action allocation failure");
+    run_case("result", OPEN, "stdout redirection setup failure");
+    run_case("result", DUP, "stderr redirection setup failure");
+    run_case("result", SPAWN, "sampler launch failure");
+    run_case("result", INTERRUPT, "interrupted wait resumes until sampler completion");
+    run_case("result", WAIT, "wait failure is reported");
+    run_case("result", EXIT_BAD, "nonzero sampler status is reported");
+    run_case("result", SIGNAL_BAD, "sampler signal is reported");
+    assert(unlink("arguments.bin") == 0 || errno == ENOENT);
+    assert(unlink("injected-marker") == 0 || errno == ENOENT);
+    assert(!unlink("sampler")); assert(!chdir("/")); assert(!rmdir(directory));
+    printf("watchdog sample: %d checks, %d failures\n", checks, failures);
+    return failures != 0;
+}

dm/tools/lint_c_baseline.tsv

@@ -105,7 +105,6 @@ dm/gfx/vk/hle_vk.m	clang-analyzer-core.uninitialized.Assign	1
 dm/gfx/vk/hle_vk.m	clang-analyzer-security.ArrayBound	2
 dm/gfx/vk/hle_vk.m	clang-analyzer-unix.Malloc	1
 dm/gfx/vk/hle_vk.m	misc-misplaced-const	5
-dm/host/crash.c	bugprone-command-processor	1
 dm/host/haptics_apple.m	clang-analyzer-nullability.NullablePassedToNonnull	1
 dm/host/hle_fs.c	bugprone-suspicious-realloc-usage	4
 dm/host/hle_fs.c	clang-analyzer-unix.Errno	2

docs/agents/symptoms.md

@@ -134,6 +134,7 @@ One line per symptom in the [subsystem references](../reference/): what you see
 - the picture stops and stays (sound goes on) at random places, about one run in two: every task worker waits on its own event, and the task a waiter waits for counts a subtask that is marked queued but is in no queue (the ring's head equals its tail). → [Lock-free queues published without barriers](../reference/threads.md#lock-free-queues-published-without-barriers)
 - the game freezes when a movie starts playing beside the game (a guide movie in a corner of the screen): the last frame stays, the process idles at a few percent of a core, and the thread dump shows a lower-priority job thread `running/not waiting` with its backtrace in the movie player (`ReleaseOutputBufferIndex`), the main thread waiting for it, and the movie's input thread `running/not waiting` too. A `sample` of the process shows the job thread in `md_pump` → `md_event` → `dm_yield_to_core_peers`, and the input thread in `h_movie_SendIn` → `md_pump` waiting for a pthread mutex. → [A host lock held across a same-core handoff](../reference/threads.md#a-host-lock-held-across-a-same-core-handoff)
 - several threads crash at once and the dumps interleave, then the process hangs instead of exiting. → [Simultaneous crashes of several threads](../reference/threads.md#simultaneous-crashes-of-several-threads)
+- `watchdog: sampling failed` and no thread stacks when `DM_WATCH_SAMPLE` contains an apostrophe, such as `/tmp/John's Mac/hang.txt`; shell syntax in the path can also run commands, and long paths are truncated. → [Watchdog sample output paths are literal arguments](../reference/threads.md#watchdog-sample-output-paths-are-literal-arguments)
 - none seen; found while investigating Fire Emblem Engage's demo crash, which it did not change (that was the address-space shortage, Size-aligned regions exhaust the alias address space). → [`SignalToAddress` type 2 with count ≤ 0](../reference/threads.md#signaltoaddress-type-2-with-count--0)
 - a 64-bit wait could return at once or block when it should not (not met in a game). → [`WaitForAddress` type 3: 64-bit wait if equal](../reference/threads.md#waitforaddress-type-3-64-bit-wait-if-equal)
 - boot stops before or soon after the first frame with the CPU at 0% and every guest thread waiting on a condition variable: the main thread in `nn::os::AcquireSemaphore` after posting a command to `UnityGfxDeviceWorker`, the worker in `AcquireSemaphore` too, one condition wait after it (`wait #` one higher); 3 of 5 Mac runs, at random points. → [Unity's stream buffer handshake loses a wake-up on Apple cores](../reference/threads.md#unitys-stream-buffer-handshake-loses-a-wake-up-on-apple-cores)

docs/reference/threads.md

@@ -102,6 +102,13 @@ Guest threads, their synchronization and how the host schedules them. Each entry
 - **Fix:** `crash.c` lets only the first crashing thread print (`crash_gate`); the others stop in `pause()`, and a 15 s alarm ends the process if the dump itself stalls.
 - **Found on:** Unity titles in general.
 
+### Watchdog sample output paths are literal arguments
+
+- **Symptom:** `watchdog: sampling failed` and no thread stacks when `DM_WATCH_SAMPLE` contains an apostrophe, such as `/tmp/John's Mac/hang.txt`; shell syntax in the path can also run commands, and long paths are truncated.
+- **Cause:** `crash.c` interpolated the path into a single-quoted command in a fixed-size buffer, then called `system()`. An apostrophe ends the shell quote.
+- **Fix:** spawn `/usr/bin/sample` directly with the PID, one-second duration, `-f` and the unchanged path as separate arguments. Spawn file actions redirect stdout and stderr to `/dev/null`. Setup, spawn, wait, nonzero exit and signal failures retain the failure log; an interrupted wait resumes. The watchdog still exits with `128 + SIGALRM` after sampling.
+- **Found on:** the [static-analysis audit in issue #313](https://github.com/seedds/nxnative/issues/313#issuecomment-6543080671), reproduced with synthetic paths; no game-specific failure is claimed.
+
 ## Synchronization and address arbitration
 
 ### `SignalToAddress` type 2 with count ≤ 0