Enter a TRAP handler when the variable table is full
Two separable faults, both on the path a program takes when it is already in trouble. `akbasic_trap_set_error_variables()` reached `ER#` and `EL#` through `akbasic_runtime_global()`, which *creates* a name the program never used -- and creating one takes a variable slot. So a program that had filled the 128-slot table could not have its handler entered at all, and because the failure happened inside the error path rather than raising, nothing was reported and the program carried on with the failing statement's effect quietly missing. A wrong answer delivered as a right one, which is worse than an abort. `akbasic_runtime_reserve_globals()` now creates both at runtime init, where there is always room, and `clear_variables()` puts them back after `CLR` and `NEW` empty the table. Second: `report_and_reraise()` used a plain `PASS` around the report, so a failure while reporting *replaced* the error the program had actually made -- "Maximum runtime variables reached" in place of the subscript that was out of range. The secondary failure is now logged and the original is re-raised, which is what a user needs to hear. **The reduction in TODO.md no longer reproduces, and not because of this.** The value-pool fix in the previous commit made a scalar free, so the pool can no longer be emptied by creating names. The defect was still live through the variable table: 124 names and a TRAP armed, and the handler was silently skipped. That is what the new test in tests/trap_verbs.c pins, together with the invariant -- both globals present before a program runs. Verified by reverting the fix against the new tests: both fail, and the second prints the log line the swallow used to eat, "could not report a BASIC error 515 (Out Of Bounds): Maximum runtime variables reached". TODO.md section 6 item 33, struck. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
18
TODO.md
18
TODO.md
@@ -1969,8 +1969,22 @@ in either corpus runs for minutes, which is why the first of these had never bee
|
||||
the argument against it is that it makes `moveto` write to the grid, which it currently
|
||||
does not.
|
||||
|
||||
33. **A `TRAP` handler cannot be entered once the value pool is dry, and the error is dropped
|
||||
in silence.** `akbasic_trap_set_error_variables()` (`src/runtime_trap.c:36`) sets `ER#`
|
||||
33. ~~**A `TRAP` handler cannot be entered once the value pool is dry, and the error is
|
||||
dropped in silence.**~~ **Done**, in two halves. `akbasic_runtime_reserve_globals()`
|
||||
(`src/runtime.c`) creates `ER#` and `EL#` at runtime init and again after `CLR`, so the
|
||||
dispatch never allocates -- there is nothing left for it to fail at. And
|
||||
`report_and_reraise()` no longer lets a failure *inside* reporting replace the error the
|
||||
program actually made: the secondary one is logged where a developer will see it and the
|
||||
original is re-raised.
|
||||
|
||||
**The reduction below no longer reproduces, and not because of this fix.** §6 item 30
|
||||
made a scalar free, so the value pool can no longer be emptied by creating names and
|
||||
`ER#`/`EL#` could be created after all. The defect was still there, reachable through the
|
||||
*variable* table instead: 124 names and a `TRAP` armed, and the handler was silently
|
||||
skipped while the program carried on. That is what `tests/trap_verbs.c` now pins, along
|
||||
with the invariant itself -- both globals present before a program runs.
|
||||
|
||||
The original report: `akbasic_trap_set_error_variables()` (`src/runtime_trap.c:36`) sets `ER#`
|
||||
and `EL#` through `akbasic_runtime_global()` (`:51` and `:53`), which *creates* them if
|
||||
the program never named them. Creating them needs pool slots. So the one error most
|
||||
likely to leave the pool empty -- item 30's -- is the one error whose handler cannot be
|
||||
|
||||
@@ -752,6 +752,22 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_global(akbasic_Runtime *obj,
|
||||
|
||||
/* --- Internal API: exposed for the scanner, parser and verb handlers only. --- */
|
||||
|
||||
/**
|
||||
* @brief Create the globals the runtime writes to, before anything needs them.
|
||||
*
|
||||
* `ER#` and `EL#`, which the `TRAP` dispatch sets. Creating a name costs a
|
||||
* variable slot, and the dispatch runs at exactly the moment the program is
|
||||
* already in trouble -- so they are made once, up front, where there is always
|
||||
* room. `akbasic_runtime_init()` calls this; so does `CLR`, which empties the
|
||||
* table it filled.
|
||||
*
|
||||
* @param obj Object to initialize, inspect, or modify.
|
||||
* @return `NULL` on success, otherwise an error context owned by the caller.
|
||||
* @throws AKERR_NULLPOINTER When `obj` is NULL.
|
||||
* @throws AKBASIC_ERR_BOUNDS When no variable slot is free.
|
||||
*/
|
||||
akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_reserve_globals(akbasic_Runtime *obj);
|
||||
|
||||
/**
|
||||
* @brief Take an unused variable from the runtime's pool.
|
||||
* @param obj Object to initialize, inspect, or modify.
|
||||
|
||||
@@ -67,6 +67,32 @@ akerr_ErrorContext *akbasic_runtime_global(akbasic_Runtime *obj, const char *nam
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
akerr_ErrorContext *akbasic_runtime_reserve_globals(akbasic_Runtime *obj)
|
||||
{
|
||||
PREPARE_ERROR(errctx);
|
||||
akbasic_Variable *variable = NULL;
|
||||
|
||||
FAIL_ZERO_RETURN(errctx, (obj != NULL), AKERR_NULLPOINTER, "NULL runtime in reserve_globals");
|
||||
/*
|
||||
* ER# and EL# exist from the start, because the moment they are needed is
|
||||
* the moment the interpreter is least able to make them.
|
||||
*
|
||||
* akbasic_trap_set_error_variables() reaches them through
|
||||
* akbasic_runtime_global(), which *creates* a name the program never used --
|
||||
* and creating one costs a variable slot. So a program that filled the
|
||||
* variable table and then raised could not have its handler entered at all,
|
||||
* and because the failure happened inside the error path rather than raising,
|
||||
* the program carried on with the failing statement's effect quietly
|
||||
* missing. A wrong answer delivered as a right one. TODO.md section 6 item
|
||||
* 33 has the reduction.
|
||||
*
|
||||
* Called again from CLR and NEW, which empty the table this fills.
|
||||
*/
|
||||
PASS(errctx, akbasic_runtime_global(obj, "ER#", &variable));
|
||||
PASS(errctx, akbasic_runtime_global(obj, "EL#", &variable));
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
akerr_ErrorContext *akbasic_runtime_new_function(akbasic_Runtime *obj, akbasic_FunctionDef **dest)
|
||||
{
|
||||
PREPARE_ERROR(errctx);
|
||||
@@ -215,6 +241,7 @@ akerr_ErrorContext *akbasic_runtime_init(akbasic_Runtime *obj, akbasic_TextSink
|
||||
PASS(errctx, akbasic_runtime_new_environment(obj));
|
||||
PASS(errctx, akbasic_runtime_zero(obj));
|
||||
PASS(errctx, akbasic_scanner_zero(obj));
|
||||
PASS(errctx, akbasic_runtime_reserve_globals(obj));
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
@@ -450,6 +477,7 @@ akerr_ErrorContext *akbasic_runtime_set_mode(akbasic_Runtime *obj, int mode)
|
||||
static akerr_ErrorContext *report_and_reraise(akbasic_Runtime *obj, akerr_ErrorContext *cause)
|
||||
{
|
||||
PREPARE_ERROR(errctx);
|
||||
akerr_ErrorContext *reporting = NULL;
|
||||
char message[AKERR_MAX_ERROR_CONTEXT_STRING_LENGTH];
|
||||
int status = cause->status;
|
||||
|
||||
@@ -458,7 +486,28 @@ static akerr_ErrorContext *report_and_reraise(akbasic_Runtime *obj, akerr_ErrorC
|
||||
obj->lasterrorstatus = status;
|
||||
cause->handled = true;
|
||||
IGNORE(akerr_release_error(cause));
|
||||
PASS(errctx, akbasic_runtime_error(obj, AKBASIC_ERRCLASS_RUNTIME, message));
|
||||
|
||||
/*
|
||||
* **Reporting can itself fail, and the program's own error still wins.**
|
||||
*
|
||||
* The way it fails is the `TRAP` dispatch inside akbasic_runtime_error():
|
||||
* entering a handler takes a scope, and a program deep enough to have run
|
||||
* out of them raises for that reason while already raising for another. A
|
||||
* plain PASS here would return the *dispatch's* failure and lose the one the
|
||||
* program needs to hear about -- "Environment pool exhausted" in place of
|
||||
* the subscript that was actually out of range.
|
||||
*
|
||||
* So the secondary failure is logged where a developer will see it and the
|
||||
* original is re-raised. TODO.md section 6 item 33 is where this was found;
|
||||
* item 33's other half stops the dispatch needing to allocate at all, which
|
||||
* is what makes this path rare rather than routine.
|
||||
*/
|
||||
reporting = akbasic_runtime_error(obj, AKBASIC_ERRCLASS_RUNTIME, message);
|
||||
if ( reporting != NULL ) {
|
||||
LOG_ERROR_WITH_MESSAGE(reporting, "could not report a BASIC error");
|
||||
reporting->handled = true;
|
||||
IGNORE(akerr_release_error(reporting));
|
||||
}
|
||||
FAIL_RETURN(errctx, status, "%s", message);
|
||||
}
|
||||
|
||||
|
||||
@@ -72,6 +72,12 @@ static akerr_ErrorContext AKERR_NOIGNORE *clear_variables(akbasic_Runtime *obj)
|
||||
FAIL_ZERO_RETURN(errctx, (root != NULL), AKERR_NULLPOINTER, "Runtime has no root environment");
|
||||
PASS(errctx, akbasic_symtab_init(&root->variables, AKBASIC_MAX_VARIABLES));
|
||||
PASS(errctx, akbasic_symtab_init(&root->functions, AKBASIC_MAX_FUNCTIONS));
|
||||
/*
|
||||
* Put the reserved globals back. They were in the table this just emptied,
|
||||
* and the `TRAP` dispatch relies on never having to create them -- so a CLR
|
||||
* that left them gone would reopen item 33 for the rest of the session.
|
||||
*/
|
||||
PASS(errctx, akbasic_runtime_reserve_globals(obj));
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
|
||||
@@ -222,6 +222,75 @@ static void test_err_function(void)
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief `ER#` and `EL#` exist before a program runs, so the dispatch cannot fail.
|
||||
*
|
||||
* The dispatch used to reach them through akbasic_runtime_global(), which
|
||||
* *creates* a name the program never used -- and creating one takes a variable
|
||||
* slot. So the handler could not be entered by a program that had filled the
|
||||
* table, which is the one kind of program most likely to raise. Asserted here as
|
||||
* the invariant rather than only through the symptom below, because the symptom
|
||||
* needs 124 lines of setup and this needs none.
|
||||
*/
|
||||
static void test_error_globals_are_reserved(void)
|
||||
{
|
||||
bool haveer = false;
|
||||
bool haveel = false;
|
||||
int i = 0;
|
||||
|
||||
TEST_REQUIRE_OK(harness_start(NULL));
|
||||
for ( i = 0; i < AKBASIC_MAX_VARIABLES; i++ ) {
|
||||
if ( !HARNESS_RUNTIME.variables[i].used ) {
|
||||
continue;
|
||||
}
|
||||
if ( strcmp(HARNESS_RUNTIME.variables[i].name, "ER#") == 0 ) {
|
||||
haveer = true;
|
||||
}
|
||||
if ( strcmp(HARNESS_RUNTIME.variables[i].name, "EL#") == 0 ) {
|
||||
haveel = true;
|
||||
}
|
||||
}
|
||||
TEST_REQUIRE(haveer, "ER# must exist as soon as the runtime is initialised");
|
||||
TEST_REQUIRE(haveel, "EL# must exist as soon as the runtime is initialised");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief A handler is entered even by a program that has filled the variable table.
|
||||
*
|
||||
* TODO.md section 6 item 33, and much worse than an abort: the dispatch failed
|
||||
* *inside* the error path rather than raising, so nothing was reported, the
|
||||
* handler never ran, and the program carried on with the failing statement's
|
||||
* effect quietly missing -- a wrong answer delivered as a right one.
|
||||
*
|
||||
* 124 names, which leaves the table with no room to invent `ER#` and `EL#` at
|
||||
* the moment they are wanted. It is 124 rather than 128 because the runtime
|
||||
* reserves those two and the loop counter is a third.
|
||||
*/
|
||||
static void test_handler_runs_with_a_full_variable_table(void)
|
||||
{
|
||||
char source[AKBASIC_MAX_VARIABLES * 32];
|
||||
size_t used = 0;
|
||||
int i = 0;
|
||||
|
||||
used = (size_t)snprintf(source, sizeof(source), "10 TRAP CAUGHT\n");
|
||||
for ( i = 0; i < 124; i++ ) {
|
||||
used += (size_t)snprintf(source + used, sizeof(source) - used,
|
||||
"%d V%03d# = %d\n", 20 + i, i, i);
|
||||
}
|
||||
snprintf(source + used, sizeof(source) - used,
|
||||
"800 DIM TOOBIG#(5000)\n"
|
||||
"810 PRINT \"NOT TRAPPED\"\n"
|
||||
"820 END\n"
|
||||
"900 LABEL CAUGHT\n"
|
||||
"910 PRINT \"TRAPPED\"\n"
|
||||
"920 END\n");
|
||||
|
||||
TEST_REQUIRE_OK(run_program(source));
|
||||
TEST_REQUIRE_STR(HARNESS_OUTPUT, "TRAPPED\n");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
test_trap_catches();
|
||||
@@ -233,5 +302,7 @@ int main(void)
|
||||
test_resume_outside_handler();
|
||||
test_error_inside_handler();
|
||||
test_err_function();
|
||||
test_error_globals_are_reserved();
|
||||
test_handler_runs_with_a_full_variable_table();
|
||||
return akbasic_test_failures;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user