terminal: various PageList hardening for capacity edge cases (#13244)

Each commit is standalone. This contains various unoffensive hardening
of edge cases in PageList layout stuff. None of this is reachable today,
but I'm just buttoning this up to experiment with compaction/compression
of pages in various scenarios.

src/terminal/PageList.zig

@@ -642,8 +642,7 @@ fn verifyIntegrity(self: *const PageList) IntegrityError!void {
     }
 }
 
-/// Deinit the pagelist. If you own the memory pool (used clonePool) then
-/// this will reset the pool and retain capacity.
+/// Deinit the pagelist, freeing all page memory and the memory pool.
 pub fn deinit(self: *PageList) void {
     // Verify integrity before cleanup
     self.assertIntegrity();
@@ -3346,21 +3345,36 @@ pub fn increaseCapacity(
             const Int = @FieldType(Capacity, field_name);
             const old = @field(cap, field_name);
 
-            // We use checked math to prevent overflow. If there is an
-            // overflow it means we're out of space in this dimension,
-            // since pages can take up to their maxInt capacity in any
-            // category.
-            const new = std.math.mul(
-                Int,
-                old,
-                2,
-            ) catch |err| overflow: {
-                comptime assert(@TypeOf(err) == error{Overflow});
-                // Our final doubling would overflow since maxInt is
-                // 2^N - 1 for an unsignged int of N bits. So, if we overflow
-                // and we haven't used all the bits, use all the bits.
-                if (old < std.math.maxInt(Int)) break :overflow std.math.maxInt(Int);
-                return error.OutOfSpace;
+            const new: Int = new: {
+                // A dimension can be zero for pages with exact
+                // capacities (see compact). Doubling zero stays zero,
+                // which would break our guarantee that we always
+                // increase by at least one unit and turn caller retry
+                // loops into infinite loops. Jump straight to the
+                // standard default instead: it is what all standard
+                // pages start with, so retrying callers are guaranteed
+                // enough room for their pending allocation.
+                if (old == 0) {
+                    const default: Capacity = .{ .cols = 0, .rows = 0 };
+                    break :new @field(default, field_name);
+                }
+
+                // We use checked math to prevent overflow. If there is
+                // an overflow it means we're out of space in this
+                // dimension, since pages can take up to their maxInt
+                // capacity in any category.
+                break :new std.math.mul(
+                    Int,
+                    old,
+                    2,
+                ) catch |err| overflow: {
+                    comptime assert(@TypeOf(err) == error{Overflow});
+                    // Our final doubling would overflow since maxInt is
+                    // 2^N - 1 for an unsignged int of N bits. So, if we overflow
+                    // and we haven't used all the bits, use all the bits.
+                    if (old < std.math.maxInt(Int)) break :overflow std.math.maxInt(Int);
+                    return error.OutOfSpace;
+                };
             };
             @field(cap, field_name) = new;
 
@@ -3484,6 +3498,14 @@ inline fn createPageExt(
 
     // In runtime safety modes, allocators fill with 0xAA. On freestanding
     // (WASM), the WasmAllocator reuses freed slots without zeroing.
+    //
+    // Otherwise, we rely on pool item buffers being zeroed: fresh items
+    // come from the OS page allocator (zeroed pages) and destroyNodeExt
+    // zeroes buffers before returning them to the pool. The one
+    // exception is the first pointer-size bytes, which hold the pool's
+    // free list node while a buffer is in the free list; initBuf below
+    // always overwrites those since the page rows start at offset 0
+    // (comptime-asserted in Page).
     if (comptime std.debug.runtime_safety or builtin.os.tag == .freestanding)
         @memset(page_buf, 0);
 
@@ -14322,6 +14344,40 @@ test "PageList compact oversized page" {
     }
 }
 
+test "PageList increaseCapacity from zero-capacity dimensions" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s = try init(alloc, 80, 24, 0);
+    defer s.deinit();
+
+    // Compact the only page. A plain page has no styled, grapheme,
+    // or hyperlink content so the exact capacity is zero in every
+    // managed dimension.
+    var node = (try s.compact(s.pages.first.?)).?;
+    try testing.expectEqual(0, node.data.capacity.styles);
+    try testing.expectEqual(0, node.data.capacity.grapheme_bytes);
+    try testing.expectEqual(0, node.data.capacity.string_bytes);
+    try testing.expectEqual(0, node.data.capacity.hyperlink_bytes);
+
+    // Increasing each dimension from zero must actually grow it.
+    // Regression: 0 * 2 == 0 used to "succeed" without growing,
+    // which turned caller retry loops into infinite loops.
+    node = try s.increaseCapacity(node, .styles);
+    try testing.expect(node.data.capacity.styles > 0);
+    node = try s.increaseCapacity(node, .grapheme_bytes);
+    try testing.expect(node.data.capacity.grapheme_bytes > 0);
+    node = try s.increaseCapacity(node, .string_bytes);
+    try testing.expect(node.data.capacity.string_bytes > 0);
+    node = try s.increaseCapacity(node, .hyperlink_bytes);
+    try testing.expect(node.data.capacity.hyperlink_bytes > 0);
+
+    // Increasing a non-zero dimension still doubles.
+    const styles = node.data.capacity.styles;
+    node = try s.increaseCapacity(node, .styles);
+    try testing.expectEqual(styles * 2, node.data.capacity.styles);
+}
+
 test "PageList compact after increaseCapacity" {
     const testing = std.testing;
     const alloc = testing.allocator;

src/terminal/Screen.zig

@@ -3775,6 +3775,54 @@ test "Screen cursorCopy hyperlink deref" {
     try testing.expect(s2.cursor.hyperlink_id == 0);
 }
 
+test "Screen write regrows compacted page capacity" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s = try Screen.init(alloc, .{
+        .cols = 80,
+        .rows = 24,
+        .max_scrollback = 0,
+    });
+    defer s.deinit();
+
+    // Compact the active page so every managed capacity dimension is
+    // zero, then reload the cursor since its cached row/cell pointers
+    // point into the replaced page.
+    {
+        const node = (try s.pages.compact(s.cursor.page_pin.node)).?;
+        try testing.expectEqual(0, node.data.capacity.styles);
+        try testing.expectEqual(0, node.data.capacity.grapheme_bytes);
+        try testing.expectEqual(0, node.data.capacity.string_bytes);
+        try testing.expectEqual(0, node.data.capacity.hyperlink_bytes);
+        s.cursorReload();
+    }
+
+    // Styled write: exercises the manualStyleUpdate single-retry
+    // path. Prior to increaseCapacity handling zero dimensions, the
+    // retry would fail and the style would be dropped.
+    try s.setAttribute(.{ .bold = {} });
+    try s.testWriteString("A");
+
+    // Grapheme write: exercises the appendGrapheme single-retry path.
+    // We can't use testWriteString here because it appends graphemes
+    // directly on the page without the capacity retry.
+    try s.testWriteString("a");
+    try s.appendGrapheme(s.cursorCellLeft(1), 0x0301);
+
+    // Hyperlink: exercises the startHyperlink retry loop, which used
+    // to loop forever when capacity growth from zero didn't grow.
+    try s.startHyperlink("https://example.com/", null);
+    try s.testWriteString("B");
+    s.endHyperlink();
+
+    // Verify the content landed on the page.
+    const page = &s.cursor.page_pin.node.data;
+    try testing.expect(page.styles.count() >= 1);
+    try testing.expect(page.hyperlink_set.count() >= 1);
+    try testing.expect(page.graphemeCount() >= 1);
+}
+
 test "Screen cursorCopy hyperlink deref new page" {
     const testing = std.testing;
     const alloc = testing.allocator;
@@ -10133,9 +10181,9 @@ test "Screen: cursorDown to page with insufficient capacity" {
     // cursor movement functions). The bug pattern:
     //
     // 1. cursorDown creates a by-value copy of the pin via page_pin.down(n)
-    // 2. cursorChangePin is called, which may trigger adjustCapacity
+    // 2. cursorChangePin is called, which may trigger increaseCapacity
     //    if the target page's style map is full
-    // 3. adjustCapacity frees the old page and creates a new one
+    // 3. increaseCapacity frees the old page and creates a new one
     // 4. The local pin copy still points to the freed page
     // 5. rowAndCell() on the stale pin accesses freed memory
 
@@ -10158,7 +10206,7 @@ test "Screen: cursorDown to page with insufficient capacity" {
     try testing.expect(start_page != new_page);
 
     // Fill new_page's style map to capacity. When we move INTO this page
-    // with a style set, adjustCapacity will be triggered.
+    // with a style set, increaseCapacity will be triggered.
     {
         new_page.pauseIntegrityChecks(true);
         defer new_page.pauseIntegrityChecks(false);
@@ -10187,7 +10235,7 @@ test "Screen: cursorDown to page with insufficient capacity" {
                 try testing.expect(&next_pin.node.data == new_page);
 
                 // This cursorDown triggers the bug: the local page_pin copy
-                // becomes stale after adjustCapacity, causing rowAndCell()
+                // becomes stale after increaseCapacity, causing rowAndCell()
                 // to access freed memory.
                 s.cursorDown(1);
 

src/terminal/page.zig

@@ -145,6 +145,15 @@ pub const Page = struct {
             @alignOf(Cell),
             StyleSet.base_align.toByteUnits(),
         ) == 0);
+
+        // The PageList memory pool requires that initBuf overwrites at
+        // least the first pointer-size bytes of the backing buffer:
+        // std.heap.MemoryPool stores its free list node there when a
+        // page buffer is returned to it, and pool reuse skips zeroing
+        // in release builds. This holds because the rows array is at
+        // offset 0 (see layout), a page always has at least one row,
+        // and initBuf fully rewrites every row.
+        assert(@sizeOf(Row) >= @sizeOf(usize));
     }
 
     /// The backing memory for the page. A page is always made up of a
@@ -238,6 +247,13 @@ pub const Page = struct {
     /// It is up to the caller to not call deinit on these pages.
     pub inline fn initBuf(buf: OffsetBuf, l: Layout) Page {
         const cap = l.capacity;
+
+        // A page must always have at least one row. Aside from being
+        // useless otherwise, the row initialization below must always
+        // overwrite the start of the buffer for pool reuse. See the
+        // comptime assert at the top of Page.
+        assert(cap.rows > 0);
+
         const rows = buf.member(Row, l.rows_start);
         const cells = buf.member(Cell, l.cells_start);
 
@@ -1703,6 +1719,11 @@ pub const Page = struct {
     /// and rows size.
     pub inline fn layout(cap: Capacity) Layout {
         const rows_count: usize = @intCast(cap.rows);
+
+        // The rows array must stay at offset 0: the PageList memory
+        // pool relies on initBuf overwriting the first bytes of a
+        // reused page buffer, which hold the pool's free list node.
+        // See the comptime assert at the top of Page.
         const rows_start = 0;
         const rows_end: usize = rows_start + (rows_count * @sizeOf(Row));
 

src/terminal/ref_counted_set.zig

@@ -497,6 +497,15 @@ pub fn RefCountedSet(
             return self.lookupContext(base, value, self.context);
         }
         pub fn lookupContext(self: *const Self, base: anytype, value: T, ctx: Context) ?Id {
+            // A zero-capacity set (a valid special case of Layout.init)
+            // contains nothing and has a zero-size table, so we can't
+            // probe it: table[0] would read whatever memory follows the
+            // set in the backing buffer.
+            if (self.layout.table_cap == 0) {
+                @branchHint(.cold);
+                return null;
+            }
+
             const table = self.table.ptr(base);
             const items = self.items.ptr(base);
 

src/terminal/style.zig

@@ -966,6 +966,36 @@ test "Set capacities" {
     _ = Set.Layout.init(16384);
 }
 
+test "Set zero capacity" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    // A zero-capacity set is a valid special case (see Layout.init).
+    // It occurs in practice for pages with exact capacities where no
+    // cell is styled (see Page.exactRowCapacity).
+    const layout: Set.Layout = .init(0);
+    try testing.expectEqual(0, layout.total_size);
+
+    // We allocate a nonzero buffer filled with 0xFF to simulate the
+    // set being embedded in a larger structure (e.g. a Page) where
+    // other data follows it. Lookups must not probe the zero-size
+    // table: table[0] would read this adjacent memory and treat it
+    // as an item ID, leading to far out-of-bounds item reads.
+    const buf = try alloc.alignedAlloc(u8, Set.base_align, 64);
+    defer alloc.free(buf);
+    @memset(buf, 0xFF);
+
+    var set = Set.init(.init(buf), layout, .{});
+
+    const style: Style = .{ .flags = .{ .bold = true } };
+    try testing.expectEqual(null, set.lookup(buf, style));
+    try testing.expectError(error.OutOfMemory, set.add(buf, style));
+    try testing.expectEqual(0, set.count());
+
+    // The adjacent memory must be untouched.
+    for (buf) |b| try testing.expectEqual(0xFF, b);
+}
+
 test "Style HTML formatting basic bold" {
     const testing = std.testing;
     const alloc = testing.allocator;