casaroli opened a new pull request, #19562: URL: https://github.com/apache/nuttx/pull/19562
*Note: Please adhere to [Contributing Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).* ## Summary This implements step 1 of the plan agreed in **#19540** ([ordering](https://github.com/apache/nuttx/issues/19540#issuecomment-5092600519), [go-ahead](https://github.com/apache/nuttx/issues/19540#issuecomment-5093571881)): the core semantics, plus the `arch/Kconfig` change that withdraws `fork()` from every architecture so that per-architecture patches can restore it, one at a time, with real POSIX semantics. **Companion PR:** apache/nuttx-apps#3673. **It must merge first.** It is written to work against NuttX with *or* without this change, so no `ostest` coverage is lost across the transition. ### The problem NuttX implements `fork()` and `vfork()` as the same function. Both are libc wrappers around a single `up_fork()`; `vfork()` differs only by a trailing `waitpid()`. Underneath, the child joins the parent's address environment — the same `addrenv_join()` that `pthread_create()` uses — and gets a private copy of the stack. The child therefore shares `.data`, `.bss` and the heap with its parent and runs concurrently with it. That is not `fork()`. It is `vfork()`-with-a-private-stack published under `fork()`'s name, and the history says so: today's `fork()` is NuttX's old `vfork()`, renamed in [`c33d1c9c97`](https://github.com/apache/nuttx/commit/c33d1c9c977680c5c1f5214c260269b782fdff09) (2023) with no change of behaviour. NuttX's own comment on `nxtask_setup_fork()` documents `fork()` by quoting the POSIX definition of `vfork()`, word for word. The failure mode is the worst one available: **silent**. A program written against POSIX `fork()` compiles, links, runs — and has its child's writes land in the parent's variables. No diagnostic, no error. ### What this does Three primitives, chosen by which function the caller called rather than by what the hardware happens to be: | | memory | parent | Kconfig | |---|---|---|---| | `fork()` | child gets **its own copy** at the same virtual addresses | runs concurrently | `ARCH_HAVE_FORK` | | `vfork()` | child **shares** the parent's memory | **suspended** until `_exit()`/`exec()` | `ARCH_HAVE_VFORK` | | `task_fork()` | child shares memory, private stack copy | runs concurrently | `ARCH_HAVE_TASK_FORK` | Below libc there are now three syscalls — `up_task_fork()`, `up_vfork()` and `up_fork()`. The per-architecture register snapshot is common to all three: each architecture's three entry points share one sequence and differ only in a `FORK_TYPE_*` selector (`include/nuttx/fork.h`) handed to `nxtask_setup_fork()`, which is the single place the memory semantics are decided. Per architecture the plumbing is +5…+42 lines; nothing is rewritten. **`task_fork()` is today's behaviour under an honest name.** Not `fork()`, not `vfork()`: a task cloned at the call site with the memory relationship of a thread. The nearest precedent is Plan 9's `rfork(RFPROC|RFMEM)` — data and bss shared, stack copied. The name deliberately avoids "vfork", because the defining property of `vfork()` is the suspension, which this primitive does not have; something like `stack_vfork()` would recreate the very confusion this removes. **The `vfork()` parent suspension moves out of libc into the kernel** — `nxtask_start_vfork()`, released from `nxsched_release_tcb()`. Two things follow. The parent is now resumed at `exec()` as POSIX requires, because `exec_swap()` has already handed the child's pid to the loaded program by the time the `vfork` stub exits. And `vfork()` no longer depends on `CONFIG_SCHED_WAITPID` — until now, a configuration without that option had no `vfork()` at all. That release point needs one fix in `nxtask_exit()`. It raises `rtcb->lockcount` directly rather than through `sched_lock()` while it tears the TCB down, so the `nxsem_post()` that wakes the `vfork()` parent queues it on `g_pendingtasks` — and the matching raw `lockcount--` does *not* merge that list the way `sched_unlock()` would. The parent is left stranded with nothing to move it off. A `nxsched_merge_pending()` after the decrement publishes it; the call is a no-op while pre-emption is still disabled, and `up_exit()` re-reads `this_task()` afterwards. Without it, `vfork()` **deadlocks** on `rv-virt:nsh64` and `rv-virt:pnsh64`, where NSH is blocked in `waitpid()` holding the lock and nothing else calls `sched_unlock()`. **`fork()` is built on a new `addrenv_fork()`**, backed by an `up_addrenv_fork()` hook that duplicates an address environment into freshly allocated pages mapped at the same virtual addresses — unlike `up_addrenv_clone()`, which copies only the representation and leaves both pointing at the same page tables. The child then adopts the parent's stack geometry rather than being given a relocated copy: a pointer to a stack local taken before `fork()` must name the same object in the child that it named in the parent, and the parent's stack is already in the duplicate, at the parent's address, with its contents. Nothing is allocated and nothing is copied for it. There is no copy-on-write — NuttX has no demand paging to build it on — so the copy is eager and can fail with `ENOMEM`. That is the nature of the primitive, not a defect: `fork()`'s value here is correctness for portable code. Spawn-heavy code should prefer `posix_spawn()` or `vfork()`, on NuttX as anywhere. ### Why `fork()` ends up unavailable everywhere No architecture implements `up_addrenv_fork()` in this PR, so `CONFIG_ARCH_HAVE_FORK` is unset in every configuration and `fork()` is not declared. That is deliberate, and it is step 3 of the agreed ordering folded into step 1 — the two cannot be cleanly separated, because step 1 is what redefines `CONFIG_ARCH_HAVE_FORK` to mean "can provide POSIX semantics", and splitting them would require an intermediate release in which the symbol means two things at once. Per-architecture PRs restore `fork()` as `up_addrenv_fork()` lands. The generic machinery is complete and needs no further work: an architecture implements the hook, adds one `default y if` line, and `fork()` becomes live. ## Impact **This is a breaking change, and it breaks loudly rather than quietly.** **`fork()` disappears from `ARCH_ARM`, flat `ARCH_ARM64`, `ARCH_RISCV`, `ARCH_SIM` and `ARCH_X86_64`.** Code that calls it fails to *build*, with an error naming the function. That is the point: a build error is strictly better than the silent runtime wrongness it replaces, and it is the only signal portable code can act on — there is no feature-test macro or `sysconf()` query by which an application can discover that a `fork()` does not copy. **`CONFIG_FORK_IS_TASK_FORK=y` restores the previous behaviour exactly** — same sharing, same concurrency, no new suspension — on precisely the configurations that had `fork()` before. `default n`, so the honest behaviour is what you get unless you ask otherwise, and the Kconfig help states plainly what you are opting into. Existing users flip one switch and are where they were. If withdrawing `fork()` in a single release is judged too abrupt, the switch can ship `default y` with a deprecation warning for a cycle. The end state should still be `default n`; the schedule is negotiable, the destination shouldn't be. **`vfork()` changes behaviour.** The parent is now suspended in the kernel and released when the child's TCB is torn down, so by the time it runs the child is completely gone. Where the child called `exec()` this makes no difference: `exec_swap()` has already given the loaded program the child's pid and that program is still running, so `waitpid()` behaves normally. Where the child called `_exit()`, `waitpid()` can only return its status if `CONFIG_SCHED_CHILD_STATUS` is enabled; otherwise it returns `ECHILD`. That is a pre-existing property of that configuration rather than a change — the previous implementation blocked in a libc `waitpid(WNOWAIT)` and an application's own `waitpid()` afterwards hit the same wall. **`vfork()` gains availability**: it no longer depends on `CONFIG_SCHED_WAITPID`. **Nothing is deleted.** Every line of the existing machinery survives as `task_fork()`. **`apps`:** companion PR, must merge first. It is deliberately tolerant of both worlds, so `ostest` keeps running `task_fork_test` and `vfork_test` against NuttX master today and against this PR tomorrow. A small follow-up drops the compatibility fallbacks afterwards. **Depends on #19544** (`sched/addrenv: do not dereference a NULL address environment`) for `BUILD_PROTECTED` configurations with `CONFIG_ARCH_ADDRENV`. That is a pre-existing bug — it hits `pthread_create()` there too — and is up for review separately. This branch will be rebased once it lands. **Documentation:** `Documentation/guides/fork_vfork_migration.rst` is new and answers "which replacement do I want?" from the reader's own reason for having called `fork()`. `reference/user/01_task_control.rst` gains `fork()` and `task_fork()` entries and rewrites `vfork()`'s. `standards/posix.rst` moves `fork()` from "No" to "Cond." and `vfork()` from "Yes" to "Cond.". **Unrelated finding, reported separately:** `arm_fork()` copies `xcp.syscall[].sysreturn` and `.excreturn` to the child but not `.ctrlreturn`. The child's TCB is `kmm_zalloc()`ed, so `CONTROL == 0` — nPRIV clear — and in `BUILD_PROTECTED` a Cortex-M child returns to user space **privileged** while its parent is unprivileged. Measured on an RP2350 (Cortex-M33): parent `ctrlreturn` `0x00000001`, child `0x00000000`. It applies to master unchanged and has nothing to do with this change, so it is filed on its own. ## Testing Host: macOS 15 (Darwin 25.5.0) on Apple Silicon. Toolchains: xPack `riscv-none-elf-gcc` 14.2.0-3; Arm GNU `arm-none-eabi-gcc` and `aarch64-none-elf-gcc` 14.2.Rel1 (darwin-arm64); Homebrew `x86_64-elf-gcc`; QEMU 11.0.3. `apps` at the companion PR's branch throughout. ### Run under QEMU — full `ostest` suite, exit status 0 on every one | config | build model | `task_fork()` | `vfork()` | `fork()` | `ostest` | |---|---|---|---|---|---| | `rv-virt:nsh64` | FLAT | **PASS** | **PASS** | absent ✓ | **status 0** | | `rv-virt:pnsh64` | PROTECTED | **PASS** | **PASS** | absent ✓ | **status 0** | | `rv-virt:knsh64` | KERNEL + `ARCH_ADDRENV` | **PASS** | **PASS** | absent ✓ | **status 0** | | `qemu-armv7a:nsh` | FLAT | **PASS** | **PASS** | absent ✓ | **status 0** | | `qemu-armv8a:nsh` | FLAT | **PASS** | **PASS** | absent ✓ | **status 0** | | `qemu-intel64:nsh` | FLAT | **PASS** | **PASS** | absent ✓ | **status 0** | ``` user_main: task_fork() test task_fork_test: Started task_fork_test: Child 6 ran successfully user_main: vfork() test vfork_test: Started vfork_test: Child 7 ran and exited before the parent resumed ... ostest_main: Exiting with status 0 ``` `rv-virt:knsh64` is the interesting row: it is a `BUILD_KERNEL` + `CONFIG_ARCH_ADDRENV` configuration, and it is where `fork()` visibly goes away. `vfork()` there also exercises the kernel-side suspension across a system call. "absent" was checked in the linked image, not merely inferred from the config. `nm` shows `up_task_fork`, `up_vfork`, `task_fork` and `vfork`, and **no** `fork`, `up_fork` or `fork_test`. For example, `rv-virt:nsh64`: ``` 0000000080000424 T up_task_fork 0000000080023eda T task_fork 0000000080000428 T up_vfork 0000000080023ede T vfork 0000000080002786 T nxtask_setup_fork 000000008000290c T nxtask_start_vfork ``` ### Built clean `stm32f4discovery:nsh` (armv7-m), `mps2-an521:nsh` (armv8-m), `sabre-6quad:nsh` (armv7-a), `sim:ostest` — the last with `CONFIG_MM_KASAN` disabled, see the note below. On `sim`, `task_fork_test` and `vfork_test` also pass at run time. ### The break, and the escape hatch, both verified Without `CONFIG_FORK_IS_TASK_FORK` (`qemu-armv8a:nsh`), a call to `fork()` is a build error naming the function, while the honest spellings still compile: ``` forkchk.c:3:28: error: implicit declaration of function 'fork' 3 | pid_t probe(void) { return fork(); } | ^~~~ ``` ```c pid_t a(void) { return vfork(); } /* compiles */ pid_t b(void) { return task_fork(); } /* compiles */ ``` With `CONFIG_FORK_IS_TASK_FORK=y` (`rv-virt:nsh64`), the same translation unit compiles, `unistd.h` declares `fork()` again, and `staging/libc.a` provides it: ``` 0000000000000000 T fork 0000000000000000 T task_fork 0000000000000000 T vfork ``` `fork_test` correctly stays out of that build: the alias sets `FORK_IS_TASK_FORK`, not `ARCH_HAVE_FORK`, so no POSIX `fork()` test is advertised for a configuration that does not have POSIX `fork()`. ### Style and docs `tools/checkpatch.sh -g origin/master..HEAD` — **✔️ All checks pass**, including `cmake-format` and `nxstyle` on every touched file. `sphinx-build -b html` over `Documentation/` — **build succeeded, 1 warning**, and that warning is the pre-existing `plantuml command cannot be run` in `guides/port_bootsequence.rst`, unrelated to this change. ### Note on two pre-existing failures met on the way Neither is caused by this change; both were worked around for the runs above. Reported here only so the workarounds in the numbers are not mistaken for something this PR needs. * `CONFIG_ALLSYMS=y` configurations fail to link from a clean tree: `LINK_ALLSYMS_KASAN` runs `tools/mkallsyms.py` on `$(NUTTX)` before anything has linked it, and the script exits 22 on the missing file. **Confirmed to reproduce on unmodified `master` (`7fd17c9d7c`)** with the same host and toolchain — `qemu-armv7a:nsh` fails identically there. Worked around by clearing `CONFIG_ALLSYMS`. * `sim:ostest` does not build on macOS/arm64: `CONFIG_MM_KASAN` passes `-fsanitize=kernel-address`, which Apple clang does not support for `arm64-apple-darwin`. Worked around by clearing the sanitizer options. This is a host-toolchain limitation, not a NuttX one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
