From 5aa47103dee0682530c73e05b3b426823dcb1c8d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 11:21:11 +0000 Subject: [PATCH] ape: guard MIN/MAX in , as glibc and the BSDs do zend_optimizer.c stopped on `/sys/include/ape/sys/param.h:36 ... Macro redefinition of MIN'. php is careful: zend_portability.h:479 is `#undef MIN' / `#undef MAX' followed by its own spelling, and main/php.h:215 then includes , which redefined them with a different token sequence. This tree's own rule that a macro's identity includes whether there is white space, arriving from the other side. The fault is ours rather than php's. MIN and MAX are not POSIX, so a program that wants them defines its own, and glibc and the BSDs both wrap theirs in #ifndef for that reason; APE's copy came from musl, which is the odd one out. The guard can only turn an error into a success, since it changes nothing unless the name is already defined and that case is a diagnostic today. A grep for every other definition of these names in the APE headers found none, and apehdr-sweep is unchanged at 152 headers, 0 findings, the same 8 pre-existing TOGETHER errors. Also records the answer to whether the missing atomics are worth implementing: no, for this configuration, with the reasoning in docs/notes/php.md. Across everything this port compiles they guard exactly two flags, EG(vm_interrupt) and EG(timed_out), written from a handler and read by the VM loop -- the sig_atomic_t case rather than the shared-memory one -- and the build is non-ZTS, so the only asynchrony is a Plan 9 note and an aligned load or store on x86-64 is already indivisible. The one genuine gap is the read-modify-write in zend_atomic_bool_exchange_ex, a two-instruction window that CLI does not reach because max_execution_time is 0 by default. Real atomics would be per-architecture assembly, the same shape as ucontext, and would not change this build. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WGAwvvTwDg2yknFkmZ3qzs --- docs/notes/php.md | 60 +++++++++++++++++++++++++++++++++++++ sys/include/ape/sys/param.h | 27 ++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/docs/notes/php.md b/docs/notes/php.md index 53ca59fa6..85b81335a 100644 --- a/docs/notes/php.md +++ b/docs/notes/php.md @@ -284,6 +284,47 @@ confidently and wrongly*, which is the clearest single argument in this port for hand-answering rather than deriving: the mechanical method is right 90 times and this is one of the others. +### The atomics: not worth implementing for this build, and here is why + +`#warning No atomics support detected` appears on every file and is +cosmetic -- `zend_atomic.h:32` falls to `HAVE_NO_ATOMICS`, a plain +`volatile` implementation. Asked directly whether to implement real +ones, the answer is **no for this configuration**, and the reasoning +is worth keeping because it is about THIS build rather than about +atomics. + +**What they actually guard here is two flags.** Across everything +this port compiles, the only consumers are `EG(vm_interrupt)` and +`EG(timed_out)` -- `zend_execute_API.c:175-176`, `:1081-1082`, +`:1466-1553`, and `zend_alloc.c:441`. Both are written from a +signal or timer handler and read by the VM loop. *That is the +`sig_atomic_t` case, not the shared-memory case.* + +**This build is non-ZTS**, so there are no threads to race: the only +asynchrony is a Plan 9 note. An aligned naturally-sized load or store +on x86-64 is already indivisible, which is exactly what the +`volatile` fallback emits. + +***THE ONE GENUINE GAP IS THE READ-MODIFY-WRITE***, not the loads and +stores: `zend_atomic_bool_exchange_ex` at `zend_execute_API.c:1081` +is a load then a store, and a note arriving between them could lose +a flag. The window is two instructions, and *CLI's +`max_execution_time` is 0 by default so nothing arms the timer that +would set the one flag at risk* -- so it is a real race that this +configuration does not reach. + +**The cost is a libap project, not a config line.** kencc has no +`_Atomic`, no `__sync_*` and no `__atomic_*`, so real atomics mean +assembly -- `LOCK XCHG` and `LOCK CMPXCHG` -- per architecture. +`arch/amd64/tas.s` already has the primitive, so the shape is the +same as `ucontext`: amd64 first, the other ten later. **It would not +change this build at all**, since the volatile path compiles and the +warning is noise. + +*Revisit if ZTS is ever enabled, or if opcache is made to actually +run with shared memory* -- those are the configurations where real +concurrency appears, and both are deliberately off here. + ## Extensions: the ones with a ported library The user's own framing, and the right one: enable what this tree already @@ -507,6 +548,25 @@ two things on its first run beyond the case it was built for: uses them; *recorded, not acted on*, which is why clisp is not in the sweep's gating list. +### `MIN`/`MAX`, and the header at fault was ours + +`zend_optimizer.c` stopped on +`/sys/include/ape/sys/param.h:36 ... Macro redefinition of MIN`. +php is careful here -- `Zend/zend_portability.h:479-482` is +`#undef MIN` / `#undef MAX` and then its own spelling -- and +`main/php.h:215` then includes ``, which redefined them +with a different token sequence. *This tree's own rule that a macro's +identity includes whether there is white space, arriving from the +other side.* + +**The fix is APE's, not php's.** `MIN`/`MAX` are not POSIX, so a +program that wants them defines its own; glibc and the BSDs both wrap +theirs in `#ifndef` for exactly that reason, and APE's copy came from +musl, which is the odd one out. Guarded now. *The guard can only turn +an error into a success*, since it changes nothing unless the name is +already defined and that case is a diagnostic today. A grep for every +other definition of these names in the APE headers found none. + ### And the same mistake had a second half: the EPILOGUE The next build got past `ZEND_API` and stopped at diff --git a/sys/include/ape/sys/param.h b/sys/include/ape/sys/param.h index 2ed9cf410..13ccec14a 100644 --- a/sys/include/ape/sys/param.h +++ b/sys/include/ape/sys/param.h @@ -32,9 +32,34 @@ extern "C" { #define NOFILES_MAX 100 -//copied from musl +/* + * Copied from musl -- ***AND THE GUARDS ARE OURS, BECAUSE musl IS THE + * ODD ONE OUT HERE.*** glibc and the BSDs both wrap these in + * `#ifndef', and they are right to: `MIN'/`MAX' are not in POSIX, so + * a program that wants them defines its own, and a header that + * redefines afterwards is a hard error rather than a nicety. + * + * php does exactly that. `Zend/zend_portability.h:479-482' is + * `#undef MIN' / `#undef MAX' and then its own spelling with a space + * after the comma; `main/php.h:215' then includes and + * kencc answered + * + * /sys/include/ape/sys/param.h:36 ... Macro redefinition of MIN + * + * -- *a redefinition with a different token sequence*, which is the + * rule this tree already carries as "a macro's identity includes + * whether there is white space". + * + * The guard can only ever turn an error into a success: it changes + * nothing unless the name is already defined, and today that case is + * a diagnostic. + */ +#ifndef MIN #define MIN(a,b) (((a)<(b))?(a):(b)) +#endif +#ifndef MAX #define MAX(a,b) (((a)>(b))?(a):(b)) +#endif #ifdef __cplusplus