Fix ABORT to actually unwind to QUIT instead of one level
ABORT is documented and tested in this codebase as standard FORTH-79 behavior -- system_words_test.c:63: "Should clear stacks and return to QUIT" -- meaning it should unwind all the way back to the outermost interpreter loop, abandoning whatever's left of the current line/block. The implementation only unwound one level: every place that checked vm->abort_requested cleared it the instant it saw it, so it never survived to propagate past the first nested frame. This surfaced via Artemis's ART-HALT-UNRECOG (capsules/artemis/init.4th): on an unrecognized disk it correctly printed "ARTEMIS HALT: unrecognized disk content" and called ABORT, but WELCOME (the next line in the same block) ran anyway, and Artemis announced ready to Hermes and joined the fleet normally -- contradicting .claude/ARTEMIS.md's "Refuse to mount... do not overwrite it" requirement. Root cause is general, not Artemis-specific, and present identically in both the hosted and kernel VM cores. Fixed at every level execution can nest through, verified by exhaustively grepping every !vm->error-gated continuation loop and adding the parallel !vm->abort_requested check: - execute_colon_word (src/vm.c, src/starkernel/vm/vm_core.c): stop clearing the flag on return -- every colon-word call is a recursive call to this same function, so leaving it set lets every enclosing frame's own check also unwind. - vm_interpret (src/vm.c, src/starkernel/vm/vm_core.c): stop parsing further words in the current input string once the flag is set. - exec_block_with_retry (src/starkernel/capsule/capsule_loader.c): capsule birth's line-by-line block executor -- stop processing further lines in the current block, but return 0 (not -1), so capsule_exec_payload still loads later blocks in the same capsule payload. Returning -1 here would have silently broken word definitions in blocks that come after the aborting one for reasons unrelated to why it aborted (concretely, Artemis's ART-PING/LOAD-DOE in blocks 4851/4852, which follow the entry block 4133). - THRU and --> (src/word_source/block_words.c): stop processing further blocks/lines in their own loops. - DODOES (src/word_source/defining_words.c): the CREATE...DOES> runtime has its own hand-rolled execution loop, separate from execute_colon_word -- same bug class, same fix. Also guarded the post-loop "if (vm->rsp < base_rsp) vm->rsp = base_rsp" clamp so it doesn't fire on an abort exit -- ABORT's own reset_vm_state() already set rsp; restoring it to base_rsp would have partially undone that. - Both REPL loops (src/repl.c, src/starkernel/repl.c x2 call sites): clear the flag after each line, mirroring the existing vm->error pattern, so a mid-line abort doesn't silently freeze subsequent interactive input. Verified directly: ": AB-TEST 1 2 3 ABORT 999 . ; AB-TEST 42 . CR 777 . CR" -- 999 never prints (stops mid-colon-word), 42 never prints (stops the rest of the same line), 777 prints fine (next line unaffected). Artemis: WELCOME/"Artemis ready" no longer fires after the halt message. No regression: all three architectures still show PASS: persist-read, PASS: E2E msg flow, and matching dict_hash on the normal (non-aborted) boot path; hosted test suite 965 passed / 0 failed. Known follow-up, not fixed here (see memory for details): Artemis still announces ready to Hermes via a separate call path (CD-INIT, block 4141) that never went through capsule_exec_payload's block chain in the first place, and the disk file still picks up incidental writes even on a correctly-halted boot -- likely generic block-subsystem housekeeping, not traced yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
3d9664d176
commit
cc6c8c43f3
@@ -113,6 +113,12 @@ void vm_repl(VM *vm, int script_mode) {
|
||||
|
||||
vm_interpret(vm, input);
|
||||
|
||||
/* ABORT stops mid-line (see vm_interpret/execute_colon_word) but the
|
||||
* flag is left set for the caller to consume -- this REPL line is
|
||||
* that boundary. Clear it here so the next line isn't silently
|
||||
* refused by vm_interpret's own abort_requested check. */
|
||||
vm->abort_requested = 0;
|
||||
|
||||
if (!script_mode) {
|
||||
/* Print status only in interactive mode */
|
||||
if (!vm->error) {
|
||||
|
||||
@@ -309,6 +309,23 @@ static int exec_block_with_retry(VM *vm, const uint8_t *block_start,
|
||||
|
||||
if (line_len > 0) {
|
||||
vm_interpret(vm, line);
|
||||
|
||||
/* ABORT: stop this block's remaining lines cleanly.
|
||||
* Not a failure (system_word_abort() clears vm->error),
|
||||
* so return 0 rather than -1 -- capsule_exec_payload
|
||||
* treats any non-zero return as fatal and would stop
|
||||
* loading the rest of the capsule payload, which would
|
||||
* silently break word definitions in later blocks that
|
||||
* have nothing to do with why this one aborted. */
|
||||
if (vm->abort_requested) {
|
||||
vm->abort_requested = 0;
|
||||
vm->error = 0;
|
||||
vm->mode = MODE_INTERPRET;
|
||||
vm->state_var = 0;
|
||||
vm_store_cell(vm, vm->state_addr, 0);
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (vm->error) {
|
||||
vm->error = 0;
|
||||
vm->mode = MODE_INTERPRET;
|
||||
|
||||
@@ -189,6 +189,11 @@ int sk_repl_step(VM *vm)
|
||||
|
||||
vm_interpret(vm, input);
|
||||
|
||||
/* ABORT stops mid-line but leaves the flag set for the caller to
|
||||
* consume -- this REPL step is that boundary. Clear it here so the
|
||||
* next line isn't silently refused by vm_interpret's own check. */
|
||||
vm->abort_requested = 0;
|
||||
|
||||
if (vm->error) {
|
||||
console_puts(" ERROR\n");
|
||||
vm->error = 0;
|
||||
@@ -237,6 +242,11 @@ void sk_repl_run(VM *vm)
|
||||
|
||||
vm_interpret(active, input);
|
||||
|
||||
/* ABORT stops mid-line but leaves the flag set for the caller to
|
||||
* consume -- this REPL step is that boundary. Clear it here so the
|
||||
* next line isn't silently refused by vm_interpret's own check. */
|
||||
active->abort_requested = 0;
|
||||
|
||||
if (active->error) {
|
||||
console_puts(" ERROR\n");
|
||||
active->error = 0;
|
||||
|
||||
@@ -811,10 +811,14 @@ void execute_colon_word(VM* vm)
|
||||
|
||||
if (vm->error) { vm->ecw_nesting--; return; }
|
||||
|
||||
/* Check for ABORT request (clears both stacks, immediate termination) */
|
||||
/* ABORT clears stacks and unwinds to QUIT: leave the flag set so
|
||||
* every enclosing colon-word frame (this function is recursive --
|
||||
* every colon word's func is execute_colon_word) also sees it and
|
||||
* also unwinds, instead of consuming it at the first frame that
|
||||
* notices. The outermost caller (vm_interpret et al.) is
|
||||
* responsible for finally clearing it. */
|
||||
if (vm->abort_requested)
|
||||
{
|
||||
vm->abort_requested = 0;
|
||||
vm->ecw_nesting--;
|
||||
return;
|
||||
}
|
||||
@@ -990,7 +994,11 @@ void vm_interpret(VM* vm, const char* input)
|
||||
vm->input_pos = 0;
|
||||
char word[64];
|
||||
size_t wlen;
|
||||
while (!vm->error && (wlen = (size_t)vm_parse_word(vm, word, sizeof(word))) > 0)
|
||||
/* !abort_requested: ABORT unwinds all the way to QUIT -- stop parsing
|
||||
* further words in this input the moment it's seen. Flag is left set
|
||||
* on return; caller (REPL, exec_block_with_retry, etc.) consumes it. */
|
||||
while (!vm->error && !vm->abort_requested &&
|
||||
(wlen = (size_t)vm_parse_word(vm, word, sizeof(word))) > 0)
|
||||
{
|
||||
vm_interpret_word(vm, word, wlen);
|
||||
|
||||
|
||||
@@ -626,8 +626,13 @@ void execute_colon_word(VM* vm)
|
||||
|
||||
if (vm->error) { vm->ecw_nesting--; return; }
|
||||
|
||||
/* ABORT clears stacks and returns immediately */
|
||||
if (vm->abort_requested) { vm->abort_requested = 0; vm->ecw_nesting--; return; }
|
||||
/* ABORT clears stacks and unwinds to QUIT: leave the flag set so
|
||||
* every enclosing colon-word frame (this function is recursive --
|
||||
* every colon word's func is execute_colon_word) also sees it and
|
||||
* also unwinds, instead of consuming it at the first frame that
|
||||
* notices. The outermost caller (vm_interpret et al.) is
|
||||
* responsible for finally clearing it. */
|
||||
if (vm->abort_requested) { vm->ecw_nesting--; return; }
|
||||
|
||||
/* EXIT discards saved IP and returns */
|
||||
if (vm->exit_colon)
|
||||
@@ -782,7 +787,11 @@ void vm_interpret(VM* vm, const char* input)
|
||||
|
||||
char word[64];
|
||||
size_t wlen;
|
||||
while (!vm->error && (wlen = (size_t)vm_parse_word(vm, word, sizeof(word))) > 0)
|
||||
/* !abort_requested: ABORT unwinds all the way to QUIT -- stop parsing
|
||||
* further words in this input the moment it's seen. Flag is left set
|
||||
* on return; caller (REPL, exec_block_with_retry, etc.) consumes it. */
|
||||
while (!vm->error && !vm->abort_requested &&
|
||||
(wlen = (size_t)vm_parse_word(vm, word, sizeof(word))) > 0)
|
||||
{
|
||||
vm_interpret_word(vm, word, wlen);
|
||||
}
|
||||
|
||||
@@ -397,6 +397,7 @@ void block_word_thru(VM *vm) {
|
||||
vm_push(vm, blk);
|
||||
block_word_load(vm);
|
||||
if (vm->error) return;
|
||||
if (vm->abort_requested) return;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -431,7 +432,7 @@ void block_word_next_block(VM *vm) {
|
||||
block_text[1024] = '\0';
|
||||
|
||||
char *p = block_text;
|
||||
while (!vm->error && *p != '\0') {
|
||||
while (!vm->error && !vm->abort_requested && *p != '\0') {
|
||||
char *nl = (char *)memchr(p, '\n', (size_t)(block_text + 1024 - p));
|
||||
if (nl) {
|
||||
*nl = '\0';
|
||||
|
||||
@@ -262,7 +262,7 @@ static void defining_runtime_dodoes(VM *vm) {
|
||||
vm->return_stack[vm->rsp] = (cell_t)(uintptr_t)
|
||||
ip;
|
||||
|
||||
while (!vm->error && !vm->exit_colon && vm->rsp >= base_rsp + 1) {
|
||||
while (!vm->error && !vm->abort_requested && !vm->exit_colon && vm->rsp >= base_rsp + 1) {
|
||||
cell_t *cur_ip = (cell_t *) (uintptr_t) vm->return_stack[vm->rsp];
|
||||
DictEntry *entry = (DictEntry *) (uintptr_t)(*cur_ip++);
|
||||
vm->return_stack[vm->rsp] = (cell_t)(uintptr_t)
|
||||
@@ -287,7 +287,11 @@ static void defining_runtime_dodoes(VM *vm) {
|
||||
vm->exit_colon = 0;
|
||||
}
|
||||
|
||||
if (vm->rsp < base_rsp) vm->rsp = base_rsp;
|
||||
/* ABORT already reset rsp (reset_vm_state(), called synchronously by
|
||||
* system_word_abort() before this loop even notices the flag) --
|
||||
* restoring it to base_rsp here would partially undo that reset.
|
||||
* Only clamp back up to base_rsp on a normal/EXIT completion. */
|
||||
if (!vm->abort_requested && vm->rsp < base_rsp) vm->rsp = base_rsp;
|
||||
}
|
||||
|
||||
/* does_rt: runs inside the defining word at DOES> time.
|
||||
|
||||
Reference in New Issue
Block a user