Stop a scalar created inside a scope costing value-pool slots
A scalar now lives in the variable record (`akbasic_Variable::inlinevalue`) rather than drawing from the value pool, so a `GOSUB` local, a `FOR` counter and a `DEF` parameter cost nothing at all. The pool is a bump allocator with no free, and its comment justified that with "nothing in BASIC destroys a variable". Scope exit does: it marks the variable slot unused, `new_variable()` memsets the slot it hands back -- clearing `values` -- and `variable_init()` therefore took *fresh* slots for a variable whose old ones were still counted. Every scope that created a local leaked, with no diagnostic until the pool ran dry on whichever line happened to be unlucky. Six thousand `GOSUB`s creating one local used to die on the 4091st at `LOC# = 1` with "Array of 1 elements does not fit in the 0 remaining value slots". They now run. A `DEF` called eight thousand times used to die between the four and five thousandth -- the leaking slot was the call scope's parameter, which is a scalar -- and both forms now run. A game creating one name per tick was dead in half a minute; the Breakout in examples/ was, after twenty-five seconds. **A `@` name is the one exclusion, and it is the whole of it.** A structure or a pointer to one keeps pool storage, because a pointer into a record outlives the scope that DIMmed it -- docs/16-structures.md says nothing is reclaimed and `prev_environment()` relies on it. The name suffix is the right test rather than `structtype`, which the DIM path sets *after* calling `variable_init()`. A local array therefore still leaks, deliberately, and is now the narrow rule the tutorial teaches. `SWAP` needed the other half: it copies whole variable records, so the `values` pointer that came over named the other variable's inline slot -- which by then held this variable's own old value -- and SWAP silently did nothing. Caught by tests/language/housekeeping/verbs.bas, which is the golden corpus earning its keep. tests/value_pool.c is the new coverage. It asserts the mechanism as well as the consequence: a later change that moved arrays inline too would pass every behavioural case and quietly break the pointer guarantee. The sharpest case takes the pool's whole 4096 slots in four arrays after two hundred scope entries, so one leaked slot has nowhere to go. Chapter 17 Step 3 taught "declare every name at the top" and no longer needs to. It now teaches what is still true -- a name first seen inside a subroutine dies at RETURN, so a routine cannot answer its caller through one -- and its demonstration is the array case, which still fails. TODO.md section 6 item 30 and section 9 item 1, both struck. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -305,6 +305,7 @@ set(AKBASIC_TESTS
|
||||
value_arithmetic
|
||||
value_bitwise
|
||||
value_compare
|
||||
value_pool
|
||||
variable_subscript
|
||||
verbs_table
|
||||
version_check
|
||||
|
||||
25
TODO.md
25
TODO.md
@@ -1863,8 +1863,22 @@ the reference's semantics reproduced faithfully; the third is the port's own.
|
||||
A complete Breakout, sprites and text grid and keyboard, running for minutes at a time. Nothing
|
||||
in either corpus runs for minutes, which is why the first of these had never been seen.
|
||||
|
||||
30. **A variable created inside a scope costs pool slots that never come back, so a long-running
|
||||
program has a hard budget of 4096 name creations.** `akbasic_ValuePool` is a bump allocator
|
||||
30. ~~**A variable created inside a scope costs pool slots that never come back, so a
|
||||
long-running program has a hard budget of 4096 name creations.**~~ **Done** — a scalar now
|
||||
lives in the variable rather than in the pool (`akbasic_Variable::inlinevalue`,
|
||||
`src/variable.c`), so a `GOSUB` local, a `FOR` counter and a `DEF` parameter all cost
|
||||
nothing at all. Twenty thousand scoped creations where four thousand used to be the whole
|
||||
run. `tests/value_pool.c` is the coverage, and it asserts the mechanism as well as the
|
||||
consequence — a later change that moved arrays inline too would pass every behavioural
|
||||
case and break the pointer guarantee. **An array created in a scope still leaks**, which is
|
||||
deliberate and is the same statement `akbasic_ValuePool` has always made: a pointer into a
|
||||
record outlives the scope that DIMmed it. The record below is what was found and why.
|
||||
|
||||
**Also fixed with it:** `SWAP` exchanges whole variable records, so the `values` pointer
|
||||
that came over named the other variable's inline slot and SWAP silently did nothing. Caught
|
||||
by `tests/language/housekeeping/verbs.bas`, which is the golden corpus earning its keep.
|
||||
|
||||
The original report: `akbasic_ValuePool` is a bump allocator
|
||||
on purpose (`include/akbasic/value.h:30`), and the reason given is that "nothing in BASIC
|
||||
destroys a variable". **Scope exit destroys variables.**
|
||||
`akbasic_runtime_prev_environment()` (`src/runtime.c:121`) marks a popped environment's
|
||||
@@ -2440,8 +2454,11 @@ using the thing rather than by testing it.** The corpus reaches none of them.
|
||||
Ordered by blast radius. Each has a minimal reproduction that fits on a screen; every one was
|
||||
reduced against `build/basic`, the stdio build, unless it says otherwise.
|
||||
|
||||
1. **Every call to a user-defined function leaks value-pool slots, so a program may call one
|
||||
about four thousand times and then die.**
|
||||
1. ~~**Every call to a user-defined function leaks value-pool slots, so a program may call one
|
||||
about four thousand times and then die.**~~ **Done** with §6 item 30, and by the same
|
||||
change: the leaking slot was the call scope's parameter, which is a scalar and no longer
|
||||
comes from the pool. Both `DEF` forms run eight thousand calls in
|
||||
`tests/user_functions.c`. The original report:
|
||||
|
||||
```basic
|
||||
DEF ADDIT(N#)
|
||||
|
||||
@@ -91,65 +91,13 @@ A brick is four cells wide, so ten of them are forty cells, and `BLEFT#` centres
|
||||
whatever `COLS#` turned out to be. Integer division truncates here, which is exactly what
|
||||
a cell index wants.
|
||||
|
||||
## Step 3: Create every name before the game starts
|
||||
## Step 3: Declare the names a subroutine has to answer through
|
||||
|
||||
This is the rule that decides whether your game runs for four minutes or for ever, and it
|
||||
is worth getting straight before you write a single subroutine.
|
||||
A `GOSUB` gets its own scope. Assignment walks up the chain and finds an outer name, but a
|
||||
name **first seen** inside a subroutine is created in that subroutine and dies at
|
||||
`RETURN` — so a routine cannot answer its caller through a name it invented itself.
|
||||
|
||||
**A name that is created costs value-pool slots that are never handed back.** The pool is
|
||||
a bump allocator with 4096 slots for the life of the run. Scope exit *does* destroy
|
||||
variables — a `GOSUB`'s locals go when it returns — but their slots are not reclaimed, so
|
||||
recreating the same local on the next call takes fresh ones.
|
||||
|
||||
Here is the whole defect on one screen:
|
||||
|
||||
```basic
|
||||
X# = 0
|
||||
FOR T# = 1 TO 6000
|
||||
GOSUB SUBA
|
||||
NEXT T#
|
||||
PRINT "OK " + X#
|
||||
END
|
||||
|
||||
LABEL SUBA
|
||||
LOC# = 1
|
||||
X# = X# + LOC#
|
||||
RETURN
|
||||
```
|
||||
|
||||
```output
|
||||
? 8 : RUNTIME ERROR Array of 1 elements does not fit in the 0 remaining value slots
|
||||
|
||||
```
|
||||
|
||||
Six thousand calls, one new name each, and it dies on the four-thousand-and-somethingth
|
||||
— naming the line that was unlucky rather than the line that was wrong. A game loop that
|
||||
creates one name per tick is dead in half a minute. This one was, and it took twenty-five
|
||||
seconds to do it.
|
||||
|
||||
The fix is one line, moved:
|
||||
|
||||
```basic
|
||||
X# = 0
|
||||
LOC# = 0
|
||||
FOR T# = 1 TO 6000
|
||||
GOSUB SUBA
|
||||
NEXT T#
|
||||
PRINT "OK " + X#
|
||||
END
|
||||
|
||||
LABEL SUBA
|
||||
LOC# = 1
|
||||
X# = X# + LOC#
|
||||
RETURN
|
||||
```
|
||||
|
||||
```output
|
||||
OK 6000
|
||||
```
|
||||
|
||||
So **declare everything at the top** — variables, scratch temporaries, loop counters, all
|
||||
of it — and after that the program only ever stores into names that already exist:
|
||||
That is why the game declares this block before anything calls anything:
|
||||
|
||||
```basic norun
|
||||
DIM BR#(60)
|
||||
@@ -161,23 +109,66 @@ BX# = 0
|
||||
BY# = 0
|
||||
HIT# = 0
|
||||
BI# = 0
|
||||
I# = 0
|
||||
R# = 0
|
||||
C# = 0
|
||||
KI# = 0
|
||||
```
|
||||
|
||||
`I#`, `R#`, `C#` and `KI#` are in that list because a `FOR` counter is a name like any
|
||||
other: name one that does not exist yet and every entry to the loop spends slots.
|
||||
`HIT#` and `BI#` are how `HITTEST` (Step 8) answers the routine that called it. Declared
|
||||
out here they are one variable the whole program shares; left to `HITTEST`, they would be
|
||||
two that vanish at `RETURN` and the caller would read zeros for ever, with no diagnostic
|
||||
of any kind. [Chapter 4](04-control-flow.md#goto-and-gosub) has the scope rules.
|
||||
|
||||
The same block does a second job. A `GOSUB` gets its own scope, and assignment walks up
|
||||
the chain to find an outer name — but a name *first seen* inside a subroutine is created
|
||||
in that subroutine and dies at `RETURN`. `HIT#` and `BI#` are how `HITTEST` answers its
|
||||
caller, so they have to exist before anybody calls it. See
|
||||
[Chapter 4](04-control-flow.md#goto-and-gosub) for the scope rules.
|
||||
**Creating a scalar inside a scope is free**, so nothing else has to be declared up front:
|
||||
|
||||
This is filed as `TODO.md` §6 item 30. Until it is fixed, the declaration block is not a
|
||||
style preference — it is the price of a program that runs.
|
||||
```basic
|
||||
X# = 0
|
||||
FOR T# = 1 TO 20000
|
||||
GOSUB SUBA
|
||||
NEXT T#
|
||||
PRINT "OK " + X#
|
||||
END
|
||||
|
||||
LABEL SUBA
|
||||
LOC# = 1
|
||||
X# = X# + LOC#
|
||||
RETURN
|
||||
```
|
||||
|
||||
```output
|
||||
OK 20000
|
||||
```
|
||||
|
||||
Twenty thousand calls, each creating a local. **This did not use to work.** A scalar drew
|
||||
storage from the same 4096-slot pool an array does, scope exit gave back the variable but
|
||||
not its storage, and a program creating one name per tick died in about half a minute —
|
||||
naming whichever line was unlucky rather than the line that was wrong. It is what killed
|
||||
the first version of this game after twenty-five seconds. `TODO.md` §6 item 30 has the
|
||||
history; `tests/value_pool.c` is what keeps it fixed.
|
||||
|
||||
**An array is different, and still costs slots for good.** `DIM` inside a subroutine takes
|
||||
pool storage that never comes back, because a pointer into a record can outlive the scope
|
||||
that declared it — see [Chapter 16](16-structures.md#limits):
|
||||
|
||||
```basic
|
||||
X# = 0
|
||||
FOR T# = 1 TO 6000
|
||||
GOSUB SUBA
|
||||
NEXT T#
|
||||
PRINT "OK " + X#
|
||||
END
|
||||
|
||||
LABEL SUBA
|
||||
DIM LOC#(4)
|
||||
LOC#(0) = 1
|
||||
X# = X# + LOC#(0)
|
||||
RETURN
|
||||
```
|
||||
|
||||
```output
|
||||
? 8 : RUNTIME ERROR Array of 4 elements does not fit in the 0 remaining value slots
|
||||
|
||||
```
|
||||
|
||||
So the rule that survives is narrow: **`DIM` at the top, not in a loop.** Which is where
|
||||
you would have put it anyway.
|
||||
|
||||
## Step 4: Build the wall, and write a row whole
|
||||
|
||||
@@ -717,8 +708,9 @@ every bounce, so it plays like something with a hand on it and occasionally miss
|
||||
demo that tracks perfectly never loses a ball and therefore never tests losing one.
|
||||
|
||||
Leave it running for two minutes and watch the score climb: that exercises the ball, the
|
||||
bricks, the level change, the speed-up and — most usefully — the value pool from Step 3,
|
||||
which is the failure that only shows up after thousands of ticks.
|
||||
bricks, the level change and the speed-up — and it is the only thing that exercises them
|
||||
*together*, for long enough. Every defect this game found took thousands of ticks to show
|
||||
itself, and a green test suite proved nothing about any of them.
|
||||
|
||||
The machine does not get on the scoreboard. Attract mode keeps score so the wall behaves,
|
||||
but only a player's score is ever the high one:
|
||||
@@ -739,7 +731,7 @@ game against:
|
||||
|
||||
| Rule | Because | Where |
|
||||
|---|---|---|
|
||||
| Create every name at the top, loop counters included | A name created inside a scope costs pool slots for good; 4096 ends the run | Step 3 |
|
||||
| `DIM` at the top, never in a loop | An array created inside a scope costs pool slots for good; 4096 ends the run. A scalar is free | Step 3 |
|
||||
| Build a text row whole and write it from column 0 | `CHAR` terminates the row where it stops | Step 4 |
|
||||
| Tell rows apart by shape, not colour | `CHAR` ignores its colour argument | Step 4 |
|
||||
| Compute a `MOVSPR` coordinate into a variable first | A leading sign means *move by*, not *move to* | Step 5 |
|
||||
|
||||
@@ -773,7 +773,7 @@ stops a feature costing an afternoon before it is abandoned:
|
||||
| Variables | 128 | 121, plus 4 the interpreter makes |
|
||||
| Labels | 64 | 61 |
|
||||
| `SSHAPE` slots | 16, none reclaimed except by `GRAPHIC 5` | 6 stamps plus 1 capture a frame |
|
||||
| Value-pool slots | 4096, none reclaimed | none after startup — see [Chapter 17](17-tutorial-breakout.md#step-3-create-every-name-before-the-game-starts) |
|
||||
| Value-pool slots | 4096 for arrays and structures; a scalar costs none | six arrays, declared once — see [Chapter 17](17-tutorial-breakout.md#step-3-declare-the-names-a-subroutine-has-to-answer-through) |
|
||||
| Scopes | 32 | 6 deep at most |
|
||||
| Tokens on a line | 32, and the 33rd kills the interpreter rather than raising | short lines, temporaries instead of long conditions |
|
||||
|
||||
|
||||
@@ -21,17 +21,24 @@
|
||||
#include <akbasic/types.h>
|
||||
|
||||
/*
|
||||
* Backing store for array variables. Scalars and arrays alike draw their storage
|
||||
* from here rather than from malloc, and a variable holds a pointer into it. The
|
||||
* runtime owns exactly one; it is passed explicitly rather than kept at file
|
||||
* scope, because the interpreter must be embeddable and may not own global
|
||||
* mutable state.
|
||||
* Backing store for arrays and structures. They draw their storage from here
|
||||
* rather than from malloc, and a variable holds a pointer into it. The runtime
|
||||
* owns exactly one; it is passed explicitly rather than kept at file scope,
|
||||
* because the interpreter must be embeddable and may not own global mutable
|
||||
* state.
|
||||
*
|
||||
* The allocator is a bump: releasing is not supported, because nothing in BASIC
|
||||
* destroys a variable. Re-DIMming a variable to the same size or smaller reuses
|
||||
* its existing slice; growing it takes fresh slots and abandons the old ones. A
|
||||
* program that re-DIMs the same array larger in a loop will exhaust the pool and
|
||||
* get AKBASIC_ERR_BOUNDS -- bounded and diagnosable, which is the point.
|
||||
* **A scalar is not in here.** It lives in the variable itself
|
||||
* (akbasic_Variable::inlinevalue), because scope exit *does* destroy variables
|
||||
* -- it returns the slot but not its storage -- and a bump allocator has no way
|
||||
* to take a scalar back. See akbasic_variable_init().
|
||||
*
|
||||
* The allocator is a bump: releasing is not supported. Re-DIMming an array to
|
||||
* the same size or smaller reuses its existing slice; growing it takes fresh
|
||||
* slots and abandons the old ones. A program that re-DIMs the same array larger
|
||||
* in a loop will exhaust the pool and get AKBASIC_ERR_BOUNDS -- bounded and
|
||||
* diagnosable, which is the point. A local *array* therefore still costs slots
|
||||
* that do not come back, which is the same statement and is deliberate: a
|
||||
* pointer into a record outlives the scope that DIMmed it.
|
||||
*/
|
||||
typedef struct
|
||||
{
|
||||
|
||||
@@ -20,8 +20,21 @@ typedef struct
|
||||
{
|
||||
char name[AKBASIC_MAX_STRING_LENGTH];
|
||||
akbasic_Type valuetype;
|
||||
akbasic_Value *values; /** Points into the runtime's value pool */
|
||||
akbasic_Value *values; /** The pool, or `inlinevalue` for a scalar */
|
||||
int valuecount;
|
||||
/*
|
||||
* A scalar's storage, so that creating one costs the value pool nothing.
|
||||
*
|
||||
* The pool is a bump allocator with no free, and a scope exit hands the
|
||||
* variable *slot* back while its storage stays counted against the pool --
|
||||
* so a name first created inside a GOSUB used to spend slots on every call
|
||||
* and 4096 creations ended the run. Keeping the one value here instead
|
||||
* means a local, a FOR counter and a DEF parameter are all free.
|
||||
*
|
||||
* akbasic_variable_init() decides which of the two `values` points at, and
|
||||
* a `@` name is the one exclusion: see the comment there.
|
||||
*/
|
||||
akbasic_Value inlinevalue;
|
||||
int64_t dimensions[AKBASIC_MAX_ARRAY_DEPTH];
|
||||
int dimensioncount;
|
||||
bool mutable_;
|
||||
|
||||
@@ -300,6 +300,18 @@ akerr_ErrorContext *akbasic_cmd_swap(akbasic_Runtime *obj, akbasic_ASTLeaf *expr
|
||||
memcpy(b, &swap, sizeof(*b));
|
||||
memcpy(a->name, namea, sizeof(a->name));
|
||||
memcpy(b->name, nameb, sizeof(b->name));
|
||||
/*
|
||||
* A scalar's storage is *inside* the record, so the pointer that came over
|
||||
* with it names the other variable's inline slot -- which now holds this
|
||||
* variable's own old value. Left alone, each side reads back what it started
|
||||
* with and SWAP silently does nothing. Rebind each to its own.
|
||||
*/
|
||||
if ( a->values == &b->inlinevalue ) {
|
||||
a->values = &a->inlinevalue;
|
||||
}
|
||||
if ( b->values == &a->inlinevalue ) {
|
||||
b->values = &b->inlinevalue;
|
||||
}
|
||||
|
||||
SUCCEED_TRUE(obj, dest);
|
||||
SUCCEED_RETURN(errctx);
|
||||
|
||||
@@ -89,12 +89,30 @@ akerr_ErrorContext *akbasic_variable_init(akbasic_Variable *obj, akbasic_ValuePo
|
||||
obj->dimensioncount = sizecount;
|
||||
|
||||
/*
|
||||
* Reuse the existing slice when it is already big enough. That makes a
|
||||
* re-DIM to the same or a smaller size free, which is the only re-DIM any
|
||||
* real program performs; growing takes fresh slots and abandons the old
|
||||
* ones, as documented on akbasic_ValuePool.
|
||||
* A scalar lives in the variable; only a run of more than one value comes
|
||||
* from the pool.
|
||||
*
|
||||
* That is not an optimisation, it is what makes a name created inside a
|
||||
* scope cost nothing. The pool never frees and a scope exit returns the
|
||||
* variable slot without returning its storage, so every GOSUB that created
|
||||
* a local used to spend slots that never came back -- 4096 creations and
|
||||
* the run was over, which a game loop reaches in half a minute. TODO.md
|
||||
* section 6 item 30 has the whole reduction.
|
||||
*
|
||||
* **A `@` name is the one exclusion**, and it is the whole of it. A
|
||||
* structure or a pointer to one keeps pool storage because a pointer may
|
||||
* outlive the scope that DIMmed it -- docs/16-structures.md says nothing is
|
||||
* reclaimed and akbasic_runtime_prev_environment() relies on it. The suffix
|
||||
* is the right test rather than `structtype`, which the DIM path sets
|
||||
* *after* calling this.
|
||||
*
|
||||
* Otherwise: reuse the existing slice when it is already big enough, which
|
||||
* makes a re-DIM to the same or a smaller size free; growing takes fresh
|
||||
* slots and abandons the old ones, as documented on akbasic_ValuePool.
|
||||
*/
|
||||
if ( obj->values == NULL || obj->valuecount < (int)totalsize ) {
|
||||
if ( totalsize == 1 && lastchar != '@' ) {
|
||||
obj->values = &obj->inlinevalue;
|
||||
} else if ( obj->values == NULL || obj->valuecount < (int)totalsize ) {
|
||||
PASS(errctx, akbasic_valuepool_take(pool, (int)totalsize, &obj->values));
|
||||
}
|
||||
obj->valuecount = (int)totalsize;
|
||||
|
||||
@@ -22,15 +22,31 @@
|
||||
#include "harness.h"
|
||||
#include "testutil.h"
|
||||
|
||||
/** @brief Run a program to completion, bounded so a hang fails rather than waits. */
|
||||
static akerr_ErrorContext AKERR_NOIGNORE *run_program(const char *source)
|
||||
/** @brief Run a program to completion under an explicit step budget. */
|
||||
static akerr_ErrorContext AKERR_NOIGNORE *run_program_bounded(const char *source, int64_t steps)
|
||||
{
|
||||
PREPARE_ERROR(errctx);
|
||||
|
||||
PASS(errctx, harness_start(NULL));
|
||||
PASS(errctx, akbasic_runtime_load(&HARNESS_RUNTIME, source));
|
||||
PASS(errctx, akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN));
|
||||
PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, 20000));
|
||||
PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, steps));
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Run a program to completion, bounded so a hang fails rather than waits.
|
||||
*
|
||||
* 20,000 steps, which is generous for every case here except the two that call a
|
||||
* function eight thousand times. Those name their own budget rather than raising
|
||||
* this one, because a bound loose enough for them would stop the recursion case
|
||||
* from failing quickly.
|
||||
*/
|
||||
static akerr_ErrorContext AKERR_NOIGNORE *run_program(const char *source)
|
||||
{
|
||||
PREPARE_ERROR(errctx);
|
||||
|
||||
PASS(errctx, run_program_bounded(source, 20000));
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
@@ -232,6 +248,42 @@ static void test_call_scopes_are_reclaimed(void)
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Eight thousand calls, which used to be about four thousand too many.
|
||||
*
|
||||
* The *variable* slot came back (the test above); the value slots behind the
|
||||
* parameter did not. A `DEF` cost one pool slot per call and died between the
|
||||
* four and five thousandth with "Array of 1 elements does not fit in the 0
|
||||
* remaining value slots" -- so a game calling one function per frame at thirty
|
||||
* frames a second got a little over two minutes, and the message named neither
|
||||
* the function nor the reason.
|
||||
*
|
||||
* TODO.md section 9 item 1. Both forms are here because both leaked: the
|
||||
* comparison that isolated it originally was that a `GOSUB` doing the same work
|
||||
* ran eight thousand times without complaint.
|
||||
*/
|
||||
static void test_calls_do_not_leak_value_slots(void)
|
||||
{
|
||||
TEST_REQUIRE_OK(run_program_bounded("10 DEF ADDIT(N#)\n"
|
||||
"20 RETURN N# + 1\n"
|
||||
"30 R# = 0\n"
|
||||
"40 FOR I# = 1 TO 8000\n"
|
||||
"50 R# = ADDIT(I#)\n"
|
||||
"60 NEXT I#\n"
|
||||
"70 PRINT \"OK \" + R#\n", 1000000));
|
||||
TEST_REQUIRE_STR(HARNESS_OUTPUT, "OK 8001\n");
|
||||
harness_stop();
|
||||
|
||||
TEST_REQUIRE_OK(run_program_bounded("10 DEF SQR1(X#) = X# * X#\n"
|
||||
"20 R# = 0\n"
|
||||
"30 FOR I# = 1 TO 8000\n"
|
||||
"40 R# = SQR1(I#)\n"
|
||||
"50 NEXT I#\n"
|
||||
"60 PRINT \"OK \" + R#\n", 1000000));
|
||||
TEST_REQUIRE_STR(HARNESS_OUTPUT, "OK 64000000\n");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
TEST_REQUIRE_OK(akbasic_error_register());
|
||||
@@ -244,6 +296,7 @@ int main(void)
|
||||
test_structure_parameters();
|
||||
test_structure_parameter_types_are_checked();
|
||||
test_call_scopes_are_reclaimed();
|
||||
test_calls_do_not_leak_value_slots();
|
||||
|
||||
return akbasic_test_failures;
|
||||
}
|
||||
|
||||
209
tests/value_pool.c
Normal file
209
tests/value_pool.c
Normal file
@@ -0,0 +1,209 @@
|
||||
/**
|
||||
* @file value_pool.c
|
||||
* @brief Tests that creating a name inside a scope costs the value pool nothing.
|
||||
*
|
||||
* This is the defect a Breakout written against the interpreter died of after
|
||||
* twenty-five seconds, and nothing in either corpus could have found it: the
|
||||
* pool holds 4096 values, so it takes *thousands* of scope entries to see, and
|
||||
* nothing else here runs that long.
|
||||
*
|
||||
* The cause was that scope exit returns the variable *slot* and not its storage.
|
||||
* akbasic_runtime_prev_environment() marks the variable unused,
|
||||
* akbasic_runtime_new_variable() memsets the slot it hands back -- clearing
|
||||
* `values` -- and akbasic_variable_init() therefore drew fresh slots for a
|
||||
* variable whose old ones were still counted. Every GOSUB that created a local
|
||||
* leaked, with no diagnostic until the pool ran dry on whichever line happened
|
||||
* to be unlucky.
|
||||
*
|
||||
* A scalar now lives in the variable itself, so the pool is not involved at all.
|
||||
* Two of the counts below are past the old 4096 ceiling on purpose -- under it,
|
||||
* they would pass against the broken interpreter too. The third takes the
|
||||
* opposite approach and asks for the whole pool after only a few scopes, which
|
||||
* is cheaper and sharper: one leaked slot and it has nowhere to go.
|
||||
*/
|
||||
|
||||
#include <string.h>
|
||||
|
||||
#include <akbasic/error.h>
|
||||
#include <akbasic/runtime.h>
|
||||
|
||||
#include "harness.h"
|
||||
#include "testutil.h"
|
||||
|
||||
/** @brief Run a program to completion, bounded so a hang fails rather than waits. */
|
||||
static akerr_ErrorContext AKERR_NOIGNORE *run_program(const char *source)
|
||||
{
|
||||
PREPARE_ERROR(errctx);
|
||||
|
||||
PASS(errctx, harness_start(NULL));
|
||||
PASS(errctx, akbasic_runtime_load(&HARNESS_RUNTIME, source));
|
||||
PASS(errctx, akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN));
|
||||
PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, 2000000));
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief 6,000 GOSUBs, each creating a local, cost nothing.
|
||||
*
|
||||
* TODO.md section 6 item 30's reduction, unchanged. It used to fail at
|
||||
* `LOC# = 1` on the 4091st call with "Array of 1 elements does not fit in the
|
||||
* 0 remaining value slots" -- naming the line that was unlucky rather than the
|
||||
* line that was wrong, which is most of why it cost an evening to find.
|
||||
*/
|
||||
static void test_scoped_scalar_costs_nothing(void)
|
||||
{
|
||||
TEST_REQUIRE_OK(run_program("10 X# = 0\n"
|
||||
"20 FOR T# = 1 TO 6000\n"
|
||||
"30 GOSUB SUBA\n"
|
||||
"40 NEXT T#\n"
|
||||
"50 PRINT \"OK \" + X#\n"
|
||||
"60 END\n"
|
||||
"70 LABEL SUBA\n"
|
||||
"80 LOC# = 1\n"
|
||||
"90 X# = X# + LOC#\n"
|
||||
"100 RETURN\n"));
|
||||
TEST_REQUIRE_STR(HARNESS_OUTPUT, "OK 6000\n");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief A `FOR` counter is a name like any other, and costs nothing either.
|
||||
*
|
||||
* Worth its own case: the counter is created by the loop rather than by an
|
||||
* assignment the author wrote, so it was the half of this defect that a program
|
||||
* could hit without appearing to create anything at all.
|
||||
*
|
||||
* `TO 2` rather than `TO 1` because equal bounds run the body zero times in this
|
||||
* dialect -- see docs/13-differences.md. A body that never runs would pass this
|
||||
* test for the wrong reason.
|
||||
*/
|
||||
static void test_scoped_loop_counter_costs_nothing(void)
|
||||
{
|
||||
TEST_REQUIRE_OK(run_program("10 X# = 0\n"
|
||||
"20 FOR T# = 1 TO 6000\n"
|
||||
"30 GOSUB SUBA\n"
|
||||
"40 NEXT T#\n"
|
||||
"50 PRINT \"OK \" + X#\n"
|
||||
"60 END\n"
|
||||
"70 LABEL SUBA\n"
|
||||
"80 FOR INNER# = 1 TO 2\n"
|
||||
"90 X# = X# + 1\n"
|
||||
"100 NEXT INNER#\n"
|
||||
"110 RETURN\n"));
|
||||
TEST_REQUIRE_STR(HARNESS_OUTPUT, "OK 12000\n");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief The pool is genuinely untouched, not merely large enough.
|
||||
*
|
||||
* Two hundred scope entries and then the pool's *entire* width -- four arrays,
|
||||
* because AKBASIC_MAX_ARRAY_ELEMENTS caps any one of them at 1024 while the pool
|
||||
* holds 4096. One leaked slot and the fourth `DIM` has nowhere to go, which
|
||||
* makes this the sharpest of the three and by far the cheapest: the two above
|
||||
* prove a program finishes, this proves nothing at all was spent. The long ones
|
||||
* stay because they are the shape the defect was found in.
|
||||
*/
|
||||
static void test_pool_is_untouched_by_scopes(void)
|
||||
{
|
||||
TEST_REQUIRE_OK(run_program("10 FOR T# = 1 TO 200\n"
|
||||
"20 GOSUB SUBA\n"
|
||||
"30 NEXT T#\n"
|
||||
"40 DIM A#(1024)\n"
|
||||
"50 DIM B#(1024)\n"
|
||||
"60 DIM C#(1024)\n"
|
||||
"70 DIM D#(1024)\n"
|
||||
"80 D#(1023) = 7\n"
|
||||
"90 PRINT D#(1023)\n"
|
||||
"100 END\n"
|
||||
"110 LABEL SUBA\n"
|
||||
"120 LOC# = 1\n"
|
||||
"130 RETURN\n"));
|
||||
TEST_REQUIRE_STR(HARNESS_OUTPUT, "7\n");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief A structure keeps pool storage, which is what a pointer relies on.
|
||||
*
|
||||
* The exclusion that makes the fix safe. docs/16-structures.md says nothing is
|
||||
* reclaimed and akbasic_runtime_prev_environment() says a pointer into a record
|
||||
* DIMmed in a scope stays sound after the scope is gone -- so a `@` name must
|
||||
* *not* move into the variable, where the next occupant of the slot would
|
||||
* overwrite it. A single-field TYPE is the case that would slip through a test
|
||||
* written against size alone.
|
||||
*/
|
||||
static void test_structure_storage_stays_in_the_pool(void)
|
||||
{
|
||||
akbasic_Variable *variable = NULL;
|
||||
|
||||
TEST_REQUIRE_OK(run_program("10 TYPE ONE\n"
|
||||
"20 N#\n"
|
||||
"30 END TYPE\n"
|
||||
"40 DIM R@ AS ONE\n"
|
||||
"50 R@.N# = 5\n"
|
||||
"60 PRINT R@.N#\n"));
|
||||
TEST_REQUIRE_STR(HARNESS_OUTPUT, "5\n");
|
||||
|
||||
TEST_REQUIRE_OK(akbasic_environment_get(HARNESS_RUNTIME.environment, "R@", &variable));
|
||||
TEST_REQUIRE(variable != NULL, "the structure variable must exist");
|
||||
TEST_REQUIRE(variable->values != &variable->inlinevalue,
|
||||
"a structure must keep pool storage, not inline storage");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief A scalar's storage is inside the variable, and an array's is not.
|
||||
*
|
||||
* Asserted directly rather than only through a program, so the mechanism is
|
||||
* pinned and not just its consequence. A future change that made arrays inline
|
||||
* would pass every test above and break the pointer guarantee.
|
||||
*/
|
||||
static void test_storage_lands_where_it_should(void)
|
||||
{
|
||||
akbasic_Variable *scalar = NULL;
|
||||
akbasic_Variable *array = NULL;
|
||||
|
||||
TEST_REQUIRE_OK(run_program("10 S# = 1\n"
|
||||
"20 DIM A#(4)\n"));
|
||||
|
||||
TEST_REQUIRE_OK(akbasic_environment_get(HARNESS_RUNTIME.environment, "S#", &scalar));
|
||||
TEST_REQUIRE(scalar != NULL, "the scalar must exist");
|
||||
TEST_REQUIRE(scalar->values == &scalar->inlinevalue,
|
||||
"a scalar must be stored in the variable");
|
||||
|
||||
TEST_REQUIRE_OK(akbasic_environment_get(HARNESS_RUNTIME.environment, "A#", &array));
|
||||
TEST_REQUIRE(array != NULL, "the array must exist");
|
||||
TEST_REQUIRE(array->values != &array->inlinevalue,
|
||||
"an array must be stored in the pool");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief `SWAP` exchanges two scalars, whose storage moves with the record.
|
||||
*
|
||||
* A regression rather than a feature: SWAP copies the whole variable record, so
|
||||
* the `values` pointer that comes over names the *other* variable's inline slot
|
||||
* -- which by then holds this variable's own old value. Left alone each side
|
||||
* reads back what it started with and SWAP silently does nothing, which is
|
||||
* exactly what the housekeeping golden case caught.
|
||||
*/
|
||||
static void test_swap_still_exchanges_scalars(void)
|
||||
{
|
||||
TEST_REQUIRE_OK(run_program("10 A# = 1 : B# = 2\n"
|
||||
"20 SWAP A#, B#\n"
|
||||
"30 PRINT A# : PRINT B#\n"));
|
||||
TEST_REQUIRE_STR(HARNESS_OUTPUT, "2\n1\n");
|
||||
harness_stop();
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
test_scoped_scalar_costs_nothing();
|
||||
test_scoped_loop_counter_costs_nothing();
|
||||
test_pool_is_untouched_by_scopes();
|
||||
test_structure_storage_stays_in_the_pool();
|
||||
test_storage_lands_where_it_should();
|
||||
test_swap_still_exchanges_scalars();
|
||||
return akbasic_test_failures;
|
||||
}
|
||||
Reference in New Issue
Block a user