diff --git a/dm/host/crash.c b/dm/host/crash.c index 026e4d771c..21e7a3c3e0 100644 --- a/dm/host/crash.c +++ b/dm/host/crash.c @@ -11,6 +11,12 @@ #include #include #include +#if TARGET_OS_OSX +#include +#include +#include +#include +#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); } diff --git a/dm/host/tests/Makefile b/dm/host/tests/Makefile index fe5028031e..01f79c3d4d 100644 --- a/dm/host/tests/Makefile +++ b/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) diff --git a/dm/host/tests/t_watch_sample.c b/dm/host/tests/t_watch_sample.c new file mode 100644 index 0000000000..64a0520d21 --- /dev/null +++ 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +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; +} diff --git a/dm/tools/lint_c_baseline.tsv b/dm/tools/lint_c_baseline.tsv index 18f9f68e31..13e2988b67 100644 --- a/dm/tools/lint_c_baseline.tsv +++ b/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 diff --git a/docs/agents/symptoms.md b/docs/agents/symptoms.md index 2145d24fa9..733c5f4bd8 100644 --- a/docs/agents/symptoms.md +++ b/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) diff --git a/docs/reference/threads.md b/docs/reference/threads.md index 01b73017e5..bd7f50e689 100644 --- a/docs/reference/threads.md +++ b/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