From d87bd68e6b49b7f897eee2cae0cf7b86b97e8b72 Mon Sep 17 00:00:00 2001 From: Bernard Ladenthin Date: Sat, 1 Aug 2026 21:39:19 +0200 Subject: [PATCH] docs: record upstream status of patches 0001 and 0009 Refresh patch 0009 to match sheredom/subprocess.h#104 at 620ce44, which added the overridable SUBPROCESS_HAVE_CWD detection. Verified to apply cleanly to the vendored header and to reproduce that head byte for byte, so the applier will skip it once the pin is bumped. Add the upstream-facing write-up for patch 0001, including a reproducer that makes llama.cpp's own test-arg-parser fail on unmodified master. Assisted-by: Claude Opus 5 --- CLAUDE.md | 4 +- TODO.md | 7 +- ...m-investigation-win32-argv-substitution.md | 136 ++++++++++++++++++ ...bprocess-guard-addchdir-np-old-glibc.patch | 41 +++--- 4 files changed, 165 insertions(+), 23 deletions(-) create mode 100644 docs/upstream-investigation-win32-argv-substitution.md diff --git a/CLAUDE.md b/CLAUDE.md index e0b3b000d..998f0674b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -577,13 +577,13 @@ Current patches: | Patch | Fixes | |-------|-------| -| `0001-win32-arg-parse-embed-guard.patch` | Windows JNI regression from llama.cpp **#24779** (introduced b9739): on Windows `common_params_parse` re-derived argv from the **process** command line (`GetCommandLineW`) and adopted it, so an embedded/JNI caller (`java.exe`) lost its `--model …` args → "Failed to parse model parameters". b9789 narrowed the unconditional override to a **count-guard** (`if (static_cast(utf8.buf.size()) == argc) { argv = utf8.ptrs.data(); }`), but that is exactly the variant the project already found breaks its Windows server-integration tests (when the embedded argv length coincides with `java.exe`'s). The patch carries the **complete upstream change** (so it can be submitted to llama.cpp verbatim and then dropped here): **(1)** `common_params_parse` parses **exactly the argv it is given** (no `GetCommandLineW` magic) and a new `common_params_parse_main()` wrapper holds the UTF-8 recovery for the standalone tools' `main()` (`common/arg.{cpp,h}`); **(2)** the **~34 standalone `main()` call sites** (every `common_params_parse(argc, argv, …)` across `tools/*`, `examples/*` and the `tests/*` programs) flip to `common_params_parse_main()`; **(3)** a `tests/test-arg-parser.cpp` regression case pins that `common_params_parse` honors a caller-supplied argv. The embedded caller (`jllama.cpp`) keeps calling `common_params_parse` and is never overridden. **Our subproject build compiles only the `arg.{cpp,h}` core** — `LLAMA_BUILD_TOOLS`/`LLAMA_BUILD_TESTS` are OFF for a FetchContent subproject — so the flips + test are applied-but-not-compiled here; they were validated via a one-off `-DLLAMA_BUILD_TOOLS=ON -DLLAMA_BUILD_TESTS=ON` build (the new test compiles and its asserts pass; `test-arg-parser`'s only red there is the live `ggml.ai` download check, which is sandbox-network, not the patch). Because it spans **37 files** it must be refreshed on every llama.cpp bump (the applier fails loud). | +| `0001-win32-arg-parse-embed-guard.patch` | Windows JNI regression from llama.cpp **#24779** (introduced b9739): on Windows `common_params_parse` re-derived argv from the **process** command line (`GetCommandLineW`) and adopted it, so an embedded/JNI caller (`java.exe`) lost its `--model …` args → "Failed to parse model parameters". b9789 narrowed the unconditional override to a **count-guard** (`if (static_cast(utf8.buf.size()) == argc) { argv = utf8.ptrs.data(); }`), but that is exactly the variant the project already found breaks its Windows server-integration tests (when the embedded argv length coincides with `java.exe`'s). The patch carries the **complete upstream change** (so it can be submitted to llama.cpp verbatim and then dropped here): **(1)** `common_params_parse` parses **exactly the argv it is given** (no `GetCommandLineW` magic) and a new `common_params_parse_main()` wrapper holds the UTF-8 recovery for the standalone tools' `main()` (`common/arg.{cpp,h}`); **(2)** the **~34 standalone `main()` call sites** (every `common_params_parse(argc, argv, …)` across `tools/*`, `examples/*` and the `tests/*` programs) flip to `common_params_parse_main()`; **(3)** a `tests/test-arg-parser.cpp` regression case pins that `common_params_parse` honors a caller-supplied argv. The embedded caller (`jllama.cpp`) keeps calling `common_params_parse` and is never overridden. **Our subproject build compiles only the `arg.{cpp,h}` core** — `LLAMA_BUILD_TOOLS`/`LLAMA_BUILD_TESTS` are OFF for a FetchContent subproject — so the flips + test are applied-but-not-compiled here; they were validated via a one-off `-DLLAMA_BUILD_TOOLS=ON -DLLAMA_BUILD_TESTS=ON` build (the new test compiles and its asserts pass; `test-arg-parser`'s only red there is the live `ggml.ai` download check, which is sandbox-network, not the patch). Because it spans **37 files** it must be refreshed on every llama.cpp bump (the applier fails loud). The upstream-facing write-up, including a standalone reproducer that makes llama.cpp's own `test-arg-parser` fail on unmodified `master`, lives in [docs/upstream-investigation-win32-argv-substitution.md](docs/upstream-investigation-win32-argv-substitution.md). | | `0002-server-preserve-caller-load-progress-callback.patch` | Load-progress-callback regression introduced in llama.cpp **b9789**: `server_context::load_model` (`tools/server/server-context.cpp`) now **unconditionally** installs the server's own load-progress reporter on `params_base.load_progress_callback` immediately before `common_init_from_params`, clobbering any callback the embedding caller already set. libjllama's `LoadProgressCallback` feature wires `common_params.load_progress_callback` to a JNI trampoline *before* calling `load_model`, so the bump silently killed it — `LoadProgressCallbackTest` saw zero progress updates and the abort-on-`false` path never threw. The patch guards the assignment with `if (params_base.load_progress_callback == nullptr)`, so the server installs its own reporter **only when the caller hasn't** — a caller-supplied callback survives and fires during load. Standalone `llama-server` (no caller callback, so the field is null) is unaffected. Same JNI-vs-standalone divergence class as `0001`. | | `0003-pr22393-server-add-slot-prompt-similarity-getter-setter.patch` | **Upstream-PR carry** of [ggml-org/llama.cpp#22393](https://github.com/ggml-org/llama.cpp/pull/22393) ("server : add slot_prompt_similarity getter/setter"). Purely additive: adds `server_context::get_slot_prompt_similarity()` / `set_slot_prompt_similarity(float)` (`tools/server/server-context.{cpp,h}`) so an embedding/JNI caller can query and tune the slot-selection threshold at runtime without reloading the model. Verbatim copy of the PR, which **upstream closed without merging** (rejected as exposing unsafe internal state — see the patch header). Carried permanently; it will not be droppable via a version bump. | | `0007-server-attach-http-frontend.patch` | **Adds `llama_server_attach(argc, argv, server_context&)`** so the `NativeServer` *attach mode* can serve an **already-loaded `LlamaModel`** over the upstream HTTP frontend — no second model load, no `start_loop()`; the LlamaModel's worker keeps driving the shared `server_context` and the HTTP routes post tasks to its queue (the queue is the synchronization point). Mechanically: (1) extracts the **pure core route table** (`health` … `slots`) out of `llama_server()` into `static void llama_server_register_common_routes(ctx_http, routes)` (shared, so the two entry points cannot drift on the core endpoint set). **Scope note (narrowed at the b10154 bump):** the helper deliberately carries **only** the stable, state-independent route table — **not** the resumable-streaming routes (their handlers differ between router / non-router), the GCP-compat shim, or the experimental **CORS-proxy / MCP-server / built-in-tools** wiring. b10154 (upstream MCP-server support) moved the streaming routes into the middle of that block and coupled tools/CORS to a per-call `server_mcp mcp_mgr` lifecycle, so the earlier contiguous "route-table + CORS-proxy + tools" extraction is no longer possible; `llama_server()` keeps all of that inline, **byte-identical to upstream b10154** (only the route-table block is factored out). (2) adds `llama_server_attach`, which parses only the HTTP-side argv via `common_params_parse`, starts the stream-session GC + `server_http_context`, registers the common route table, the **non-router** resumable-streaming handlers (upstream b10154 paths `/v1/stream` GET/DEL + `/v1/streams/lookup` POST), the GCP-compat shim, and **403 "disabled" stubs for `/cors-proxy` + `/tools`** (attach mode does not wire the experimental CORS-proxy / MCP / built-in-tools host — those belong to a full `llama-server`, not an embedded model), marks ready immediately (model already loaded), and blocks on the HTTP thread until `llama_server_request_shutdown()` — never calling `common_init()`, backend init, `ctx_server.terminate()` or `llama_backend_free()` (the embedding caller owns those). Applies after `0001`+`0006` (same file); closes the "NativeServer — reuse an already-loaded LlamaModel" TODO. Upstream-submittable ("server: let embedding callers attach the HTTP frontend to an existing server_context"). | | `0008-server-models-worker-cmd-override.patch` | **Makes router mode usable in-JVM.** The router (`server-models.cpp`) spawns each model worker by re-executing its own binary (`get_server_exec_path()` = `/proc/self/exe` & friends) — inside a JVM that binary is `java`, not a llama-server, so embedded router workers could never start. The patch adds env `LLAMA_SERVER_WORKER_CMD` (whitespace-split; read in `server_model_meta::update_args`) which replaces only the leading binary-path token of the rendered worker args, letting an embedding host relaunch workers through its own bootstrap — e.g. `java -cp app.jar net.ladenthin.llama.server.NativeServer` (each worker is then a fresh JVM running the classic single-model `NativeServer`). Exposed in Java as `NativeServer.setWorkerCommand(String...)` (JNI `setenv`); exercised by `RouterModeIntegrationTest` (Linux CI). Upstream-submittable (also useful for containerized/wrapped deployments). | | `0006-server-embed-native-server-jni.patch` | **Makes `server.cpp`'s `llama_server` embeddable in the JVM** so the `NativeServer` JNI bridge can run the full upstream HTTP server (WebUI included) inside `libjllama` — see "Two server modes" below. b9870 already exposes `int llama_server(int, char**)` (non-static; no `main` in the file), so the patch only adds embedded-mode support: (1) a `g_llama_server_embedded` flag + `llama_server_set_embedded()` / `llama_server_request_shutdown()` (declared in the committed `src/main/cpp/native_server_bridge.h`); (2) skips installing the process-wide SIGINT/SIGTERM handlers when embedded (they would hijack the JVM's); (3) in embedded mode parses the **forwarded** argv via `common_params_parse` instead of `common_params_parse_main` (whose `GetCommandLineW` recovery would pick up `java.exe`'s command line — the same Windows class of bug `0001` fixes). `llama_server_request_shutdown()` mirrors the SIGTERM path (invokes the installed `shutdown_handler` → `ctx_server.terminate()` unblocks `start_loop()`), giving JNI an out-of-band stop since `ctx_server` is loop-local. Applies **after `0001`** (which flips this call site to `common_params_parse_main`), so its context is the post-`0001` tree; regenerate against `0001`+source on a bump. Only touches `tools/server/server.cpp`. | -| `0009-subprocess-guard-addchdir-np-old-glibc.patch` | **Fixes the b10154 cross-compile break on old glibc.** b10154 bumped the vendored `vendor/sheredom/subprocess.h` to a version that calls `posix_spawn_file_actions_addchdir_np` (a non-portable extension: glibc **≥ 2.29**, bionic API ≥ 34, macOS ≥ 10.15) to honor a spawn `process_cwd`, and added `common/subproc.cpp` (both pulled in via the new MCP-server support). Upstream guards that call **only for macOS**, so on **manylinux2014 (glibc 2.17)** the declaration is absent and `subprocess.h` (via `subproc.cpp` + `mtmd-helper.cpp` + `server-mcp.cpp`) fails to compile (`'posix_spawn_file_actions_addchdir_np' was not declared`). Android is unaffected — `__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__` already makes the declaration visible (weak). The patch adds a `SUBPROCESS_HAVE_ADDCHDIR_NP` compile-time probe (`__GLIBC_PREREQ(2, 29)`, nested under `defined(__GLIBC__)` so non-glibc platforms never evaluate it) and, when unavailable, reports a requested cwd as `ENOSYS` instead of failing to compile — this build never spawns with a cwd. **Not reproducible on a modern-glibc dev box** (the `addchdir_np` branch is taken there); the manylinux CI job is the gate. Upstream-submittable (to sheredom/subprocess.h). Only touches `vendor/sheredom/subprocess.h`. | +| `0009-subprocess-guard-addchdir-np-old-glibc.patch` | **Fixes the b10154 cross-compile break on old glibc.** b10154 bumped the vendored `vendor/sheredom/subprocess.h` to a version that calls `posix_spawn_file_actions_addchdir_np` (a non-portable extension: glibc **≥ 2.29**, bionic API ≥ 34, macOS ≥ 10.15) to honor a spawn `process_cwd`, and added `common/subproc.cpp` (both pulled in via the new MCP-server support). Upstream guards that call **only for macOS**, so on **manylinux2014 (glibc 2.17)** the declaration is absent and `subprocess.h` (via `subproc.cpp` + `mtmd-helper.cpp` + `server-mcp.cpp`) fails to compile (`'posix_spawn_file_actions_addchdir_np' was not declared`). Android is unaffected — `__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__` already makes the declaration visible (weak). The patch adds a `SUBPROCESS_HAVE_CWD` compile-time probe (`__GLIBC_PREREQ(2, 29)`, nested under `defined(__GLIBC__)` so non-glibc platforms never evaluate it, and skipped entirely when the macro is already defined so unrecognized platforms can override it) next to the POSIX includes and, when unavailable, reports a requested cwd as `ENOSYS` instead of failing to compile — this build never spawns with a cwd. **Not reproducible on a modern-glibc dev box** (the `addchdir_np` branch is taken there); the manylinux CI job is the gate. Also affects `manylinux_2_28` (glibc 2.28). **Submitted upstream as [sheredom/subprocess.h#104](https://github.com/sheredom/subprocess.h/pull/104)** and byte-identical to its head `620ce44`, so the applier will report "already applied" and skip once llama.cpp bumps the vendored pin. Two side findings from that work were filed separately and do not affect this project: [#105](https://github.com/sheredom/subprocess.h/pull/105) (`-std=c++20` unknown to GCC 8) and [#106](https://github.com/sheredom/subprocess.h/pull/106) (`posix_spawn` not reporting exec failures before glibc 2.24). Only touches `vendor/sheredom/subprocess.h`. | **`0005` was dropped at the b9981 bump.** Upstream's own `server-context.cpp` picked up an equivalent — and broader — fix for the same checkpoint-starvation problem: `create_checkpoint` diff --git a/TODO.md b/TODO.md index a61917482..221c0b6f2 100644 --- a/TODO.md +++ b/TODO.md @@ -98,7 +98,8 @@ upstream PR #22393 — it drops automatically when that merges.) - **`0001` Windows arg-parse embed guard** (against #24779): `common_params_parse` trusts the caller's argv; `common_params_parse_main()` keeps the standalone tools' UTF-8 recovery. Ship with the standalone-safe repro (synthetic argv discarded on Windows because `GetCommandLineW()` returns the - host process line). + host process line) — written up, with the reproducer executed, in + `docs/upstream-investigation-win32-argv-substitution.md`. - **`0002` preserve caller load-progress callback** (b9789 regression: server clobbers `params_base.load_progress_callback`). - **`0006` embeddable `llama_server`** (no process signal handlers, forwarded-argv parse, out-of-band @@ -107,7 +108,9 @@ upstream PR #22393 — it drops automatically when that merges.) - **`0008` `LLAMA_SERVER_WORKER_CMD` router worker override** (also useful for containerized/wrapped deployments). - **`0009` guard `posix_spawn_file_actions_addchdir_np` on old glibc** (b10154 cross-compile break on - manylinux2014 / glibc 2.17; adds a `__GLIBC_PREREQ(2, 29)` probe — submittable to sheredom/subprocess.h). + manylinux2014 / glibc 2.17 and manylinux_2_28 / glibc 2.28; adds an overridable + `SUBPROCESS_HAVE_CWD` probe via `__GLIBC_PREREQ(2, 29)` — submitted as sheredom/subprocess.h#104, + drops automatically once llama.cpp bumps the vendored pin). ### llama.cpp upstream feature exposure (queued, deferred by policy) diff --git a/docs/upstream-investigation-win32-argv-substitution.md b/docs/upstream-investigation-win32-argv-substitution.md new file mode 100644 index 000000000..cda5701e2 --- /dev/null +++ b/docs/upstream-investigation-win32-argv-substitution.md @@ -0,0 +1,136 @@ +# `common_params_parse` can silently discard the caller's argv on Windows + +Technical findings for llama.cpp. Everything below was verified against +`ggml-org/llama.cpp` `master` @ [`ddd4ec142`](https://github.com/ggml-org/llama.cpp/commit/ddd4ec1428a6201e18975ea52b07c71e0f9aef26) +(`chat : enable tool call in thinking for DS4 (#26269)`). + +Downstream context: this is patch `0001` of [java-llama.cpp](https://github.com/bernardladenthin/java-llama.cpp), +which embeds `llama-server` in a JVM process and therefore builds its own `argv`. + +## Summary + +On Windows, `common_params_parse` replaces the `argv` it was passed with one +reconstructed from the process command line, whenever the element counts happen to +match. The contents are never compared. A caller that constructs its own `argv` can +therefore have completely unrelated arguments parsed, with no warning and no way to +detect it from the return value. + +This is observable today in llama.cpp's own test suite, without any patch. + +## Affected code + +[`common/arg.cpp:1203-1209`](https://github.com/ggml-org/llama.cpp/blob/ddd4ec1428a6201e18975ea52b07c71e0f9aef26/common/arg.cpp#L1203-L1209) + +```c +bool common_params_parse(int argc, char ** argv, common_params & params, llama_example ex, void(*print_usage)(int, char **)) { +#ifdef _WIN32 + auto utf8 = make_utf8_argv(); + // repair argv only when it matches the process command line + if (static_cast(utf8.buf.size()) == argc) { + argv = utf8.ptrs.data(); + } +#endif +``` + +`make_utf8_argv()` ([`common/arg.cpp:1180-1200`](https://github.com/ggml-org/llama.cpp/blob/ddd4ec1428a6201e18975ea52b07c71e0f9aef26/common/arg.cpp#L1180-L1200)) +builds the UTF-8 argv from `GetCommandLineW()` via `CommandLineToArgvW()`. It reads the +**process** command line, independently of what the caller passed in. + +Origin of the code: + +- PR [#24779](https://github.com/ggml-org/llama.cpp/pull/24779) - `mtmd, arg: fix utf8 handling on windows`, merged 2026-06-19 by @ngxson +- Issue [#18571](https://github.com/ggml-org/llama.cpp/issues/18571) - the UTF-8 problems it addressed + +The recovery itself is correct and needed. Only its placement inside the general parsing +entry point is the problem. + +## Reproducer + +No JNI, no patch, no special configuration - just `master` and the existing test binary, +invoked two different ways. + +``` +> test-arg-parser.exe +test-arg-parser: test invalid usage +test-arg-parser: all tests OK +exit 0 + +> test-arg-parser.exe -m spoofed.gguf +to show complete usage, run with -h +Assertion failed: false == common_params_parse(argv.size(), list_str_to_char(argv).data(), params, LLAMA_EXAMPLE_COMMON), +file X:\...\llama.cpp\tests\test-arg-parser.cpp, line 96 +exit 0xC0000409 +``` + +The failing assertion is a negative test, +[`tests/test-arg-parser.cpp:95-96`](https://github.com/ggml-org/llama.cpp/blob/ddd4ec1428a6201e18975ea52b07c71e0f9aef26/tests/test-arg-parser.cpp#L95-L96): + +```c +// wrong value (int) +argv = {"binary_name", "-ngl", "hello"}; +assert(false == common_params_parse(argv.size(), list_str_to_char(argv).data(), params, LLAMA_EXAMPLE_COMMON)); +``` + +Three elements. The process command line also had three tokens, so the constructed argv +was replaced by `{test-arg-parser.exe, -m, spoofed.gguf}` - which parses successfully, +while the test expects a parse failure. + +Note that `test-arg-parser.cpp` declares `int main(void)` and never touches its own +arguments. `GetCommandLineW()` sees them regardless. + +That this does not surface in CI is only due to the token count the test binary happens +to be invoked with there. + +### Environment + +| | | +|---|---| +| llama.cpp | `master` @ `ddd4ec142`, unmodified | +| OS | Windows 11 Pro 26200 | +| Compiler | MSVC 19.44.35228 (VS Build Tools 2022 17.14), Ninja | + +``` +cmake -S llama.cpp -B build -G Ninja -DCMAKE_BUILD_TYPE=Release ^ + -DLLAMA_BUILD_TESTS=ON -DLLAMA_BUILD_EXAMPLES=OFF -DLLAMA_BUILD_TOOLS=OFF ^ + -DLLAMA_BUILD_SERVER=OFF -DLLAMA_BUILD_APP=OFF -DLLAMA_BUILD_UI=OFF ^ + -DLLAMA_OPENSSL=OFF -DLLAMA_SUBPROCESS=OFF -DGGML_OPENMP=OFF +cmake --build build --target test-arg-parser +``` + +## Who is affected + +- `tests/test-arg-parser.cpp`, as shown above +- any embedded user of the `common` library that supplies its own `argv` rather than the + process command line + +## Possible directions + +### 1. Separate the two meanings + +`common_params_parse` parses exactly what it is given. A new `common_params_parse_main` +performs the Windows recovery first and is what the standalone tools' `main()` calls. + +- the intent becomes explicit at the call site, and the library function stops second + guessing its own parameters +- cost: 37 files, because every `main()` has to be moved over. The substantive part is + about 30 lines in `common/arg.cpp` and `common/arg.h`; the rest is mechanical. + +Reference implementation, based on `ddd4ec142`: + +- branch: [`fix/win32-arg-parse-honor-caller-argv`](https://github.com/bernardladenthin/llama.cpp/tree/fix/win32-arg-parse-honor-caller-argv) +- diff: +- single commit `6468993c2`, 37 files, +67 / -42 +- adds a regression test to `tests/test-arg-parser.cpp` + +### 2. Tighten the condition + +Compare the contents rather than only the element count, and substitute only on a full +match. + +- much smaller, no API change, no call site churn +- risk: the comparison between the CRT `argv` and the ANSI round-trip of the wide argv + may not hold under every codepage. Where it does not, the substitution stops happening + and the fix from #24779 silently stops applying - breaking exactly what it protects. + +Direction 1 avoids that risk because it never has to guess whether the caller's argv is +the process command line: the call site says so. diff --git a/llama/patches/0009-subprocess-guard-addchdir-np-old-glibc.patch b/llama/patches/0009-subprocess-guard-addchdir-np-old-glibc.patch index 40cd1a691..fd862be16 100644 --- a/llama/patches/0009-subprocess-guard-addchdir-np-old-glibc.patch +++ b/llama/patches/0009-subprocess-guard-addchdir-np-old-glibc.patch @@ -1,32 +1,35 @@ diff --git a/vendor/sheredom/subprocess.h b/vendor/sheredom/subprocess.h -index 5e809023a..f980bf386 100644 +index 5e80902..dc0153c 100644 --- a/vendor/sheredom/subprocess.h +++ b/vendor/sheredom/subprocess.h -@@ -1203,10 +1203,27 @@ cleanup: - } - actions_created = 1; +@@ -274,6 +274,21 @@ subprocess_weak int subprocess_alive(struct subprocess_s *const process); + #include + #endif -+ /* [jllama] posix_spawn_file_actions_addchdir_np is a non-portable extension -+ (glibc >= 2.29, bionic API >= 34, macOS >= 10.15). On older glibc such as -+ manylinux2014 (glibc 2.17) the declaration is absent, which broke the -+ cross-compile build at b10154 (new vendored subprocess.h + common/subproc.cpp). -+ Detect it and fall back to reporting cwd-spawning as unsupported (ENOSYS) -+ instead of failing to compile. This build path never spawns with a cwd. */ ++/* Whether subprocess_create_ex can honour process_cwd. glibc only gained ++ posix_spawn_file_actions_addchdir_np in 2.29. Define this yourself to ++ override the detection, for instance on musl older than 1.1.24. */ ++#if !defined(SUBPROCESS_HAVE_CWD) +#if defined(__GLIBC__) -+# if __GLIBC_PREREQ(2, 29) -+# define SUBPROCESS_HAVE_ADDCHDIR_NP 1 -+# else -+# define SUBPROCESS_HAVE_ADDCHDIR_NP 0 -+# endif ++#if __GLIBC_PREREQ(2, 29) ++#define SUBPROCESS_HAVE_CWD 1 +#else -+# define SUBPROCESS_HAVE_ADDCHDIR_NP 1 ++#define SUBPROCESS_HAVE_CWD 0 +#endif - // Set working directory ++#else ++#define SUBPROCESS_HAVE_CWD 1 ++#endif ++#endif ++ + #if defined(_WIN32) + + #include +@@ -1207,6 +1222,8 @@ cleanup: if (process_cwd) { #if defined(__APPLE__) && MAC_OS_X_VERSION_MIN_REQUIRED >= 260000 posix_error = posix_spawn_file_actions_addchdir(&actions, process_cwd); -+#elif !SUBPROCESS_HAVE_ADDCHDIR_NP -+ posix_error = ENOSYS; /* [jllama] old glibc: addchdir_np unavailable */ ++#elif !SUBPROCESS_HAVE_CWD ++ posix_error = ENOSYS; #else #if defined(__APPLE__) && defined(__clang__) #pragma clang diagnostic push