diff options
| -rw-r--r-- | docs/correctness-audit.md | 93 | ||||
| -rw-r--r-- | tests/correctness_probe.c | 80 |
2 files changed, 173 insertions, 0 deletions
diff --git a/docs/correctness-audit.md b/docs/correctness-audit.md new file mode 100644 index 0000000..bb85e24 --- /dev/null +++ b/docs/correctness-audit.md @@ -0,0 +1,93 @@ +# Correctness audit + +The 2026-10-15 audit reproduced three issues that the existing tests missed. +The unit suite reported 41 passing test entries under AddressSanitizer and +UndefinedBehaviorSanitizer. The existing resize allocation-failure test skips +its checks under sanitizers; the dedicated probe below uses linker wrappers +to inject failures even in a sanitized build. + +## Run the reproducer + +From the repository root, with Meson, a C compiler, and a linker supporting +GNU `--wrap`: + +```sh +meson setup build-audit --buildtype=debug -Db_lto=false \ + -Db_sanitize=address,undefined -Dkitty=disabled +meson compile -C build-audit -j 4 +cc -std=c11 -g -UNDEBUG -fsanitize=address,undefined \ + -Iinclude/term -Isrc tests/correctness_probe.c build-audit/src/libterm.a \ + -Wl,--wrap=realloc -Wl,--wrap=calloc -lm -o build-audit/correctness-probe +ASAN_OPTIONS=detect_leaks=0 build-audit/correctness-probe +``` + +`tests/correctness_probe.c` prints the observed behavior. It intentionally +includes `src/snapshot.c` to inspect its private arena counters and links +against the static library. It is a diagnostic, outside the default suite; +exit status zero alone does not mean the findings have been fixed. + +Leak detection was disabled because LeakSanitizer could not run in the audit +sandbox. Address and undefined-behavior checks remained enabled. To repeat +the unit suite in that environment: + +```sh +ASAN_OPTIONS=detect_leaks=0 meson test -C build-audit --suite unit \ + --num-processes 4 --print-errorlogs +``` + +Kitty compatibility and cross-platform tests were outside this sanitized run. + +## Findings + +### Snapshot allocation failure silently loses combining marks + +In `src/snapshot.c`, `arena_add()` returns the first code point when its +allocation fails. The update reports success and caches the row key, so a +later update does not restore the omitted code points. + +The probe prints an `e` followed by U+0301, fails the next `realloc`, and +captures twice. Both captures report a cluster length of 1 instead of 2. +An allocation failure should report false and leave the snapshot empty, +as documented. A successful retry should capture both code points. + +### Scrolling leaks snapshot arena records + +The scroll optimization moves `row_clusters` without adding the discarded +rows' records to `arena_dead`. Compaction therefore misses those records. + +On an 8-column, 24-row terminal with history capped at five lines, the probe +feeds `e` plus U+0301 and CRLF, capturing after each line. The original +output is: + +```text +after 1000 lines: arena_words=3000 dead_words=0 history_rows=5 +after 10000 lines: arena_words=30000 dead_words=0 history_rows=5 +``` + +Snapshot arena storage should remain bounded by the viewport and compaction +threshold, independently of how many lines have scrolled past it. + +### Failed height shrink changes terminal state + +In `src/screen.c`, `term_resize_internal()` scrolls and adjusts the cursor +before allocating the replacement screen. If that allocation fails, it +returns false after changing the terminal, violating the documented promise +that a failed resize leaves it unchanged. + +The probe fills an 8-by-4 terminal with rows A through D and fails the +replacement screen allocation while shrinking to two rows. The original +result is: + +```text +before failed height shrink: top=A cursor_y=3 history=0 +after failed height shrink: resize=0 height=4 top=C cursor_y=1 history=2 +``` + +After failure, the screen should still start with A, the cursor should +remain on row 3, and history should remain empty. + +## Checks to retain + +- Inject allocation failures and verify each API's documented failure state. +- Compare incremental snapshots with fresh captures, including grapheme text. +- Assert bounded storage during prolonged scrolling with a fixed viewport. diff --git a/tests/correctness_probe.c b/tests/correctness_probe.c new file mode 100644 index 0000000..0b55211 --- /dev/null +++ b/tests/correctness_probe.c @@ -0,0 +1,80 @@ +#include <assert.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +/* Include the implementation to inspect snapshot arena accounting. Link + * against the static library so snapshot.c is compiled only here. */ +#include "../src/snapshot.c" + +static int fail_realloc; +static int fail_resize_calloc; +void *__real_realloc(void *, size_t); +void *__real_calloc(size_t, size_t); +void *__wrap_calloc(size_t n, size_t size) +{ + if (fail_resize_calloc && n == 2 && size == sizeof(struct srow)) { + fail_resize_calloc = 0; + return NULL; + } + return __real_calloc(n, size); +} +void *__wrap_realloc(void *ptr, size_t size) +{ + if (fail_realloc) { fail_realloc = 0; return NULL; } + return __real_realloc(ptr, size); +} + +int main(void) +{ + struct term_options options = { .size = { 8, 3 }, .scrollback = 5 }; + struct term *t = term_new(&options); + struct term_snapshot *s = term_snapshot_new(); + assert(t && s && term_snapshot_update(s, t)); + const char accented[] = "e\xcc\x81"; + term_feed(t, accented, sizeof(accented) - 1, 0); + fail_realloc = 1; + bool ok = term_snapshot_update(s, t); + uint32_t cps[16]; + int n = term_snapshot_cluster(s, term_snapshot_row(s, 0)[0].text, cps, 16); + printf("snapshot arena allocation failure: update=%d cluster_length=%d (expected false, or length 2)\n", ok, n); + assert(term_snapshot_update(s, t)); + n = term_snapshot_cluster(s, term_snapshot_row(s, 0)[0].text, cps, 16); + printf("retry after allocation recovers: cluster_length=%d (expected 2)\n", n); + term_snapshot_free(s); + term_free(t); + + options.size.y = 24; + t = term_new(&options); + s = term_snapshot_new(); + assert(t && s); + const char line[] = "e\xcc\x81\r\n"; + for (int i = 1; i <= 10000; i++) { + term_feed(t, line, sizeof(line) - 1, 0); + assert(term_snapshot_update(s, t)); + if (i == 1000 || i == 10000) + printf("after %d lines: arena_words=%u dead_words=%u history_rows=%d\n", + i, s->arena_len, s->arena_dead, term_history_rows(t)); + } + term_snapshot_free(s); + term_free(t); + + options.size = (struct term_vector2i){ 8, 4 }; + t = term_new(&options); + s = term_snapshot_new(); + assert(t && s); + const char rows[] = "A\r\nB\r\nC\r\nD"; + term_feed(t, rows, sizeof(rows) - 1, 0); + assert(term_snapshot_update(s, t)); + printf("before failed height shrink: top=%c cursor_y=%d history=%d\n", + (char)term_snapshot_row(s, 0)[0].text, term_cursor(t).pos.y, term_history_rows(t)); + fail_resize_calloc = 1; + ok = term_resize(t, (struct term_vector2i){ 8, 2 }); + assert(!fail_resize_calloc); + assert(term_snapshot_update(s, t)); + printf("after failed height shrink: resize=%d height=%d top=%c cursor_y=%d history=%d\n", + ok, term_size(t).y, (char)term_snapshot_row(s, 0)[0].text, + term_cursor(t).pos.y, term_history_rows(t)); + term_snapshot_free(s); + term_free(t); + return 0; +} |