diff --git a/CLAUDE.md b/CLAUDE.md index d0ae89fbd..54db9e4ff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -440,6 +440,29 @@ every answer a confident `absent`, and a missing `\b` that reported `fesetenv`. *The second is the worse shape: a hit, naming a real file.* The control checks WHERE and not just whether, for that reason), +`cfghdr-sweep.py` (**gates on ONE thing**: a `#define` in an +autoconf TEMPLATE that the hand-answered `config.h` does not carry. +`autoheader` emits only `#undef NAME`, so anything spelled `#define` +in a `.h.in` is text copied STRAIGHT THROUGH -- and *reading the +GENERATED config as a list of capabilities, which is how php's was +written, does not show it*. php's template copies three, and without +them `zend_map_ptr.h:82` answered `not a function` / `syntax error, +last name: ZEND_API`: an undefined identifier where a storage class +belongs, in a shape **every `ZEND_API` in php has**. 21 names were +missing in all. It does NOT gate on an unanswered `#undef` -- php's +template asks 696 questions and 474 are deliberately silent -- nor on +a name the answered file invents, which is reported because only the +author can tell a port's own addition from a typo. *It found one of +each on its first run*: `PHP_OS_FAMILY`, which `main/php.h:38` computes +itself so defining it was a collision, and five in clisp's config +that nobody has looked at. **Its LIMIT is that it reads names and not +arms** -- a `#define` inside a platform arm nobody takes is reported +though the omission is right -- so `PAIRS` holds only pairs whose +findings someone has read. **Its control caught two faults in two +runs**: the include guard is a `#define` like any other and was +demanded of the answered file, and a name the template both `#define`s +in a dead arm and `#undef`s elsewhere is a QUESTION rather than +verbatim text), `truefalse-sweep.py` (**does NOT gate**: which vendored TUs use `true`/`false` as an IDENTIFIER without defining the names themselves — the shape kencc's C23 keywords break. **Its positive control is diff --git a/docs/notes/php.md b/docs/notes/php.md index 374911c99..92f383133 100644 --- a/docs/notes/php.md +++ b/docs/notes/php.md @@ -464,6 +464,55 @@ beats meta-rule in every mk, which needs no checking at all.* with the real include paths, filtered to `No such file or directory`. One sweep instead of 31 round trips. +### The first real C diagnostic, and it was an omission in the config.h + +`TSRM/TSRM.c` compiled -- the first object of the port -- and +`Zend/Optimizer/block_pass.c` then answered, from +`Zend/zend_map_ptr.h:82`: + +``` +not a function +not a parameter: zend_map_ptr_reset +syntax error, last name: ZEND_API +``` + +`ZEND_API void zend_map_ptr_reset(void);` with `ZEND_API` an +undefined identifier -- and **every `ZEND_API` in php is that shape**, +so the first file to declare anything would have failed whichever one +it was. + +***THE CAUSE IS A BLIND SPOT IN HOW THIS FILE WAS WRITTEN.*** +`autoheader` emits only `#undef NAME` lines; anything spelled +`#define` in a `.h.in` is **text copied straight through**. +`main/php_config.h.in:7-15` copies three -- `ZEND_API`, +`ZEND_DLEXPORT`, `ZEND_DLIMPORT` -- under an `#if defined(__GNUC__)` +whose `#else` arm makes all three empty. *Reading the generated +config as a list of capabilities, which is exactly the method used +here, does not show them.* The prologue is copied verbatim now; its +guard was already right for kencc. + +**21 names were missing in all**, and the one check that shows them +is a diff of the two name sets. That is `sys/lib/tests/cfghdr-sweep.py` +now, which gates on this class and reports two others -- and found +two things on its first run beyond the case it was built for: + +* **`PHP_OS_FAMILY` was mine and was a collision.** `main/php.h:38-51` + computes it from the platform macros, ending in an unconditional + `#else "Unknown"`, so php.h would have redefined it with a different + string. Plan 9 defines none of `BSD`, `__linux__` or the rest, so + php answers `"Unknown"`, which is simply true. Removed. +* **Five in clisp's hand-answered config** -- `_GL_UNUSED`, + `_UNUSED_PARAMETER_`, `__GNUC_STDC_INLINE__` and two under a dead + `__SUNPRO_CC` arm. clisp builds and runs, so nothing it compiles + uses them; *recorded, not acted on*, which is why clisp is not in + the sweep's gating list. + +The other 18 are `_GNU_SOURCE` and its family, `__STDC_WANT_*`, and +two Linux headers. **Left out deliberately**: APE's feature macros are +`_POSIX_SOURCE`, `_BSD_EXTENSION` and `_PLAN9_SOURCE`, which the +mkfile already passes, and `_GNU_SOURCE` would be a claim about +headers this platform does not have. + ## What the config.h decided, and why by hand The long reasoning is in the file itself; what matters as a summary: diff --git a/sys/lib/tests/cfghdr-sweep.py b/sys/lib/tests/cfghdr-sweep.py new file mode 100644 index 000000000..e136996a7 --- /dev/null +++ b/sys/lib/tests/cfghdr-sweep.py @@ -0,0 +1,241 @@ +#!/usr/bin/env python3 +""" +cfghdr-sweep -- does a hand-answered `config.h' answer the questions +its own TEMPLATE asks, and does it carry the template's verbatim text? + + python3 sys/lib/tests/cfghdr-sweep.py # every pair it knows + python3 sys/lib/tests/cfghdr-sweep.py T.h.in A.h # one pair + python3 sys/lib/tests/cfghdr-sweep.py --control + +A HOST SWEEP. **It GATES on one thing only** -- see "what it gates +on" below -- and reports the rest, because most of what it finds is a +deliberate omission rather than a fault. + +------------------------------------------------------------------ +WHAT IT IS FOR, AND IT COST A VM ROUND TRIP. + +This tree hand-maintains a `config.h' for perl, bash, clisp, chicken +and now php, each against an autoconf template that is vendored +beside it. php's was written by reading the GENERATED config from a +host `configure' run and answering each capability -- which is the +right method for the `#undef' lines and **blind to everything else**. + +`autoheader' emits only `#undef NAME' lines. Anything spelled +`#define' in a template is text copied STRAIGHT THROUGH, and +`main/php_config.h.in:7-15' copies three: + + #if defined(__GNUC__) && __GNUC__ >= 4 + # define ZEND_API __attribute__ ((visibility("default"))) + ... + #define ZEND_DLIMPORT + +Reading the generated file as a list of capabilities does not show +them, so the hand-written one had none, and `Zend/zend_map_ptr.h:82' +answered + + not a function + not a parameter: zend_map_ptr_reset + syntax error, last name: ZEND_API + +-- an undefined identifier where a storage class belongs. *Every +`ZEND_API' in php is that shape*, so the first file to declare +anything would have failed whichever it was. **21 names were missing +in all**, and the one `grep` that would have shown them is this file. + +------------------------------------------------------------------ +WHAT IT GATES ON, AND WHY ONLY THAT. + +A template `#define` the answered file does not define. That is the +copied-through text, it is never optional, and there are three of +them in php's. + +It does NOT gate on a template `#undef` the answered file is silent +about. php's template asks about 1100 names, hundreds of them +`COMPILE_DL_` for extensions this port does not build, and +silence there is the correct answer. *A gate that demands an answer +to every question would demand noise*, which is the same reason +`hosthelper-sweep` does not gate. + +***AND ITS LIMIT, STATED RATHER THAN DISCOVERED LATER.*** It reads +names, not arms. A template that `#define's something only inside a +platform arm nobody takes -- clisp's `_Restrict' and `__restrict__' +live under `#if defined __SUNPRO_CC' with no `#else' -- is reported +as missing when the answered file is right to omit it. Telling those +apart needs real preprocessing, which is more than this should do, so +a finding is READ rather than obeyed and `PAIRS' holds only the pairs +whose findings someone has looked at. + +It also reports, without gating, a name the answered file defines +that the template has never heard of. That is usually a port's own +addition and sometimes a typo; only the author can tell, so it is +printed rather than counted. +""" + +import os +import re +import sys + +ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), + '..', '..', '..')) +if not os.path.isdir(os.path.join(ROOT, 'sys', 'src', 'ape')): + sys.exit('cfghdr-sweep: %s is not the APExp root -- refusing to ' + 'report 0 findings from the wrong tree' % ROOT) + +# (template, answered) -- both relative to the repository root. +PAIRS = [ + ('sys/src/external/php/main/php_config.h.in', + 'sys/src/ape/cmd/php/cfg/main/php_config.h'), +] + +DEFINE = re.compile(r'^\s*#\s*define\s+([A-Za-z_][A-Za-z0-9_]*)') +IFNDEF = re.compile(r'^\s*#\s*ifndef\s+([A-Za-z_][A-Za-z0-9_]*)') +UNDEF_C = re.compile(r'^\s*/\*\s*#\s*undef\s+([A-Za-z_][A-Za-z0-9_]*)\s*\*/') +UNDEF_H = re.compile(r'^\s*#\s*undef\s+([A-Za-z_][A-Za-z0-9_]*)') + + +def scan(path): + """(defines, undefs) as name sets. + + ***THE INCLUDE GUARD IS EXCLUDED, AND THE CONTROL IS WHAT SAID + SO.*** A guard is a `#define' like any other, so the first + version of this sweep demanded that an answered file carry the + TEMPLATE's guard name -- which is nonsense: `X_CONFIG_H' came + back as a gating finding beside the one real one. php's happens + to be `PHP_CONFIG_H' in both files, so the live pair could never + have shown it. *A fixture is allowed to differ from the real + case in exactly the way that exposes the bug.* + """ + d, u = set(), set() + guarded = set() + with open(path, encoding='utf-8', errors='replace') as f: + for line in f: + m = IFNDEF.match(line) + if m: + guarded.add(m.group(1)) + m = UNDEF_C.match(line) # the answered form, first: + if m: # `/* #undef X */' also matches + u.add(m.group(1)) # nothing else, but check it + continue # before DEFINE either way + m = DEFINE.match(line) + if m: + if m.group(1) not in guarded: # not the include guard + d.add(m.group(1)) + continue + m = UNDEF_H.match(line) + if m: + u.add(m.group(1)) + return d, u + + +def check(tmpl, ans): + """Returns the number of GATING findings.""" + tdef, tundef = scan(tmpl) + adef, aundef = scan(ans) + gating = 0 + + # 1. the copied-through text. This is the gate. + # + # ***A NAME THE TEMPLATE BOTH `#define's AND `#undef's IS A + # QUESTION, NOT VERBATIM TEXT***, and subtracting `tundef' is what + # says so. clisp's template `#define's WORDS_BIGENDIAN inside an + # `#if defined AC_APPLE_UNIVERSAL_BUILD' arm and `#undef's it in + # the `#else' -- the second is the real question, and the answered + # file answers it. Without this the sweep reported it as missing + # verbatim text, which is the cry-wolf failure. + missing = sorted(tdef - tundef - adef) + if missing: + print(' MISSING template #defines -- these are verbatim text,') + print(' not capability answers, and are never optional:') + for n in missing: + print(' %s' % n) + gating += len(missing) + + # 2. questions left unanswered. Information only. + silent = sorted(tundef - adef - aundef) + print(' %d of the template\'s %d questions are unanswered' + % (len(silent), len(tundef))) + if len(silent) <= 12: + for n in silent: + print(' %s' % n) + else: + print(' (too many to list; most will be deliberate)') + + # 3. names the template never heard of. Information only. + unknown = sorted((adef | aundef) - tdef - tundef) + if unknown: + print(' %d name(s) answered that the template does not ask about:' + % len(unknown)) + for n in unknown: + print(' %s' % n) + return gating + + +CONTROL_TMPL = """\ +#ifndef X_CONFIG_H +#define X_CONFIG_H + +#if defined(__GNUC__) +# define COPIED_THROUGH __attribute__((whatever)) +#else +# define COPIED_THROUGH +#endif + +/* a capability */ +#undef HAVE_ANSWERED + +/* another */ +#undef HAVE_SILENT +#endif +""" + +CONTROL_ANS = """\ +#define HAVE_ANSWERED 1 +/* #undef HAVE_DENIED */ +#define INVENTED_HERE 1 +""" + + +def control(): + """The template `#define' must gate; the unanswered question must + not; the invented name must be reported and not counted. *All + three in one fixture*, because the bug this sweep exists for was a + missing `#define' sitting beside hundreds of correctly unanswered + `#undef's -- a control with only the first would pass for a sweep + that gated on everything.""" + import tempfile + with tempfile.TemporaryDirectory() as d: + t = os.path.join(d, 't.h.in'); open(t, 'w').write(CONTROL_TMPL) + a = os.path.join(d, 'a.h'); open(a, 'w').write(CONTROL_ANS) + print('control:') + n = check(t, a) + ok = (n == 1) + print('control: %d gating finding(s), want 1 (COPIED_THROUGH) -- %s' + % (n, 'ok' if ok else 'FAILED')) + return 0 if ok else 1 + + +def main(): + if '--control' in sys.argv: + return control() + args = [a for a in sys.argv[1:] if not a.startswith('-')] + pairs = [(args[0], args[1])] if len(args) == 2 else \ + [(os.path.join(ROOT, t), os.path.join(ROOT, a)) + for t, a in PAIRS] + gating = 0 + for tmpl, ans in pairs: + if not (os.path.exists(tmpl) and os.path.exists(ans)): + print('cfghdr-sweep: skipping, missing %s or %s' + % (tmpl, ans)) + continue + print('%s' % os.path.relpath(ans, ROOT)) + gating += check(tmpl, ans) + print() + print('---- %d template #define(s) not carried through' % gating) + if gating: + print('Copy them VERBATIM from the template: they are not') + print('capability answers and autoheader never generates them.') + return gating + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/sys/src/ape/cmd/php/cfg/main/php_config.h b/sys/src/ape/cmd/php/cfg/main/php_config.h index 0d2abd9d1..06e46d72b 100644 --- a/sys/src/ape/cmd/php/cfg/main/php_config.h +++ b/sys/src/ape/cmd/php/cfg/main/php_config.h @@ -33,6 +33,41 @@ #ifndef PHP_CONFIG_H #define PHP_CONFIG_H +/* + * ***THE PROLOGUE IS UPSTREAM'S OWN, COPIED VERBATIM FROM + * `main/php_config.h.in:7-15', AND LEAVING IT OUT BROKE THE BUILD.*** + * These three are NOT autoconf answers -- autoheader copies them + * through from the template -- so they are invisible to anyone + * reading the generated file as a list of capabilities, which is how + * this file was first written. `Zend/zend_map_ptr.h:82' then + * answered + * + * not a function + * not a parameter: zend_map_ptr_reset + * syntax error, last name: ZEND_API + * + * because `ZEND_API void zend_map_ptr_reset(void);' had an undefined + * identifier where a storage class belongs. *Every `ZEND_API' in php + * is that shape*, so the first file to declare anything would have + * failed whichever it was. + * + * The guard is upstream's and is already right for kencc, which is + * not `__GNUC__': all three come out empty, which is what a platform + * with no visibility attributes and no dynamic linking wants. + */ +#if defined(__GNUC__) && __GNUC__ >= 4 +# define ZEND_API __attribute__ ((visibility("default"))) +# define ZEND_DLEXPORT __attribute__ ((visibility("default"))) +#else +# define ZEND_API +# define ZEND_DLEXPORT +#endif + +#define ZEND_DLIMPORT + +/* `cfg/build-defs.h' is ours; `main/php.h' includes it unguarded. */ +#define HAVE_BUILD_DEFS_H 1 + /* ------------------------------------------------------------------ * * Type sizes. **kencc amd64 is LLP64**: `long' is FOUR bytes while * pointers and `size_t' are eight. php survives that because @@ -62,8 +97,10 @@ * ------------------------------------------------------------------ */ /* #undef HAVE_ASM_GOTO */ /* #undef HAVE_GCC_GLOBAL_REGS */ -/* #undef HAVE_MUSTTAIL */ -/* #undef HAVE_PRESERVE_NONE */ +/* HAVE_MUSTTAIL and HAVE_PRESERVE_NONE are NOT config.h's: they are + * set by `Zend/zend_portability.h:342' from `__has_attribute', which + * answers 0 for kencc. Saying anything about them here would only + * suggest this file owns a decision it does not. */ /* #undef HAVE_ALIGNOF */ /* #undef HAVE_ATTRIBUTE_ALIGNED */ /* #undef HAVE_FUNC_ATTRIBUTE_IFUNC */ @@ -353,11 +390,21 @@ #define PHP_OS "Plan9" #define PHP_UNAME "Plan9" #define PHP_BUILD_SYSTEM "Plan9 APExp" -#define PHP_OS_FAMILY "BSD" +/* + * ***`PHP_OS_FAMILY' IS NOT OURS TO ANSWER, AND DEFINING IT HERE WAS + * A COLLISION.*** `main/php.h:38-51' computes it from the platform + * macros and its chain ends in an unconditional + * `#else # define PHP_OS_FAMILY "Unknown"', so php.h would have + * redefined this one with a different string. `cfghdr-sweep.py' + * reported it as a name the template never asks about, which is what + * an invented macro looks like from outside. + * + * Plan 9 defines none of `BSD', `__linux__', `__sun__' or the rest, + * so php answers "Unknown" -- which is simply true. + */ #define DEFAULT_SHORT_OPEN_TAG "1" #define PHP_SIGCHILD 0 #define ZEND_DEBUG 0 -#define ZEND_DLIMPORT #define PHP_WRITE_STDOUT 1 #define PHP_USE_PHP_CRYPT_R 1 /* no crypt.h; php bundles four */ #define HAVE_BUNDLED_PCRE 1