diff --git a/TODO.md b/TODO.md index 5f8d236..306f8e8 100644 --- a/TODO.md +++ b/TODO.md @@ -1095,18 +1095,33 @@ What remains, in priority order: - **Item 17** is ours: a host cannot reliably create a script variable while a script is suspended. The fix is `akbasic_runtime_global()` and roughly fifteen lines. §6 describes it and names the one design question to settle first. -6. **Mutation survivors in `src/value.c`.** The harness is in place (`scripts/mutation_test.py`, - the `mutation` CMake target, and a CI job on `src/symtab.c`), and a partial run over - `src/value.c` turned up test gaps worth closing. These are *not* equivalent mutants; each - is a real bug of that shape the suite would not notice: +6. **Mutation survivors in `src/value.c`** — ~~closed~~, with one correction to what was + written here before. `tests/value_arithmetic.c` grew two functions, + `test_maximum_length_string()` and `test_pool_starts_empty()`, and each was verified by + hand-applying the mutant and watching the test fail rather than by assuming it would: - - `AKBASIC_MAX_STRING_LENGTH - 1` → `+ 1` and → `- 0` survive at `src/value.c:47-48`, in - `set_string`'s `strncpy` and its NUL terminator. Nothing in the suite writes a - *maximum-length* string, so the off-by-one that would truncate or overrun goes unseen. - One test that round-trips a 255-character string kills all five. - - `memset(obj, 0, ...)` → `memset(obj, 1, ...)` and its deletion survive in - `akbasic_valuepool_init`, as does `obj->next = 0` → `= 1`. Nothing asserts a freshly - initialised pool is actually empty and zeroed. + | Mutant | Result | + |---|---| + | `strncpy(dest->stringval, src, AKBASIC_MAX_STRING_LENGTH - 1)` → `- 2` | killed | + | `dest->stringval[AKBASIC_MAX_STRING_LENGTH - 1] = '\0'` → `- 2` | killed | + | `strncpy(..., AKBASIC_MAX_STRING_LENGTH - 1)` → `- 0` | **equivalent — cannot be killed** | + | `memset(obj, 0, sizeof(*obj))` deleted from `akbasic_valuepool_init` | killed | + | `obj->next = 0` → `= 1` | killed | + + **The correction.** This entry used to say all five were real bugs and that "one test that + round-trips a 255-character string kills all five". Two things about that were wrong. + + The `- 0` mutant is *equivalent*. `strncpy(dest, src, 256)` into a 256-byte buffer writes at + most 256 bytes, and `set_string()` has already refused anything longer than 255 two lines + above, so the extra byte is always the terminator `strncpy` would have written anyway. It + cannot be killed and should not be counted against the score. + + And the obvious test does *not* kill the truncating mutant. Joining a full-length string to + an empty one passes with the mutant in place, because `akbasic_value_math_plus` clones + `self` into the scratch before writing and the byte a short copy fails to write is already + the right one. The operands have to sum to the limit **without either of them being the + answer** — the test uses 200 `X` plus 55 `Z`, and says so at the site, because this is + exactly the sort of thing that gets "simplified" back into a passing no-op. Two survivors there are genuinely equivalent and cannot be killed: `rval_as_int` and `rval_as_float` (`src/value.c:30,35`) tolerate `+` → `-` because the reference's habit of diff --git a/tests/value_arithmetic.c b/tests/value_arithmetic.c index c85f822..cfa737a 100644 --- a/tests/value_arithmetic.c +++ b/tests/value_arithmetic.c @@ -39,6 +39,113 @@ static void set_string(akbasic_Value *v, const char *s) strncpy(v->stringval, s, AKBASIC_MAX_STRING_LENGTH - 1); } +/** + * @brief A string that exactly fills the value's inline buffer round-trips. + * + * Written because mutation testing said nothing would have noticed if it did + * not: `AKBASIC_MAX_STRING_LENGTH - 1` mutated to `+ 1` and to `- 0` survived at + * both the strncpy and the NUL terminator in set_string(), and no test in the + * suite wrote a maximum-length string. Both mutants are a real bug of that shape + * -- one truncates a string that fits, the other writes one byte past the end of + * a 256-byte buffer -- and the boundary is the only place either shows. + * + * Concatenation rather than a direct set, because set_string() is static and + * concatenation is how every string in a BASIC program actually gets built. + * + * **The two operands are deliberately different lengths and different letters.** + * A first attempt joined a full-length string to an empty one, and the + * truncating mutant survived it: math_plus clones self into the scratch before + * writing, so the byte a short copy failed to write was already the right one. + * The parts have to sum to the limit without either of them being the answer. + */ +static void test_maximum_length_string(void) +{ + enum { LIMIT = AKBASIC_MAX_STRING_LENGTH - 1, HEAD = LIMIT - 55, TAIL = 55 }; + akbasic_Value *out = NULL; + char head[AKBASIC_MAX_STRING_LENGTH]; + char tail[AKBASIC_MAX_STRING_LENGTH]; + char joined[AKBASIC_MAX_STRING_LENGTH]; + + memset(head, 'X', HEAD); + head[HEAD] = '\0'; + memset(tail, 'Z', TAIL); + tail[TAIL] = '\0'; + /* memcpy rather than snprintf: the lengths are known here and -Wformat-truncation + * cannot see that HEAD + TAIL is exactly the buffer's capacity. */ + memcpy(joined, head, HEAD); + memcpy(joined + HEAD, tail, TAIL); + joined[HEAD + TAIL] = '\0'; + + /* Exactly the limit: every byte lands, and the terminator is where it should be. */ + set_string(&A, head); + set_string(&B, tail); + TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out)); + TEST_REQUIRE_INT(strlen(out->stringval), LIMIT); + TEST_REQUIRE_STR(out->stringval, joined); + TEST_REQUIRE_INT(out->stringval[LIMIT - 1], 'Z'); + TEST_REQUIRE_INT(out->stringval[LIMIT], '\0'); + + /* One character past it is an error, not a silent truncation. */ + set_string(&A, joined); + set_string(&B, "!"); + TEST_REQUIRE_STATUS(akbasic_value_math_plus(&A, &B, &SCRATCH, &out), AKBASIC_ERR_VALUE); + + /* The same boundary through the repeat operator, which has its own copy. */ + memset(head, 'Q', LIMIT / 2); + head[LIMIT / 2] = '\0'; + set_string(&A, head); + set_int(&B, 2); + TEST_REQUIRE_OK(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out)); + TEST_REQUIRE_INT(strlen(out->stringval), (LIMIT / 2) * 2); + TEST_REQUIRE_INT(out->stringval[((LIMIT / 2) * 2) - 1], 'Q'); +} + +/** + * @brief A freshly initialised value pool is empty, and its storage is zeroed. + * + * Also a mutation-driven test: `memset(obj, 0, ...)` mutated to `memset(obj, 1, + * ...)`, the memset deleted outright, and `obj->next = 0` mutated to `= 1` all + * survived, because nothing asserted what an initialised pool looks like. The + * deleted-memset mutant is the one that matters -- a pool over stale stack + * memory hands a BASIC program somebody else's array contents. + */ +static void test_pool_starts_empty(void) +{ + static akbasic_ValuePool pool; + akbasic_Value *slice = NULL; + int i = 0; + + /* Dirty every byte first, so a missing memset cannot pass by luck. */ + memset(&pool, 0xAB, sizeof(pool)); + TEST_REQUIRE_OK(akbasic_valuepool_init(&pool)); + + TEST_REQUIRE_INT(pool.next, 0); + for ( i = 0; i < AKBASIC_MAX_ARRAY_VALUES; i++ ) { + if ( pool.values[i].valuetype != 0 || pool.values[i].intval != 0 || + pool.values[i].stringval[0] != '\0' ) { + TEST_REQUIRE(false, "pool slot %d was not zeroed by init", i); + break; + } + } + + /* The first take starts at slot zero and the bump advances by exactly count. */ + TEST_REQUIRE_OK(akbasic_valuepool_take(&pool, 4, &slice)); + TEST_REQUIRE(slice == &pool.values[0], "the first take must start at slot zero"); + TEST_REQUIRE_INT(pool.next, 4); + TEST_REQUIRE_OK(akbasic_valuepool_take(&pool, 1, &slice)); + TEST_REQUIRE(slice == &pool.values[4], "the second take must start where the first ended"); + TEST_REQUIRE_INT(pool.next, 5); + + /* Re-initialising takes it back to empty rather than merely rewinding. */ + TEST_REQUIRE_OK(akbasic_valuepool_init(&pool)); + TEST_REQUIRE_INT(pool.next, 0); + + TEST_REQUIRE_STATUS(akbasic_valuepool_init(NULL), AKERR_NULLPOINTER); + TEST_REQUIRE_STATUS(akbasic_valuepool_take(&pool, 0, &slice), AKBASIC_ERR_BOUNDS); + TEST_REQUIRE_STATUS(akbasic_valuepool_take(&pool, AKBASIC_MAX_ARRAY_VALUES + 1, &slice), + AKBASIC_ERR_BOUNDS); +} + int main(void) { akbasic_Value *out = NULL; @@ -122,5 +229,8 @@ int main(void) /* nil rval is rejected everywhere. */ TEST_REQUIRE_STATUS(akbasic_value_math_plus(&A, NULL, &SCRATCH, &out), AKERR_NULLPOINTER); + test_maximum_length_string(); + test_pool_starts_empty(); + return akbasic_test_failures; }