summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--docs/correctness-audit.md20
-rw-r--r--src/screen.c18
-rw-r--r--src/snapshot.c59
-rw-r--r--tests/correctness_probe.c13
-rw-r--r--tests/meson.build12
-rw-r--r--tests/resize_failure_test.c51
-rw-r--r--tests/snapshot_failure_test.c91
-rw-r--r--tests/snapshot_scroll_arena_test.c29
8 files changed, 271 insertions, 22 deletions
diff --git a/docs/correctness-audit.md b/docs/correctness-audit.md
index bb85e24..4c9a14c 100644
--- a/docs/correctness-audit.md
+++ b/docs/correctness-audit.md
@@ -24,7 +24,7 @@ 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.
+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
@@ -91,3 +91,21 @@ remain on row 3, and history should remain empty.
- 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.
diff --git a/src/screen.c b/src/screen.c
index bd65564..1712faf 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -1236,6 +1236,14 @@ static bool term_resize_internal(struct term *t, int cols, int rows)
int old_cols = active_scr->cols;
int old_rows = active_scr->rows;
+ struct screen new_scr;
+ if (!resize_target(t, &new_scr, cols, rows, active_scr)) {
+ term_gstore_unpin(t->gstore);
+ return false;
+ }
+
+ /* Allocate the replacement before scrolling rows into history. A
+ * failed height shrink must leave the active screen and cursor alone. */
if (rows < old_rows && t->cursor_y >= rows) {
int shift = t->cursor_y - rows + 1;
for (int i = 0; i < shift; i++)
@@ -1243,12 +1251,10 @@ static bool term_resize_internal(struct term *t, int cols, int rows)
t->cursor_y -= shift;
t->saved.y = t->saved.y >= shift ? t->saved.y - shift : 0;
}
-
- struct screen new_scr;
- if (!resize_target(t, &new_scr, cols, rows, active_scr)) {
- term_gstore_unpin(t->gstore);
- return false;
- }
+ /* Scrolling can advance the screen's history mapping when there is
+ * no history buffer, so capture its final value after the scroll. */
+ new_scr.top_id = active_scr->top_id;
+ new_scr.top_seg = active_scr->top_seg;
int pull = 0;
if (rows > old_rows && !t->alternate)
diff --git a/src/snapshot.c b/src/snapshot.c
index 29ed98c..6578c9a 100644
--- a/src/snapshot.c
+++ b/src/snapshot.c
@@ -73,6 +73,20 @@ static bool reserve(struct term_snapshot *s, int cols, int rows)
return true;
}
+/* The public contract makes every failed update an empty, retryable snapshot. */
+static void invalidate(struct term_snapshot *s)
+{
+ s->view = (struct term_view){ .palette = s->palette };
+ s->source = 0;
+ s->arena_len = 0;
+ s->arena_dead = 0;
+ if (s->row_cap > 0) {
+ memset(s->keys, 0, (size_t)s->row_cap * sizeof(*s->keys));
+ memset(s->row_clusters, 0, (size_t)s->row_cap * sizeof(*s->row_clusters));
+ memset(s->selection, 0, (size_t)s->row_cap * sizeof(*s->selection));
+ }
+}
+
/* Attribute contributions of the low and high byte of cell flags. */
static uint16_t lo_attrs[256], hi_attrs[256];
static uint8_t hi_underline[256];
@@ -100,7 +114,8 @@ void term_snapshot_tables_init(void)
}
}
-static uint32_t arena_add(struct term_snapshot *s, const struct gstore *g, uint32_t handle)
+static uint32_t arena_add(struct term_snapshot *s, const struct gstore *g, uint32_t handle,
+ bool *ok)
{
uint32_t cps[GRAPHEME_MAX_LEN];
int n = term_gstore_expand(g, handle, cps, GRAPHEME_MAX_LEN);
@@ -110,8 +125,10 @@ static uint32_t arena_add(struct term_snapshot *s, const struct gstore *g, uint3
uint32_t cap = term_grow_cap32(s->arena_cap ? s->arena_cap * 2 : 256,
s->arena_len + (uint32_t)n + 1);
uint32_t *a = realloc(s->arena, cap * sizeof(*a));
- if (!a)
- return cps[0];
+ if (!a) {
+ *ok = false;
+ return 0;
+ }
s->arena = a;
s->arena_cap = cap;
}
@@ -125,7 +142,7 @@ static uint32_t arena_add(struct term_snapshot *s, const struct gstore *g, uint3
/* Convert one stored row; returns the number of arena words it added. */
static uint32_t convert_row(struct term_snapshot *s, const struct gstore *g,
struct term_cell *dst,
- const struct cell *src, int cols)
+ const struct cell *src, int cols, bool *ok)
{
uint32_t before = s->arena_len;
for (int x = 0; x < cols; x++) {
@@ -138,8 +155,11 @@ static uint32_t convert_row(struct term_snapshot *s, const struct gstore *g,
uint32_t text = c->c;
if (TERM_EXPECT(text < 0x20, 0))
text = text ? ' ' : 0;
- else if (TERM_EXPECT(grapheme_is_handle(text), 0))
- text = arena_add(s, g, text);
+ else if (TERM_EXPECT(grapheme_is_handle(text), 0)) {
+ text = arena_add(s, g, text, ok);
+ if (!*ok)
+ return 0;
+ }
dst[x] = (struct term_cell){
.text = text,
.fg = c->fg,
@@ -158,8 +178,7 @@ static bool term_snapshot_update_inner(struct term_snapshot *s, struct term *t)
return false;
int cols = t->cols, rows = t->rows;
if (!reserve(s, cols, rows)) {
- s->view.size = (struct term_vector2i){ 0 };
- s->source = 0;
+ invalidate(s);
return false;
}
@@ -175,13 +194,17 @@ static bool term_snapshot_update_inner(struct term_snapshot *s, struct term *t)
bool changed = all;
term_hist_prepare_rows(&t->hist, cols);
- if (!term_hist_view_sync(&t->view, &t->hist, cols))
+ if (!term_hist_view_sync(&t->view, &t->hist, cols)) {
+ invalidate(s);
return false;
+ }
term_hist_view_refine(&t->view, &t->hist, cols, rows);
uint64_t stack_keys[256];
uint64_t *new_keys = rows <= 256 ? stack_keys : malloc((size_t)rows * sizeof(uint64_t));
- if (!new_keys)
+ if (!new_keys) {
+ invalidate(s);
return false;
+ }
for (int y = 0; y < rows; y++)
new_keys[y] = term_screen_view_row_key(t, y);
@@ -230,6 +253,8 @@ static bool term_snapshot_update_inner(struct term_snapshot *s, struct term *t)
if (scrolled > 0) {
int k = scrolled;
+ for (int y = 0; y < k; y++)
+ s->arena_dead += s->row_clusters[y];
memmove(s->cells, s->cells + (size_t)k * cols,
(size_t)(rows - k) * cols * sizeof(struct term_cell));
memmove(s->row_clusters, s->row_clusters + k,
@@ -247,7 +272,15 @@ static bool term_snapshot_update_inner(struct term_snapshot *s, struct term *t)
if (!all)
s->arena_dead += s->row_clusters[y];
const struct cell *src = term_screen_view_row(t, y);
- s->row_clusters[y] = convert_row(s, t->gstore, s->cells + (size_t)y * cols, src, cols);
+ bool converted = true;
+ s->row_clusters[y] = convert_row(s, t->gstore, s->cells + (size_t)y * cols,
+ src, cols, &converted);
+ if (!converted) {
+ if (new_keys != stack_keys)
+ free(new_keys);
+ invalidate(s);
+ return false;
+ }
s->keys[y] = key;
changed = true;
}
@@ -256,8 +289,10 @@ static bool term_snapshot_update_inner(struct term_snapshot *s, struct term *t)
int stack[2][256];
int *c0 = rows <= 256 ? stack[0] : malloc((size_t)rows * 2 * sizeof(int));
- if (!c0)
+ if (!c0) {
+ invalidate(s);
return false;
+ }
int *c1 = rows <= 256 ? stack[1] : c0 + rows;
term_selection_view_spans(t, c0, c1);
for (int y = 0; y < rows; y++) {
diff --git a/tests/correctness_probe.c b/tests/correctness_probe.c
index 0b55211..a7c15df 100644
--- a/tests/correctness_probe.c
+++ b/tests/correctness_probe.c
@@ -26,6 +26,7 @@ void *__wrap_realloc(void *ptr, size_t size)
int main(void)
{
+ bool failed = false;
struct term_options options = { .size = { 8, 3 }, .scrollback = 5 };
struct term *t = term_new(&options);
struct term_snapshot *s = term_snapshot_new();
@@ -35,11 +36,14 @@ int main(void)
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);
+ const struct term_cell *row = term_snapshot_row(s, 0);
+ int n = row ? term_snapshot_cluster(s, row[0].text, cps, 16) : 0;
+ printf("snapshot arena allocation failure: update=%d cluster_length=%d (expected false and empty)\n", ok, n);
+ failed |= ok || term_snapshot_view(s)->size.x != 0 || term_snapshot_view(s)->size.y != 0;
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);
+ failed |= n != 2 || cps[0] != 'e' || cps[1] != 0x301;
term_snapshot_free(s);
term_free(t);
@@ -55,6 +59,7 @@ int main(void)
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));
}
+ failed |= s->arena_len >= 8192 || term_history_rows(t) != 5;
term_snapshot_free(s);
term_free(t);
@@ -74,7 +79,9 @@ int main(void)
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));
+ failed |= ok || term_size(t).y != 4 || term_snapshot_row(s, 0)[0].text != 'A' ||
+ term_cursor(t).pos.y != 3 || term_history_rows(t) != 0;
term_snapshot_free(s);
term_free(t);
- return 0;
+ return failed ? 1 : 0;
}
diff --git a/tests/meson.build b/tests/meson.build
index 4cf559c..69f3c49 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -24,6 +24,7 @@ term_tests = [
['gstore-test', files('gstore_test.c'), [], []],
['hist-test', files('hist_test.c'), [], []],
['history-text-test', files('history_text_test.c'), [thread_dep], []],
+ ['snapshot-scroll-arena-test', files('snapshot_scroll_arena_test.c'), [], []],
['view-test', files('view_test.c'), [], []],
['resize-property-test', files('resize_property_test.c'), [], []],
['term-resize-failure-test', files('resize_failure_test.c'), [dl_dep], ['extlib']],
@@ -105,6 +106,17 @@ test('term-header-test', bash,
args: [files('header_test.sh'), meson.project_source_root()],
suite: 'unit',
)
+if cc.has_link_argument('-Wl,--wrap=realloc') and cc.has_link_argument('-Wl,--wrap=malloc')
+ snapshot_failure_test = executable('snapshot-failure-test', 'snapshot_failure_test.c',
+ c_args: test_c_args,
+ link_args: ['-Wl,--wrap=realloc', '-Wl,--wrap=malloc'],
+ dependencies: [term_internal_dep],
+ )
+ test('snapshot-failure-test', snapshot_failure_test,
+ env: test_env,
+ suite: 'unit',
+ )
+endif
test('term-cross-test', bash,
args: [files('cross_test.sh'), meson.project_source_root()],
suite: 'portability',
diff --git a/tests/resize_failure_test.c b/tests/resize_failure_test.c
index 5d7e63c..fbfa38d 100644
--- a/tests/resize_failure_test.c
+++ b/tests/resize_failure_test.c
@@ -146,6 +146,57 @@ int main(void)
assert(tt_cols(term) == 10);
term_free(term);
+ /* A failed same-width height shrink must not scroll rows into history or
+ * move the cursor. The same resize must succeed when allocation recovers. */
+ term = tt_new(8, 4);
+ assert(term);
+ const char four_rows[] = "A\r\nB\r\nC\r\nD";
+ tt_write(term, four_rows, sizeof four_rows - 1);
+ assert(tt_cursor_y(term) == 3);
+ assert(tt_history(term) == 0);
+
+ fail_next_calloc = true;
+ resized = tt_resize(term, 8, 2);
+ assert(!fail_next_calloc);
+ assert(!resized);
+ assert(tt_cols(term) == 8 && tt_rows(term) == 4);
+ assert(tt_cell(term, 0, 0)->text == 'A');
+ assert(tt_cell(term, 0, 1)->text == 'B');
+ assert(tt_cell(term, 0, 2)->text == 'C');
+ assert(tt_cell(term, 0, 3)->text == 'D');
+ assert(tt_cursor_y(term) == 3);
+ assert(tt_history(term) == 0);
+
+ resized = tt_resize(term, 8, 2);
+ assert(resized);
+ assert(tt_cols(term) == 8 && tt_rows(term) == 2);
+ assert(tt_cell(term, 0, 0)->text == 'C');
+ assert(tt_cell(term, 0, 1)->text == 'D');
+ assert(tt_cursor_y(term) == 1);
+ assert(tt_history(term) == 2);
+ term_free(term);
+
+ /* The alternate screen uses the same allocation-before-scroll path. */
+ term = tt_new(8, 4);
+ assert(term);
+ tt_write(term, "\033[?1049h", 8);
+ tt_write(term, four_rows, sizeof four_rows - 1);
+ assert(term_modes(term).alternate);
+ assert(tt_cursor_y(term) == 3);
+ fail_next_calloc = true;
+ resized = tt_resize(term, 8, 2);
+ assert(!fail_next_calloc);
+ assert(!resized);
+ assert(tt_rows(term) == 4 && tt_cursor_y(term) == 3);
+ assert(tt_cell(term, 0, 0)->text == 'A');
+ assert(tt_cell(term, 0, 3)->text == 'D');
+ resized = tt_resize(term, 8, 2);
+ assert(resized);
+ assert(tt_rows(term) == 2 && tt_cursor_y(term) == 1);
+ assert(tt_cell(term, 0, 0)->text == 'C');
+ assert(tt_cell(term, 0, 1)->text == 'D');
+ term_free(term);
+
puts("lazy screen allocation failures preserve observable terminal state");
return 0;
#endif
diff --git a/tests/snapshot_failure_test.c b/tests/snapshot_failure_test.c
new file mode 100644
index 0000000..4133bfd
--- /dev/null
+++ b/tests/snapshot_failure_test.c
@@ -0,0 +1,91 @@
+#include "term/term.h"
+
+#include <assert.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdlib.h>
+
+static bool fail_next_realloc;
+static int malloc_failure_countdown = -1;
+static bool malloc_was_failed;
+void *__real_malloc(size_t);
+void *__wrap_malloc(size_t size)
+{
+ if (malloc_failure_countdown >= 0) {
+ if (malloc_failure_countdown-- == 0) {
+ malloc_failure_countdown = -1;
+ malloc_was_failed = true;
+ return NULL;
+ }
+ }
+ return __real_malloc(size);
+}
+
+void *__real_realloc(void *, size_t);
+void *__wrap_realloc(void *p, size_t size)
+{
+ if (fail_next_realloc) {
+ fail_next_realloc = false;
+ return NULL;
+ }
+ return __real_realloc(p, size);
+}
+
+int main(void)
+{
+ struct term_options options = { .size = { 8, 3 }, .scrollback = 5 };
+ struct term *term = term_new(&options);
+ struct term_snapshot *snapshot = term_snapshot_new();
+ assert(term && snapshot && term_snapshot_update(snapshot, term));
+
+ const char accented[] = "e\xcc\x81";
+ term_feed(term, accented, sizeof(accented) - 1, 0);
+ fail_next_realloc = true;
+ assert(!term_snapshot_update(snapshot, term));
+ assert(!fail_next_realloc);
+ assert(term_snapshot_view(snapshot)->size.x == 0);
+ assert(term_snapshot_view(snapshot)->size.y == 0);
+ assert(term_snapshot_row(snapshot, 0) == NULL);
+
+ assert(term_snapshot_update(snapshot, term));
+ const struct term_cell *row = term_snapshot_row(snapshot, 0);
+ assert(row);
+ uint32_t cps[4];
+ assert(term_snapshot_cluster(snapshot, row[0].text, cps, 4) == 2);
+ assert(cps[0] == 'e' && cps[1] == 0x301);
+
+ term_snapshot_free(snapshot);
+ term_free(term);
+
+ /* More than 256 rows moves both temporary arrays off the stack. */
+ options.size.y = 300;
+ term = term_new(&options);
+ snapshot = term_snapshot_new();
+ assert(term && snapshot && term_snapshot_update(snapshot, term));
+
+ /* Fail new_keys allocation, then make sure retry fills all rows. */
+ malloc_was_failed = false;
+ malloc_failure_countdown = 0;
+ assert(!term_snapshot_update(snapshot, term));
+ assert(malloc_was_failed);
+ assert(term_snapshot_view(snapshot)->size.y == 0);
+ assert(term_snapshot_row(snapshot, 0) == NULL);
+ assert(term_snapshot_update(snapshot, term));
+ assert(term_snapshot_view(snapshot)->size.y == 300);
+ assert(term_snapshot_row(snapshot, 299));
+
+ /* The first malloc is new_keys; fail the following selection-span array. */
+ malloc_was_failed = false;
+ malloc_failure_countdown = 1;
+ assert(!term_snapshot_update(snapshot, term));
+ assert(malloc_was_failed);
+ assert(term_snapshot_view(snapshot)->size.y == 0);
+ assert(term_snapshot_row(snapshot, 0) == NULL);
+ assert(term_snapshot_update(snapshot, term));
+ assert(term_snapshot_view(snapshot)->size.y == 300);
+ assert(term_snapshot_row(snapshot, 299));
+
+ term_snapshot_free(snapshot);
+ term_free(term);
+ return 0;
+}
diff --git a/tests/snapshot_scroll_arena_test.c b/tests/snapshot_scroll_arena_test.c
new file mode 100644
index 0000000..7da6507
--- /dev/null
+++ b/tests/snapshot_scroll_arena_test.c
@@ -0,0 +1,29 @@
+/* Include snapshot.c to inspect its private arena accounting. The static
+ * library supplies the terminal implementation; this translation unit's
+ * snapshot symbols satisfy the test's references before the archive is read.
+ */
+#include <assert.h>
+#include "../src/snapshot.c"
+
+int main(void)
+{
+ struct term_options options = { .size = { 8, 24 }, .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 line[] = "e\xcc\x81\r\n";
+ for (int i = 0; i < 10000; i++) {
+ term_feed(t, line, sizeof(line) - 1, 0);
+ assert(term_snapshot_update(s, t));
+ }
+
+ /* A fixed viewport holds fewer than 24 * 8 cluster records at once.
+ * Dead records are compacted before they can dominate this arena. */
+ assert(term_history_rows(t) == 5);
+ assert(s->arena_len < 8192);
+
+ term_snapshot_free(s);
+ term_free(t);
+ return 0;
+}
generated by cgit v1.2.3 (git 2.39.1) at 2026-10-25 03:38:09 +0000