diff --git a/.claude/memory/schos-smp-userspace-corruption.md b/.claude/memory/schos-smp-userspace-corruption.md index eba2676..7979f0d 100644 --- a/.claude/memory/schos-smp-userspace-corruption.md +++ b/.claude/memory/schos-smp-userspace-corruption.md @@ -85,3 +85,24 @@ Rebuilding between runs dominates the clock. Build both variants once, save restore-and-boot. Faults land during boot (six tty shells exec'ing at once), so a 40-second timeout is enough — no typed input needed. `-smp 4` roughly doubles the rate over `-smp 2`. + + +## Found while here: `$(command substitution)` hangs every shell + +Unrelated to the above and **still open**. `X=$(echo hi)` never returns, in zsh +*and* mksh, on 1 CPU and on 4, interactive and not, with `set +m`. Pipelines, +subshells, background jobs and foreground externals are all fine. + +Ruled out: `setpgid` (real now), SMP, job control, zsh specifically, and the +kernel's SIGCHLD/sigsuspend contract — `cmd/sigtest` proves that one and passes +all eight checks. A full syscall trace of a hang has every return value +correct. + +The contradiction is at `Src/jobs.c:1569`: zsh's `waitforpid()` ends its loop +on `ESRCH` from `kill(pid, 0)`, and the trace shows that `kill` returning ESRCH +with the loop entered anyway. Next step is a musl-linked probe that does +`fork; let the handler reap; kill(pid,0); print errno` — everything upstream of +that line is known good. + +Documented in full at `docs/history/kernel.md` section 7, which is where to +start. diff --git a/Makefile b/Makefile index 2cbe9a0..fcb116d 100644 --- a/Makefile +++ b/Makefile @@ -108,7 +108,7 @@ KERN_NOENTRY = $(filter-out $(BUILD)/kernel/arch/x86_64/entry.o, $(KERNOBJS)) # Third-party software does NOT go here — see ports/. # ----------------------------------------------------------------------- ULIB = $(addprefix $(BUILD)/lib/, start.o glue.o usys.o) -CMDS = schinit schinitd fbinfo shmtest socktest inputtest nettest reflink schattr ping chvt loadkeys pcitest vgpud schdbg schdbgd resolvconf httpd schmode netecho ldlsd +CMDS = schinit schinitd fbinfo shmtest socktest inputtest nettest reflink schattr ping chvt loadkeys sigtest pcitest vgpud schdbg schdbgd resolvconf httpd schmode netecho ldlsd CMDBINS = $(addprefix $(BUILD)/cmd/, $(CMDS)) # ----------------------------------------------------------------------- diff --git a/cmd/sigtest/main.c b/cmd/sigtest/main.c new file mode 100644 index 0000000..03d6aa8 --- /dev/null +++ b/cmd/sigtest/main.c @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// sigtest -- the SIGCHLD/sigsuspend contract every shell is built on. +// +// This is not one shell's quirk. It is what POSIX says to do and what zsh, +// mksh, bash and dash all do to wait for a child: +// +// sigprocmask(SIG_BLOCK, {SIGCHLD}, &old); // close the window +// pid = fork(); // child may exit at once +// while (!flag) // flag set by the handler +// sigsuspend(&old_without_SIGCHLD); // unblock and wait, atomically +// sigprocmask(SIG_SETMASK, &old, 0); +// +// Blocking first is the whole point: the child can exit *before* the parent +// reaches sigsuspend. Blocked, the signal stays pending; sigsuspend then +// unblocks and waits in one step, so a pending signal is delivered at once and +// the wait returns immediately. Get that wrong in either direction and the +// parent waits for a signal that has already happened -- which is a hang, in +// every shell, on every `$(command substitution)`. +// +// Written against the raw syscalls rather than a libc, because the thing under +// test is the kernel's half of the contract. **The user-facing sigset has +// signal N at bit N-1** (Linux's layout); the kernel converts at the boundary. +// Getting that backwards is a real bug this tree has had, so the masks below +// are spelled out rather than built by a macro. + +#include "types.h" +#include "user.h" +#include "posix.h" + +#define U_SIGCHLD_BIT (1ULL << (SIGCHLD - 1)) + +// rt_sigaction takes a pointer to Linux's struct sigaction, not a function +// pointer: the handler is at offset 0 and the restorer at 144. The kernel +// reads exactly those two words (SA_HANDLER_OFF / SA_RESTORER_OFF in +// kernel/sys/sys_sig.c), so a 152-byte zeroed block with the handler written +// into the first word is the whole of what it needs. +#define SA_SIZE 152 + +static int failures; +static volatile int handler_runs; + +static void +puts_(const char *s) +{ + int n = 0; + while (s[n]) n++; + write(1, (void *)s, n); +} + +static void +check(const char *what, int ok) +{ + puts_(ok ? "[ok] " : "[FAIL] "); + puts_(what); + puts_("\n"); + if (!ok) failures++; +} + +static void +on_chld(int sig) +{ + (void)sig; + handler_runs++; +} + +// A bounded wait. A wrong answer here is a hang, so nothing may wait for ever: +// sigsuspend returns -EINTR each time a signal is delivered, and a kernel that +// never delivers spins out instead of stopping the test dead. +#define SPINS 300 + +int +main(void) +{ + uint64 blocked, unblocked, old = 0; + int pid, status, i; + + puts_("sigtest: the SIGCHLD/sigsuspend contract\n\n"); + + { + static char act[SA_SIZE]; + *(uint64 *)act = (uint64)(void *)on_chld; + rt_sigaction(SIGCHLD, act, 0, 8); + } + + // --- 1. blocking actually blocks -------------------------------------- + blocked = U_SIGCHLD_BIT; + sigprocmask(SIG_BLOCK, (const sigset_t *)&blocked, (sigset_t *)&old); + + handler_runs = 0; + pid = fork(); + if (pid == 0) + exit_group(0); + + // Every chance for the child to exit and for the kernel to deliver + // wrongly. If the mask is honoured, the handler has not run. + for (i = 0; i < 50; i++) + getpid(); + sleep(10); + check("SIGCHLD stays pending while blocked", handler_runs == 0); + + // --- 2. sigsuspend delivers what is already pending -------------------- + // The case that matters, and the one that hangs: the child is already + // gone, so nothing new will ever arrive. sigsuspend has to deliver what is + // *pending*, not wait for the next one. + unblocked = ~U_SIGCHLD_BIT; + for (i = 0; i < SPINS && handler_runs == 0; i++) + rt_sigsuspend(&unblocked); + + check("sigsuspend delivers a signal that was already pending", + handler_runs > 0); + check("and it did not have to spin to get it", i <= 1); + check("the handler ran exactly once", handler_runs == 1); + + // --- 3. the mask survives ---------------------------------------------- + { + uint64 now = 0; + sigprocmask(SIG_BLOCK, 0, (sigset_t *)&now); + check("sigsuspend restores the mask it was called with", + (now & U_SIGCHLD_BIT) != 0); + } + + sigprocmask(SIG_SETMASK, (const sigset_t *)&old, 0); + + // --- 4. the child is still reapable ------------------------------------ + status = -1; + check("the child can still be reaped after the handler ran", + waitpid(pid, &status, 0) == pid); + check("and its status says it exited cleanly", status == 0); + + // --- 5. the other order: the signal arrives while waiting --------------- + handler_runs = 0; + sigprocmask(SIG_BLOCK, (const sigset_t *)&blocked, (sigset_t *)&old); + pid = fork(); + if (pid == 0) { + sleep(30); // let the parent reach sigsuspend first + exit_group(0); + } + for (i = 0; i < SPINS && handler_runs == 0; i++) + rt_sigsuspend(&unblocked); + check("sigsuspend wakes for a signal that arrives while it waits", + handler_runs > 0); + sigprocmask(SIG_SETMASK, (const sigset_t *)&old, 0); + waitpid(pid, &status, 0); + + puts_("\n"); + puts_(failures ? "sigtest: FAILED\n" : "sigtest: all checks passed\n"); + return failures ? 1 : 0; +} diff --git a/docs/history/kernel.md b/docs/history/kernel.md index 2f8c5b9..0604597 100644 --- a/docs/history/kernel.md +++ b/docs/history/kernel.md @@ -432,6 +432,88 @@ likelihood: its own `job_list` (`ports/mksh (mksh R59c source): jobs.c:1372-1385`) silently fails to find the job for reasons not yet traced. +### 2026-09-21: narrowed a long way, still open + +Picked up again with the debugger that did not exist when the above was +written. What is now *ruled out* is most of what that note suspected: + +- **Not `setpgid`/`getpgid`.** Both are real now — `proc_setpgid`, `p->pgid`, + `p->sid` — and `waitpid()` honours the POSIX pid selection rules. +- **Not signal delivery, and not the sigsuspend contract.** `cmd/sigtest` + asserts the whole pattern every shell uses and passes all eight checks: a + blocked SIGCHLD stays pending, `sigsuspend` delivers an *already pending* + signal without spinning, the handler runs exactly once, the mask is restored, + and the child is still reapable afterwards. A permissive stub cannot pass it. +- **Not SMP.** It hangs identically on `-smp 1` and `-smp 4`. +- **Not job control.** `set +m` makes no difference, and a non-interactive + `zsh -c` hangs the same way. +- **Not zsh.** `mksh -c 'X=$(echo hi)'` hangs in the same place. +- **Not the values the kernel returns.** The full syscall trace of a hang is + below and every one of them is correct. + +The symptom is narrower than "job control": it is **`$(command +substitution)`**, and only that. Pipelines work. Subshells work. Background +jobs and `wait` work. Foreground external commands work. + +A complete trace of `X=$(echo inline)` in zsh, from the fork to the hang: + +``` +fork() -> 14 +sigprocmask x2 (child_block) +close(pipe write end) +sigprocmask(SIG_UNBLOCK, SIGCHLD) (child_unblock) + -> SIGCHLD delivered to zsh's handler + wait4(-1, WNOHANG|WUNTRACED|WCONTINUED) -> 14, status 0 + getrusage() -> 0 + wait4(same) -> -ECHILD + rt_sigreturn +read(pipe) -> 7 ("inline\n") +read(pipe) -> 0 (EOF) +sigprocmask, close +kill(14, 0) -> -ESRCH +rt_sigsuspend(mask=SIGINT) -> never returns +``` + +Everything there is right. The child is reaped with status 0, the pipe carries +its output and closes, and `kill` on the reaped pid correctly says ESRCH. + +**And that last pair is the contradiction.** zsh's `waitforpid()` +(`Src/jobs.c:1569`) is + +```c +while (!errflag && (kill(pid, 0) >= 0 || errno != ESRCH)) { + ... + signal_suspend(SIGCHLD, wait_cmd); +``` + +with a comment above it saying, in as many words, that ESRCH from that `kill` +is how the loop ends. The trace shows the `kill` returning ESRCH **and the +loop being entered anyway**, which leaves only two possibilities: `errno` is +not `ESRCH` when zsh reads it, or `kill()` did not return negative. errno is +demonstrably fine elsewhere — `kill -0 31000` prints "no such process" from +both zsh's builtin and toybox's `kill`, and `cat` of a missing file prints "No +such file or directory". + +That is where it stands. The next step is to distinguish those two: a musl- +linked probe that does exactly `fork; wait for the handler to reap; kill(pid, +0); print errno`, because everything upstream of that line is now known good. + +Two real bugs were found on the way and fixed, neither of them this one: + +- **The signal return trampoline was ABI-blind.** `deliver_signals()` wrote + `mov eax, 15 ; syscall` as the handler's return address. Fifteen is *Linux's* + `rt_sigreturn`, correct for anything musl-linked; for a first-party binary it + is native syscall 15, which is `open`. So a handler in any `cmd/` program + ran, returned into the trampoline, called `open()` with the handler's + leftover registers, got `-ENOENT`, and carried on executing the stack bytes + after the trampoline. It emits the right number for the process's ABI now. + This is why `cmd/sigtest` had to exist before anything else could be trusted. +- **`SIG_BLOCK`, `SIG_UNBLOCK` and `SIG_SETMASK` were not defined anywhere.** + The kernel branched on bare `0`, `1` and `2` with the names in comments, so + anything outside the kernel had to spell the number and hope. + +--- + This was left here deliberately rather than continuing to iterate blind: SIGCHLD delivery is left **enabled** (`kernel/proc/proc.c`'s `exit()`) since disabling it guarantees zero chance of mksh's wait loop ever exiting, diff --git a/docs/plan/1.0.md b/docs/plan/1.0.md index 25f0e58..66dfc54 100644 --- a/docs/plan/1.0.md +++ b/docs/plan/1.0.md @@ -206,6 +206,24 @@ right then. in 18 after.** See [../architecture/kernel.md](../architecture/kernel.md), "Per-CPU state". +- [ ] **`$(command substitution)` hangs every shell.** Not job control, not + SMP, not zsh — `mksh -c 'X=$(echo hi)'` hangs the same way, on one CPU, + with `set +m`. Pipelines, subshells, background jobs and foreground + externals all work; only `$( )` does not. + + Narrowed a long way this session and still open. The kernel's half of + the SIGCHLD/sigsuspend contract is now *proven* rather than assumed — + `cmd/sigtest` asserts the whole pattern and passes — and a full syscall + trace of a hang shows every return value correct. The contradiction is + at one line of zsh's `waitforpid()`, which ends its loop on `ESRCH` from + `kill(pid, 0)`; the trace shows that `kill` returning ESRCH and the loop + being entered regardless. See + [../history/kernel.md](../history/kernel.md), section 7. + + Two real bugs were found and fixed on the way: the signal-return + trampoline was hardcoded to Linux's `rt_sigreturn` number, so handlers + in first-party binaries called `open()` and then executed stack bytes; + and `SIG_BLOCK`/`SIG_UNBLOCK`/`SIG_SETMASK` were not defined anywhere. - [ ] **`getrusage` does not distinguish `RUSAGE_CHILDREN`** from `RUSAGE_SELF`. - [ ] **`/proc//exe` is a regular file, not a symlink**, so `readlink()` fails and `ps -o comm` prints `[init]`. Symlinks themselves now work — diff --git a/include/posix.h b/include/posix.h index fe4a4b8..8bd3458 100644 --- a/include/posix.h +++ b/include/posix.h @@ -347,6 +347,14 @@ struct statfs { #define SIGBUS 7 #define SIGFPE 8 #define SIGKILL 9 +// sigprocmask(2)'s `how`. Linux's values. The kernel has always implemented +// these -- sys_sigprocmask() branches on 0, 1 and 2 with the names in comments +// -- but never named them, so anything outside the kernel had to spell the +// number and hope it had the right one. +#define SIG_BLOCK 0 +#define SIG_UNBLOCK 1 +#define SIG_SETMASK 2 + #define SIGUSR1 10 #define SIGSEGV 11 #define SIGUSR2 12 diff --git a/include/user.h b/include/user.h index 42e7497..42ae7ea 100644 --- a/include/user.h +++ b/include/user.h @@ -113,6 +113,10 @@ int pipe2(int*, int); int clock_gettime(int, void*); int getppid(void); int rt_sigaction(int, void*, void*, uint64); +// The syscall, not glibc's wrapper: the argument is a pointer to one 64-bit +// sigset with signal N at bit N-1, which is Linux's layout and not the +// kernel's internal one. Returns -EINTR always, as sigsuspend must. +int rt_sigsuspend(const uint64 *mask); int mount(const char *src, const char *tgt, const char *fstype, uint64 flags, void *data); // Directory reading. The kernel has had getdents64 since the Linux ABI landed, diff --git a/kernel/sys/sys_sig.c b/kernel/sys/sys_sig.c index 0e2bba0..9745225 100644 --- a/kernel/sys/sys_sig.c +++ b/kernel/sys/sys_sig.c @@ -5,6 +5,7 @@ #include "syscall.h" #include "uaccess.h" #include "errno.h" +#include "posix.h" /* SIG_BLOCK, SIG_UNBLOCK, SIG_SETMASK, signal numbers */ // Linux sigset_t puts signal N at bit N-1; kernel-internal masks // (pending_signals / signal_mask / deliver_signals) put signal N at bit N. @@ -44,9 +45,9 @@ sys_sigprocmask(void) uint64 uset; if (copy_from_user(&uset, setp, sizeof(uset)) == 0) { uint64 set = sigset_from_user(uset); - if (how == 0) myproc()->signal_mask |= set; // SIG_BLOCK - else if (how == 1) myproc()->signal_mask &= ~set; // SIG_UNBLOCK - else myproc()->signal_mask = set; // SIG_SETMASK + if (how == SIG_BLOCK) myproc()->signal_mask |= set; + else if (how == SIG_UNBLOCK) myproc()->signal_mask &= ~set; + else myproc()->signal_mask = set; } } return 0; @@ -201,10 +202,26 @@ deliver_signals(struct trapframe *tf) uint64 *retaddr = (uint64*)uva2ka(myproc()->pgdir, (char*)sp); if (!retaddr) continue; - // mov eax, 15 ; syscall - tramp[0] = 0xb8; tramp[1] = 0x0f; tramp[2] = 0x00; - tramp[3] = 0x00; tramp[4] = 0x00; - tramp[5] = 0x0f; tramp[6] = 0x05; + // The return trampoline: `mov eax, ; syscall`. + // + // **Which number depends on the process's ABI**, and it used to be + // hardcoded to 15. Fifteen is Linux's rt_sigreturn, which is right for + // anything musl-linked -- every port, and the shells. It is *native* + // syscall 15 for a first-party binary, and native 15 is `open`. So a + // handler in any cmd/ program ran, returned into the trampoline, called + // open() with the handler's leftover registers as arguments, got + // -ENOENT, and carried on executing the bytes after the trampoline -- + // which are stack, not code. + // + // See table.inc: SYSCALL(97, 15, rt_sigreturn). syscall_map is the + // thing that says which numbering this process speaks. + uint32 sret = myproc()->syscall_map ? 15 : SYS_rt_sigreturn; + tramp[0] = 0xb8; // mov eax, imm32 + tramp[1] = (uint8)(sret ); + tramp[2] = (uint8)(sret >> 8); + tramp[3] = (uint8)(sret >> 16); + tramp[4] = (uint8)(sret >> 24); + tramp[5] = 0x0f; tramp[6] = 0x05; // syscall *retaddr = tramp_addr; myproc()->pending_signals &= ~(1ULL << sig); diff --git a/lib/usys.S b/lib/usys.S index ca0966e..e970b85 100644 --- a/lib/usys.S +++ b/lib/usys.S @@ -102,6 +102,7 @@ SYSCALL(pipe2) SYSCALL(clock_gettime) SYSCALL(getppid) SYSCALL(rt_sigaction) +SYSCALL(rt_sigsuspend) SYSCALL(shmget) SYSCALL(shmat) SYSCALL(shmdt)