Zero C-compiler warnings on all three architectures; fix real restore_vm_state() bug
Maintainability sweep (prompted by "this is getting hard to maintain"): fixed the remaining three warning classes after the missing-field- initializers commit -- 2x -Wsign-compare (control_words.c, cast at the comparison site rather than changing cf_last_mode's type, which deliberately holds a -999 sentinel outside vm_mode_t's valid range), 2x -Wstringop-truncation (mkcapsule.c, strncpy+manual-null-terminate replaced with the idiomatic snprintf equivalent), and 26x -Wunused-parameter (mostly documented stubs, silenced with the repo's existing (void)param; idiom). One of the unused-parameter warnings was not a deliberate stub -- a real bug. restore_vm_state() (test_common.c) is named, documented, and called by nine real call sites (acl_words_test.c x8 plus its own internal use) as "restore saved VM state", but ignored all four of its parameters and hard-reset to a fixed baseline instead, silently not restoring what any caller actually saved. Fixed to actually assign the passed-in dsp/rsp/error/mode. Found while fixing warnings, reported before touching it, fixed/tested/documented/committed on explicit instruction. Verified: all three architectures build with zero C-compiler warnings (amd64: 3040 -> 0; aarch64's one remaining note is lld-link's own unrelated linker warning, not a C warning). Full amd64 acceptance boot post-fix: POST 1003/965/0/0/38 (total/passed/failed/errors/stubs), "ALL IMPLEMENTED TESTS PASSED!", contract checks (A4'/A1) all passed, dict_hash=0x24b4279f0670aa3a -- an exact match to this document's own previously-recorded baseline hash. .claude/CLAUDE.md corrected to describe the real -Wno-error= exemption list instead of the "-Wall -Werror" oversimplification. FABRIC-2.md Section J records the full sweep, including doc-tree staleness findings flagged but not fixed this pass (docs/lithosananke/ROADMAP.md branch topology, docs/03-architecture/word-acl/DESIGN.md's Phase 7 claim contradicting CLAUDE.md, top-level ROADMAP.md's stale StarForth-era status, the Isabelle pipeline-metrics model mismatch). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
bf59c4916e
commit
1a2ec565e8
+89
@@ -1328,3 +1328,92 @@ from June/July 2026 crashes. This fix explains and resolves the former. Whether
|
||||
was the same underlying SMC/HVC conduit issue (plausible — an illegal-instruction trap can
|
||||
manifest with different ESR encodings depending on exact CPU/QEMU state) or a genuinely
|
||||
separate bug was not re-investigated; nothing currently reproduces it to check against.
|
||||
|
||||
---
|
||||
|
||||
## J. Maintainability sweep — C99 warnings, doc-tree staleness, `restore_vm_state()` bug — 2026-08-18
|
||||
|
||||
Captain Bob's request, prompted by "this is getting hard to maintain": sweep for dead/
|
||||
unreachable code, C99 smells, `.thy` proof coverage, doc generation (LaTeX), experiment
|
||||
report toolchain (LaTeX/R/Python), and Doxygen coverage/wiring. Three parallel forks covered
|
||||
this; findings and fixes recorded here.
|
||||
|
||||
**`.claude/CLAUDE.md`'s "zero warnings" claim was wrong, corrected.** Documented as
|
||||
`-Wmissing-field-initializers` isolated to one file (`vocabulary_words_test.c`) — actual
|
||||
build (`make -f Makefile.starkernel ARCH=amd64`) showed **3,040 total warnings**, not one.
|
||||
Root cause: `TestCase` gained a trailing `contract` field (`WordContract`, item unrelated to
|
||||
this sweep) after all 20 files in `src/test_runner/modules/` had already written their
|
||||
compound-literal initializers; every single one — sentinels, real test entries, per-suite
|
||||
entries — omitted it. **Fixed mechanically across all 20 files**: added the missing `{0}`
|
||||
trailing initializer everywhere (several distinct literal shapes needed separate passes —
|
||||
single-line, nested double-brace sentinels, column-aligned whitespace, multi-line entries,
|
||||
one file using a macro instead of a literal for the `implemented` field, trailing `//`
|
||||
comments breaking an end-of-line anchor). Semantically a no-op (C99 already zero-fills
|
||||
unlisted trailing struct fields) — the fix only silences the diagnostic. `-Wmissing-field-
|
||||
initializers`: **3,010 → 0**.
|
||||
|
||||
**Remaining three warning classes, also fixed:**
|
||||
- **2× `-Wsign-compare`** (`src/word_source/control_words.c:127`) — `vm->mode` (`vm_mode_t`,
|
||||
an enum) compared against `cf_last_mode` (deliberately `int`, holding a `-999` sentinel
|
||||
outside the enum's valid range — its type could not be changed to match without losing
|
||||
that sentinel). Fixed with an explicit `(int)` cast at the comparison site, not a type
|
||||
change.
|
||||
- **2× `-Wstringop-truncation`** (`tools/mkcapsule.c`, two `strncpy`+manual-null-terminate
|
||||
call sites) — replaced with `snprintf(dest, N, "%s", src)`, the idiomatic warning-free
|
||||
equivalent of the same truncating, always-null-terminated copy. This file is a separate
|
||||
host build tool (`cc -Wall -Wextra -O2`, no `-Werror` at all) — these warnings were never
|
||||
actually gated by the kernel's zero-warnings policy, just genuinely unfixed until now.
|
||||
- **26× `-Wunused-parameter`**, scattered across `src/inference_engine.c`,
|
||||
`src/physics_pipelining_metrics.c`, `src/word_source/io_words.c`,
|
||||
`src/word_source/string_words.c`, `src/test_runner/test_runner.c`. All but one were
|
||||
documented, deliberate stubs/API-compatibility placeholders (comments already said so) —
|
||||
silenced with the repo's existing `(void)param;` idiom, no behavior change.
|
||||
|
||||
**One of those 26 was not a deliberate stub — a real bug, found, fixed, tested per explicit
|
||||
instruction.** `restore_vm_state()` (`src/test_runner/test_common.c`) takes `dsp`/`rsp`/
|
||||
`error`/`mode` parameters, is named and documented as restoring saved VM state, and has nine
|
||||
real call sites (`acl_words_test.c` ×8, plus its own internal use in this file) that all
|
||||
capture genuine pre-test state via `save_vm_state()` specifically so it can be restored
|
||||
afterward. **The function ignored all four parameters** and hard-reset to a fixed baseline
|
||||
(`dsp=-1, rsp=-1, error=0, mode=MODE_INTERPRET`) instead, silently not restoring what any
|
||||
caller actually saved. Fixed: the four fields are now assigned from the caller-supplied
|
||||
parameters, matching the function's own name, doc comment, and every caller's expectation.
|
||||
The rest of the function's unconditional cleanup (control-flow flags, compiling-word state)
|
||||
is untouched — those aren't part of the restore contract and have no corresponding
|
||||
parameters.
|
||||
|
||||
**Verified:** all three architectures (amd64/aarch64/riscv64) build with **zero C-compiler
|
||||
warnings** (aarch64 retains one unrelated `lld-link` linker-invocation note, not a C
|
||||
warning, pre-existing all session). Full amd64 acceptance boot post-fix: POST suite
|
||||
`1003 total, 965 passed, 0 failed, 0 errors, 38 stubs`, "ALL IMPLEMENTED TESTS PASSED!",
|
||||
contract checks (A4'/A1) all passed, `dict_hash=0x24b4279f0670aa3a` — an **exact match** to
|
||||
the baseline hash this document already recorded from a prior clean run (this section's own
|
||||
earlier `53 theories`/`54 theories` reconciliation entry cites the same hash) — strong
|
||||
evidence nothing regressed.
|
||||
|
||||
**Doc-tree findings from the same sweep, some fixed here, some flagged for later** (see
|
||||
`docs/CLAUDE.md`'s own correction, made in the same pass, for the doc-toolchain findings —
|
||||
`doxygen` installed, `docs/Makefile` pointer corrected — and this section's own proof-count
|
||||
reconciliation above for the Isabelle/HOL findings):
|
||||
- [ ] `docs/lithosananke/ROADMAP.md` and `M7.1.md` — stale `Branch: lithosananke` (no such
|
||||
branch exists post-split), `M7.1.md`'s "Status: Design Complete" (shipped and live, not
|
||||
just designed), `ROADMAP.md`'s self-contradiction (M8 marked OBSOLETE in one place,
|
||||
still a live success criterion in another), and its stale "AHCI driver" claim for M9
|
||||
(real implementation is `virtio_blk.c`) — not fixed this pass, flagged.
|
||||
- [ ] Top-level `ROADMAP.md` (StarForth-era, "Phase 0 Complete... Phase 1 Starting," dated
|
||||
2025-12-14) — badly stale, no historical/superseded banner to warn a reader. Not fixed.
|
||||
- [ ] `docs/03-architecture/word-acl/DESIGN.md` says ACL Phase 7 (LithosAnanke kernel parity)
|
||||
is still "remaining" — direct contradiction with `.claude/CLAUDE.md`, which states
|
||||
Phase 7 is independently verified complete. Not fixed.
|
||||
- [ ] `VM-FLEET-ATTRACTOR-DESIGN-20260705.md` claims `doe-campaign.4th` is "broken and being
|
||||
superseded" — unverified against this session's own repeated successful `L8-DOE` runs
|
||||
(a different FORTH entry point; not confirmed either way).
|
||||
- [ ] Isabelle/HOL: the pipeline-metrics model/C-struct mismatch this sweep surfaced (see
|
||||
this document's item 5.2 entry above) — flagged in the `.thy` file itself, not
|
||||
independently tracked elsewhere until this sweep, not fixed.
|
||||
|
||||
**Not covered by this pass, honestly:** non-static dead-function cross-referencing (needs
|
||||
symbol-by-symbol call-site verification across ~150+ C files, out of budget), C99 smell
|
||||
categories beyond warnings (magic numbers, function length, duplication), actually running
|
||||
`isabelle build` end-to-end (static inspection only), and the bulk of `docs/formal/`/
|
||||
`docs/patent/`/`docs/working/` beyond the specific files named above.
|
||||
|
||||
Reference in New Issue
Block a user