Stage D batch 3: fix silent error sites in vm_core.c, incl. the two highest-value primitives; two real NULL-deref bugs found and fixed (FABRIC-3.md §XXXII.6)
All 13 silent vm->error=1 sites in vm_core.c now log a diagnostic
first: vm_enter_compile_mode, vm_compile_word, vm_compile_literal,
vm_compile_call, vm_exit_compile_mode, execute_colon_word (2 sites
each/combined), and the four fundamental memory primitives
vm_load_u8/vm_store_u8/vm_load_cell/vm_store_cell.
Found and fixed two real NULL-pointer-dereference risks while adding
the diagnostics: vm_compile_call() and vm_exit_compile_mode() each had
a combined `if (!vm || <cond>) { vm->error = 1; ... }` guard that
dereferenced vm->error even on the !vm branch of its own condition.
Split both, and applied the same defensive split to the four memory
primitives since vm_ptr()/vm_addr_ok() both tolerate vm==NULL
internally.
Live-verified, amd64: 999999999 @ . recovers correctly at Zuse's own
console (Stage A's mechanism holds), but the new vm_load_cell
diagnostic itself didn't print -- traced to memory_words.c's own
redundant, still-silent vm_addr_ok() pre-check in memory_word_fetch()
(and the same shape in memory_word_store()), which intercepts before
ever reaching vm_load_cell(). memory_words.c is vendored, out of this
initiative's scope, spun off to FABRIC-4.md -- recorded as a concrete
cross-reference for that future work rather than left to be
rediscovered.
Three-arch clean qemu acceptance passed, full POST suite included.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BWpNjdwPtFLuVLaAq44L9K
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
1a8c0e4fcf
commit
66c2f3e539
+38
@@ -4567,3 +4567,41 @@ regression unaffected. Not separately live-fired against any of the 11 sites' sp
|
||||
conditions -- same reasoning as batch 1, all fixes are mechanical diagnostic additions with no
|
||||
logic change.
|
||||
|
||||
**Batch 3 -- DONE, 2026-09-15: `vm_core.c` (13 silent sites), including the two highest-value
|
||||
fixes named in §XXXII.3.** All 13 gained a `log_message(LOG_ERROR, ...)` diagnostic:
|
||||
`vm_enter_compile_mode()` (2), `vm_compile_word()` (2), `vm_compile_literal()`'s `vm_allot`
|
||||
guard (1), `vm_compile_call()`'s mode check (1), `vm_exit_compile_mode()`'s compiling-word check
|
||||
(1), `execute_colon_word()`'s data-field/body-address checks (2), and the four fundamental
|
||||
memory primitives `vm_load_u8`/`vm_store_u8`/`vm_load_cell`/`vm_store_cell` (4).
|
||||
|
||||
**Two real, distinct bugs found and fixed while adding the diagnostics, not just missing
|
||||
messages.** `vm_compile_call()` and `vm_exit_compile_mode()` each had a combined
|
||||
`if (!vm || <other condition>) { vm->error = 1; ... }` guard that dereferenced `vm->error` even
|
||||
on the `!vm` branch of their own `||` -- a real NULL-pointer-dereference risk if either was ever
|
||||
called with `vm == NULL`, found only because adding a proper diagnostic required splitting the
|
||||
condition apart to know which branch actually fired. Both now check `!vm` first and return
|
||||
bare, matching every sibling function in this file's own existing convention. Applied the same
|
||||
split preemptively to the four memory primitives (`vm_ptr()`/`vm_addr_ok()` both tolerate
|
||||
`vm == NULL` internally, so the identical risk existed there too, unexercised until now only
|
||||
because no caller has ever actually passed a NULL `vm`).
|
||||
|
||||
**Live-verified, amd64, and an honest scope-boundary finding recorded rather than glossed
|
||||
over.** `999999999 @ .` at Zuse's own console: recovers correctly (`VM fault -- session
|
||||
recovered, resuming`, console fully interactive afterward, `5 6 + .` computes `11` immediately
|
||||
next) -- Stage A's mechanism holds. **But the new `vm_load_cell` diagnostic itself did not
|
||||
print.** Traced, not guessed: `memory_words.c`'s `memory_word_fetch()` (backing `@`) does its
|
||||
own redundant `vm_addr_ok()` bounds check and sets `vm->error = 1` silently *before* ever
|
||||
calling `vm_load_cell()` -- confirmed the same shape exists in `memory_word_store()` (`!`) too.
|
||||
`memory_words.c` is vendored, shared `word_source/`, explicitly out of this initiative's scope
|
||||
(§XXXII.3) and spun off to `FABRIC-4.md`. This is not a defect in this batch's fix -- the
|
||||
four primitives are correctly diagnosed now for any caller that reaches their own guard -- but
|
||||
it means the *practical* reach of this batch's highest-value fix, for the specific everyday
|
||||
words `@`/`!` (and likely `C@`/`C!`/`2@`, same file, not individually checked), stays zero
|
||||
until `FABRIC-4.md` fixes `memory_words.c`'s own identical, still-silent pre-checks. Recorded
|
||||
here so `FABRIC-4.md` starts with this concrete cross-reference rather than rediscovering it.
|
||||
|
||||
**Verified:** three-architecture `clean qemu` acceptance (amd64/aarch64/riscv64) passed, standard
|
||||
regression unaffected -- including the full POST word-test suite, which exercises
|
||||
`vm_compile_word`/`vm_compile_call`/`execute_colon_word` extensively via every colon-definition
|
||||
test case, with zero regressions from either the diagnostics or the two NULL-guard fixes.
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
# Capsule Block Manifest — Auto-generated
|
||||
<!-- Generated by mkcapsule --manifest 2026-09-15T20:43:41Z -->
|
||||
<!-- Generated by mkcapsule --manifest 2026-09-15T21:25:23Z -->
|
||||
<!-- DO NOT EDIT — re-run mkcapsule --manifest to refresh. -->
|
||||
<!-- Hand-written justifications and immutability notes live -->
|
||||
<!-- in MANIFEST.md alongside this auto-generated index. -->
|
||||
|
||||
Binary file not shown.
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -495,6 +495,7 @@ void vm_enter_compile_mode(VM* vm, const char* name, size_t len)
|
||||
vm->compiling_word = de;
|
||||
if (!de)
|
||||
{
|
||||
log_message(LOG_ERROR, ": vm_create_word failed for '%.*s'", (int)len, name);
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -505,6 +506,7 @@ void vm_enter_compile_mode(VM* vm, const char* name, size_t len)
|
||||
cell_t* df = vm_dictionary_get_data_field(de);
|
||||
if (!df)
|
||||
{
|
||||
log_message(LOG_ERROR, ": no data field for '%.*s'", (int)len, name);
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -518,6 +520,7 @@ void vm_compile_word(VM* vm, DictEntry* entry)
|
||||
if (!vm || vm->mode != MODE_COMPILE) return;
|
||||
if (!entry)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_compile_word: NULL entry");
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -525,6 +528,7 @@ void vm_compile_word(VM* vm, DictEntry* entry)
|
||||
cell_t* slot = (cell_t*)vm_allot(vm, sizeof(cell_t));
|
||||
if (!slot)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_compile_word: vm_allot failed");
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -554,6 +558,7 @@ void vm_compile_literal(VM* vm, cell_t value)
|
||||
cell_t* val = (cell_t*)vm_allot(vm, sizeof(cell_t));
|
||||
if (!val)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_compile_literal: vm_allot failed");
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -562,8 +567,15 @@ void vm_compile_literal(VM* vm, cell_t value)
|
||||
|
||||
void vm_compile_call(VM* vm, word_func_t func)
|
||||
{
|
||||
if (!vm || vm->mode != MODE_COMPILE)
|
||||
/* FABRIC-3.md §XXXII.6, 2026-09-15: split from the original combined
|
||||
* `if (!vm || vm->mode != MODE_COMPILE) { vm->error = 1; ... }` guard
|
||||
* -- that form dereferenced vm->error even on the !vm branch of its
|
||||
* own condition, a real NULL-pointer-deref risk found while adding
|
||||
* this diagnostic, not just a missing message. */
|
||||
if (!vm) return;
|
||||
if (vm->mode != MODE_COMPILE)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_compile_call: not in compile mode");
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -592,8 +604,12 @@ void vm_compile_exit(VM* vm)
|
||||
|
||||
void vm_exit_compile_mode(VM* vm)
|
||||
{
|
||||
if (!vm || !vm->compiling_word)
|
||||
/* Split from the original `if (!vm || !vm->compiling_word)` guard --
|
||||
* same NULL-deref risk as vm_compile_call() above (§XXXII.6). */
|
||||
if (!vm) return;
|
||||
if (!vm->compiling_word)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_exit_compile_mode: no compiling word (unbalanced ';'?)");
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -718,6 +734,8 @@ void execute_colon_word(VM* vm)
|
||||
cell_t* df = vm_dictionary_get_data_field(entry);
|
||||
if (!df)
|
||||
{
|
||||
log_message(LOG_ERROR, "execute_colon_word: no data field for '%.*s'",
|
||||
(int)entry->name_len, entry->name);
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -727,6 +745,8 @@ void execute_colon_word(VM* vm)
|
||||
cell_t* ip = (cell_t*)vm_ptr(vm, body_addr);
|
||||
if (!ip)
|
||||
{
|
||||
log_message(LOG_ERROR, "execute_colon_word: bad body address for '%.*s'",
|
||||
(int)entry->name_len, entry->name);
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -1213,11 +1233,24 @@ uint8_t* vm_ptr(struct VM* vm, vaddr_t addr)
|
||||
return vm->memory + (size_t)addr;
|
||||
}
|
||||
|
||||
/* FABRIC-3.md §XXXII.6, 2026-09-15: all four of these previously
|
||||
* dereferenced vm->error after a failed vm_ptr()/vm_addr_ok() call --
|
||||
* both of those tolerate vm==NULL internally (returning NULL/0), so a
|
||||
* NULL vm reaching any of these four would have crashed on the error
|
||||
* report itself, the same NULL-deref shape already found and fixed in
|
||||
* vm_compile_call()/vm_exit_compile_mode() above. Each now guards `!vm`
|
||||
* first, and reports the genuinely common case -- a bad/misaligned
|
||||
* address from an ordinary interactive `@ ! C@ C!` -- with a diagnostic
|
||||
* these four calls' sheer reach (every FORTH word built on them) makes
|
||||
* the highest-value fix in this whole audit (§XXXII.3). */
|
||||
|
||||
uint8_t vm_load_u8(struct VM* vm, vaddr_t addr)
|
||||
{
|
||||
if (!vm) return 0;
|
||||
uint8_t* p = vm_ptr(vm, addr);
|
||||
if (!p)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_load_u8: bad address %lu", (unsigned long)addr);
|
||||
vm->error = 1;
|
||||
return 0;
|
||||
}
|
||||
@@ -1226,9 +1259,11 @@ uint8_t vm_load_u8(struct VM* vm, vaddr_t addr)
|
||||
|
||||
void vm_store_u8(struct VM* vm, vaddr_t addr, uint8_t v)
|
||||
{
|
||||
if (!vm) return;
|
||||
uint8_t* p = vm_ptr(vm, addr);
|
||||
if (!p)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_store_u8: bad address %lu", (unsigned long)addr);
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
@@ -1237,8 +1272,10 @@ void vm_store_u8(struct VM* vm, vaddr_t addr, uint8_t v)
|
||||
|
||||
cell_t vm_load_cell(struct VM* vm, vaddr_t addr)
|
||||
{
|
||||
if (!vm) return 0;
|
||||
if (!vm_addr_ok(vm, addr, sizeof(cell_t)) || (addr % sizeof(cell_t)) != 0)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_load_cell: bad/misaligned address %lu", (unsigned long)addr);
|
||||
vm->error = 1;
|
||||
return 0;
|
||||
}
|
||||
@@ -1249,8 +1286,10 @@ cell_t vm_load_cell(struct VM* vm, vaddr_t addr)
|
||||
|
||||
void vm_store_cell(struct VM* vm, vaddr_t addr, cell_t v)
|
||||
{
|
||||
if (!vm) return;
|
||||
if (!vm_addr_ok(vm, addr, sizeof(cell_t)) || (addr % sizeof(cell_t)) != 0)
|
||||
{
|
||||
log_message(LOG_ERROR, "vm_store_cell: bad/misaligned address %lu", (unsigned long)addr);
|
||||
vm->error = 1;
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user