Stage C: primitive error-handling audit triage complete, 105 sites classified, no code changed (FABRIC-3.md §XXXII.3)
Read every one of the 105 vm->error=1 sites across the 8 kernel-only files in context (not sampled): 28 already correct (diagnostic before/ without erroring, e.g. defer_words.c's 15-for-15 gold-standard pattern), 75 silent (the exact USE-defect shape), 2 special cases requiring individual handling rather than a generic fix. Two findings flagged above the rest: SWITCH-MARK-WORK and VM-HEAT (mama_forth_words.c) each set vm->error on their own guard despite their own doc comments explicitly saying they must never error -- SWITCH-MARK-WORK is the same function whose off-by-one already caused a stray silent error to fire on every MSG-SEND once before (§XXVIII.3). And vm_core.c's four fundamental memory primitives (vm_load_u8/ vm_store_u8/vm_load_cell/vm_store_cell) silently fault on any out-of-bounds address -- the highest-reach fix candidates in the audit, hit by far more FORTH words than any single mama_forth_words.c site. Full per-file site list and per-category breakdown recorded for Stage D's own reference. Triage only -- no code changed this stage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BWpNjdwPtFLuVLaAq44L9K
This commit is contained in:
Robert Allan James
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
c05b70c8d6
commit
09959c6ca2
+92
@@ -4331,6 +4331,98 @@ primitives vs. the interpreter core vs. the REPL dispatch loop). Proposed shape:
|
||||
- `USE`'s own fix (§XXXII.1) is exempt from waiting on this -- already fully diagnosed, lands on
|
||||
its own as the worked example the triage pass can point back to.
|
||||
|
||||
**Stage C -- DONE, 2026-09-15. Triage complete, read-only, no code changed.** Surface recounted
|
||||
after Stages A/B's fixes (two `USE` sites, one `log_words.c` site changed shape): **105 sites**.
|
||||
Every site read in context, not sampled. Totals:
|
||||
|
||||
| File | (i) already correct | (ii) silent -- fix | (iii) flagged individually | Total |
|
||||
|---|---|---|---|---|
|
||||
| `mama_forth_words.c` | 1 | 48 | 0 | 49 |
|
||||
| `defer_words.c` | 15 | 0 | 0 | 15 |
|
||||
| `inference_words.c` | 0 | 3 | 0 | 3 |
|
||||
| `log_words.c` | 2 | 10 | 1 | 13 |
|
||||
| `vm_core.c` | 10 | 13 | 1 | 24 |
|
||||
| `repl.c` | 0 | 1 | 0 | 1 |
|
||||
| **Total** | **28** | **75** | **2** | **105** |
|
||||
|
||||
**`defer_words.c` is the gold-standard pattern already in this codebase** -- every one of its 15
|
||||
sites pairs `log_message(LOG_ERROR, ...)` with `vm->error = 1`, matching option (b)'s shape from
|
||||
§XXXII.1 (report, then fault) rather than USE's five siblings' shape (report, don't fault) --
|
||||
both are legitimate, the distinction is whether the condition is a usage mistake (don't fault,
|
||||
matching `USE`) or a genuine abort-worthy fault (fault, matching `defer_words.c`). Stage D's
|
||||
per-site fixes should pick whichever of the two this project's own precedent already establishes
|
||||
for that condition's *kind*, not apply one shape uniformly.
|
||||
|
||||
**`mama_forth_words.c` is overwhelmingly the same anti-pattern `USE` had**, copy-pasted since the
|
||||
file's earliest words: 46 of its 48 silent sites are one of exactly two boilerplate shapes --
|
||||
`if (vm->dsp < N) { vm->error = 1; return; }` (stack underflow) or
|
||||
`if (!p) { vm->error = 1; return; }` (`vm_ptr()` returned NULL, i.e. a bad address on the stack)
|
||||
-- across nearly every `S" name"`-taking word (`BIRTH`, `START`, `KILL`, `VM-STEP`, `VM-EXEC`,
|
||||
`VM-CALL`, `RUNCAP-TEST`, `PAIR-TEST`, `EXEC`, the four `CAPSULE*@` fetch words, `MINT`'s own
|
||||
`mint_pop_string()` helper, `ZUSE-ELIGIBILITY-ADD`, `ZUSE-ELIGIBLE?`, `NAME>XT`,
|
||||
`ELEVATE-PUBKEY-UNPACK`, `CAPSULE-BIRTH`, all eight `STADIUM-*` primitives). The remaining 2 are
|
||||
distinct conditions with the same silent shape: `CAPSULE-RUN`'s out-of-range capsule index
|
||||
(line 1369) and `mama_word_use()`'s own already-fixed pair (now category i, excluded from this
|
||||
count).
|
||||
|
||||
**Two findings worth flagging above the rest, not just more of the same pattern:**
|
||||
|
||||
1. **`SWITCH-MARK-WORK` (`mama_forth_words.c:800-824`) and `VM-HEAT` (`:928-960`) each set
|
||||
`vm->error` on their own stack-underflow/bad-address guards while their own doc comments say
|
||||
they must never error.** `SWITCH-MARK-WORK`'s comment: *"Silently ignores a malformed or
|
||||
unknown name -- this fires on the hot path of every message send, not a user-facing command,
|
||||
so it must never error or spam the console."* `VM-HEAT`'s: *"Silent on an unknown name...
|
||||
callers scanning many VM names each turn... should not have to filter console noise."* Both
|
||||
already handle the *expected* miss case silently (unknown name -> return/push 0, no error) --
|
||||
but their *first* guard (stack underflow / bad address, lines 807 and 936/948) still sets
|
||||
`vm->error`, contradicting the function's own stated contract. `SWITCH-MARK-WORK` is
|
||||
especially load-bearing here: it is exactly the function whose off-by-one (§XXVIII.3) already
|
||||
caused a stray `vm->error` to fire silently on every `MSG-SEND` in the system once before --
|
||||
the off-by-one is fixed, but the *shape* that let it go unnoticed (nothing downstream checks
|
||||
this specific error) is unchanged. Recommend these two guards drop `vm->error` entirely
|
||||
(return silently, matching the function's own documented contract) rather than gain a
|
||||
diagnostic (matching `USE`'s fix) -- the fix shape here is "stop erroring," not "add a
|
||||
message," since the function's own doc comment already rules out both console noise and
|
||||
faulting.
|
||||
2. **`vm_core.c`'s four fundamental memory-access primitives -- `vm_load_u8`/`vm_store_u8`/
|
||||
`vm_load_cell`/`vm_store_cell` (lines 1221/1232/1242/1254) -- silently fault on any
|
||||
out-of-bounds or misaligned address.** These are called far more often, and by far more
|
||||
FORTH words (`@ ! C@ C!` and everything built on them), than any single `mama_forth_words.c`
|
||||
site -- an ordinary interactive mistake like `999999 @` hits this silently today, with the
|
||||
exact same undiagnosed-halt consequence `USE` had. Highest-value fix candidates in the entire
|
||||
audit by call-site reach, not by how unusual the triggering condition is.
|
||||
|
||||
**The two category-(iii) sites, correctly left alone:**
|
||||
- `log_words.c`'s `log_word_append_raw()` (post-Stage-B, now checking `< 0`): a genuine write
|
||||
failure here cannot be reported via `log_message()` -- this function runs inside Artemis's own
|
||||
log-ring write path, and `log_region.c`'s own doc comment already establishes why nothing on
|
||||
this path may call `log_message()` (recursion into the same buffered flush). Fixing this one
|
||||
the way the rest of the audit will needs a different mechanism (`console_println`, if this
|
||||
path is ever reached interactively) or is accepted as a silent internal fault -- a Stage D
|
||||
decision, not a triage-time fix.
|
||||
- `vm_core.c`'s `vm_assert_interpreter_enabled()` (line 180-193): sets `vm->error` and logs
|
||||
(reversed order from the usual convention, but the diagnostic does print) immediately before
|
||||
calling `host->panic()` -- a genuine bootstrap-ordering invariant violation (the interpreter
|
||||
was invoked before bootstrap finished), not a usage mistake a diagnostic-and-recover fix would
|
||||
help with. Already correctly escalates past `vm->error` to a real panic; nothing to change.
|
||||
|
||||
**Ten more `vm_core.c` sites (498/508/521/528/557/567/597/721/730) are colon-definition
|
||||
compilation internals** (`:`/`;`/`IS`-adjacent dictionary-entry and threaded-body construction) --
|
||||
silent today, structurally rare to hit (most require a corrupted dictionary or an
|
||||
allocator-exhaustion condition rather than an ordinary typo), but still worth diagnostics in
|
||||
Stage D since a silent failure here would be exceptionally hard to debug blind.
|
||||
|
||||
**Full per-file site list (line numbers, current as of this pass) kept for Stage D's own
|
||||
reference rather than re-grepped from scratch:** `mama_forth_words.c` --
|
||||
96,118,140,162,243,258,350,365,561,576,664,676,725,744,752,807(flag 1),844,862,870,936(flag
|
||||
2),948(flag 2),980,983,986,1013,1079,1086,1110,1117,1137,1149,1169,1178,1205,1216,1255,1266,1321,
|
||||
1361,1369,1467,1479,1822,1857,1899,1919,1936,1958 (category i: 910). `defer_words.c` -- all
|
||||
category i (42,50,59,80,88,96,117,128,136,143,151,171,179,186,194). `inference_words.c` --
|
||||
77,82,142. `log_words.c` -- 48,78,80,109,114,127,133,217,223,224 (category ii), 172,180 (category
|
||||
i), 248 (category iii). `vm_core.c` -- 498,508,521,528,557,567,597,721,730,1221,1232,1242,1254
|
||||
(category ii), 547,573,586,604,836,877,1064,1083,1107,1267 (category i), 186 (category iii).
|
||||
`repl.c` -- 231.
|
||||
|
||||
### XXXII.4 -- Logging cleanup: two already-open gaps decided before the audit's fixes land
|
||||
|
||||
**Standing, already-flagged item** (`project_production_logging_cleanup_needed.md`):
|
||||
|
||||
Reference in New Issue
Block a user