terminal: fix crash when reflowing grapheme with a spacer head

Fixes #7536

When we're reflowing a row and we need to insert a spacer head, we must
move to the next row to insert it. Previously, we were setting a spacer
head and then copying data into that spacer head, which could lead to
corrupt data and an eventual crash.

In debug builds this triggers assertion failures but in release builds
this would lead to silent corruption and a crash later on.

The unit test shows the issue clearly but effectively you need a
multi-codepoint grapheme such as `👨‍👨‍👦‍👦` to wrap across a row by changing
the columns.
This commit is contained in:
Mitchell Hashimoto
2025-06-06 20:36:34 -07:00
parent 269d29624b
commit aab00da242
2 changed files with 144 additions and 12 deletions

View File

@@ -908,16 +908,6 @@ const ReflowCursor = struct {
const cell = &cells[x];
x += 1;
// std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={} wide={}", .{
// src_y,
// x,
// self.y,
// self.x,
// self.page.size.cols,
// cell.content.codepoint,
// cell.wide,
// });
// Copy cell contents.
switch (cell.content_tag) {
.codepoint,
@@ -937,8 +927,15 @@ const ReflowCursor = struct {
};
// Decrement the source position so that when we
// loop we'll process this source cell again.
// loop we'll process this source cell again,
// since we can't copy it into a spacer head.
x -= 1;
// Move to the next row (this sets pending wrap
// which will cause us to wrap on the next
// iteration).
self.cursorForward();
continue;
} else {
self.page_cell.* = cell.*;
}
@@ -990,6 +987,17 @@ const ReflowCursor = struct {
self.page_cell.hyperlink = false;
self.page_cell.style_id = stylepkg.default_id;
// std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={X} wide={} page_cell_wide={}", .{
// src_y,
// x,
// self.y,
// self.x,
// self.page.size.cols,
// cell.content.codepoint,
// cell.wide,
// self.page_cell.wide,
// });
// Copy grapheme data.
if (cell.content_tag == .codepoint_grapheme) {
// Copy the graphemes
@@ -8375,6 +8383,125 @@ test "PageList resize reflow less cols to wrap a wide char" {
}
}
test "PageList resize reflow less cols to wrap a multi-codepoint grapheme with a spacer head" {
const testing = std.testing;
const alloc = testing.allocator;
var s = try init(alloc, 4, 2, 0);
defer s.deinit();
{
try testing.expect(s.pages.first == s.pages.last);
const page = &s.pages.first.?.data;
// We want to make the screen look like this:
//
// 👨‍👨‍👦‍👦👨‍👨‍👦‍👦
// First family emoji at (0, 0)
{
const rac = page.getRowAndCell(0, 0);
rac.cell.* = .{
.content_tag = .codepoint,
.content = .{ .codepoint = 0x1F468 }, // First codepoint of the grapheme
.wide = .wide,
};
try page.setGraphemes(rac.row, rac.cell, &.{
0x200D, 0x1F468,
0x200D, 0x1F466,
0x200D, 0x1F466,
});
}
{
const rac = page.getRowAndCell(1, 0);
rac.cell.* = .{
.content_tag = .codepoint,
.content = .{ .codepoint = 0 },
.wide = .spacer_tail,
};
}
// Second family emoji at (2, 0)
{
const rac = page.getRowAndCell(2, 0);
rac.cell.* = .{
.content_tag = .codepoint,
.content = .{ .codepoint = 0x1F468 }, // First codepoint of the grapheme
.wide = .wide,
};
try page.setGraphemes(rac.row, rac.cell, &.{
0x200D, 0x1F468,
0x200D, 0x1F466,
0x200D, 0x1F466,
});
}
{
const rac = page.getRowAndCell(3, 0);
rac.cell.* = .{
.content_tag = .codepoint,
.content = .{ .codepoint = 0 },
.wide = .spacer_tail,
};
}
}
// Resize
try s.resize(.{ .cols = 3, .reflow = true });
try testing.expectEqual(@as(usize, 3), s.cols);
try testing.expectEqual(@as(usize, 2), s.totalRows());
{
try testing.expect(s.pages.first == s.pages.last);
const page = &s.pages.first.?.data;
{
const rac = page.getRowAndCell(0, 0);
try testing.expectEqual(@as(u21, 0x1F468), rac.cell.content.codepoint);
try testing.expectEqual(pagepkg.Cell.Wide.wide, rac.cell.wide);
const cps = page.lookupGrapheme(rac.cell).?;
try testing.expectEqual(@as(usize, 6), cps.len);
try testing.expectEqual(@as(u21, 0x200D), cps[0]);
try testing.expectEqual(@as(u21, 0x1F468), cps[1]);
try testing.expectEqual(@as(u21, 0x200D), cps[2]);
try testing.expectEqual(@as(u21, 0x1F466), cps[3]);
try testing.expectEqual(@as(u21, 0x200D), cps[4]);
try testing.expectEqual(@as(u21, 0x1F466), cps[5]);
// Row should be wrapped
try testing.expect(rac.row.wrap);
}
{
const rac = page.getRowAndCell(1, 0);
try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint);
try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide);
}
{
const rac = page.getRowAndCell(2, 0);
try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint);
try testing.expectEqual(pagepkg.Cell.Wide.spacer_head, rac.cell.wide);
}
{
const rac = page.getRowAndCell(0, 0);
try testing.expectEqual(@as(u21, 0x1F468), rac.cell.content.codepoint);
try testing.expectEqual(pagepkg.Cell.Wide.wide, rac.cell.wide);
const cps = page.lookupGrapheme(rac.cell).?;
try testing.expectEqual(@as(usize, 6), cps.len);
try testing.expectEqual(@as(u21, 0x200D), cps[0]);
try testing.expectEqual(@as(u21, 0x1F468), cps[1]);
try testing.expectEqual(@as(u21, 0x200D), cps[2]);
try testing.expectEqual(@as(u21, 0x1F466), cps[3]);
try testing.expectEqual(@as(u21, 0x200D), cps[4]);
try testing.expectEqual(@as(u21, 0x1F466), cps[5]);
}
{
const rac = page.getRowAndCell(1, 1);
try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint);
try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide);
}
}
}
test "PageList resize reflow less cols copy kitty placeholder" {
const testing = std.testing;
const alloc = testing.allocator;

View File

@@ -1316,7 +1316,12 @@ pub const Page = struct {
/// Set the graphemes for the given cell. This asserts that the cell
/// has no graphemes set, and only contains a single codepoint.
pub fn setGraphemes(self: *Page, row: *Row, cell: *Cell, cps: []u21) GraphemeError!void {
pub fn setGraphemes(
self: *Page,
row: *Row,
cell: *Cell,
cps: []const u21,
) GraphemeError!void {
defer self.assertIntegrity();
assert(cell.codepoint() > 0);