1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
|
# 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;
it returns a nonzero status if any of the three findings still reproduces.
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.
## Fixes
- Snapshot scroll accounting now counts discarded cluster records as dead,
allowing compaction to reclaim them.
- Snapshot allocation failures now report false, empty the snapshot, and
force a complete capture on retry.
- Height shrink allocates the replacement screen before moving rows or
changing the cursor, preserving state if allocation fails.
Dedicated regression tests cover sustained scrolling, snapshot allocation
failure and recovery, and failed height shrink on both normal and alternate
screens. The output above records the original failures for reference.
Validation after the fixes: all 43 unit test entries passed in an
unsanitized debug build and all 43 passed with address and undefined-behavior
sanitizers. The standalone probe also passed in both builds. Leak detection
remained disabled in the sandbox.
|
