diff options
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/correctness-audit.md | 93 |
1 files changed, 93 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. |
