From 36240b897ca17819a3cb47027191af2c6f48df6d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 24 Mar 2024 20:27:47 -0700 Subject: [PATCH] terminal: many more assertions around spacer state --- src/terminal/Screen.zig | 11 +++++-- src/terminal/Terminal.zig | 17 +++++++++- src/terminal/page.zig | 66 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 51e86d782..6b09823a4 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -820,6 +820,15 @@ pub fn clearCells( row: *Row, cells: []Cell, ) void { + // This whole operation does unsafe things, so we just want to assert + // the end state. + page.pauseIntegrityChecks(true); + defer { + page.pauseIntegrityChecks(false); + page.assertIntegrity(); + self.assertIntegrity(); + } + // If this row has graphemes, then we need go through a slow path // and delete the cell graphemes. if (row.grapheme) { @@ -866,8 +875,6 @@ pub fn clearCells( } @memset(cells, self.blankCell()); - page.assertIntegrity(); - self.assertIntegrity(); } /// Clear cells but only if they are not protected. diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 72b03c12f..ebcf06bf9 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -536,6 +536,12 @@ fn printCell( .spacer_tail => { assert(self.screen.cursor.x > 0); + // So integrity checks pass. We fix this up later so we don't + // need to do this without safety checks. + if (comptime std.debug.runtime_safety) { + cell.wide = .narrow; + } + const wide_cell = self.screen.cursorCellLeft(1); self.screen.clearCells( &self.screen.cursor.page_pin.page.data, @@ -1564,13 +1570,22 @@ pub fn insertBlanks(self: *Terminal, count: usize) void { // "scroll_amount" is the number of such cols. const scroll_amount = rem - adjusted_count; if (scroll_amount > 0) { + page.pauseIntegrityChecks(true); + defer page.pauseIntegrityChecks(false); + var x: [*]Cell = left + (scroll_amount - 1); // If our last cell we're shifting is wide, then we need to clear // it to be empty so we don't split the multi-cell char. const end: *Cell = @ptrCast(x); if (end.wide == .wide) { - self.screen.clearCells(page, self.screen.cursor.page_row, end[0..1]); + const end_multi: [*]Cell = @ptrCast(end); + assert(end_multi[1].wide == .spacer_tail); + self.screen.clearCells( + page, + self.screen.cursor.page_row, + end_multi[0..2], + ); } // We work backwards so we don't overwrite data. diff --git a/src/terminal/page.zig b/src/terminal/page.zig index ea0192b33..b4c75121f 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -100,6 +100,11 @@ pub const Page = struct { /// memory and is fixed at page creation time. capacity: Capacity, + /// If this is true then verifyIntegrity will do nothing. This is + /// only present with runtime safety enabled. + pause_integrity_checks: if (std.debug.runtime_safety) bool else void = + if (std.debug.runtime_safety) false else void, + /// Initialize a new page, allocating the required backing memory. /// The size of the initialized page defaults to the full capacity. /// @@ -191,8 +196,18 @@ pub const Page = struct { UnmarkedStyleRow, MismatchedStyleRef, InvalidStyleCount, + InvalidSpacerTailLocation, + InvalidSpacerHeadLocation, + UnwrappedSpacerHead, }; + /// Temporarily pause integrity checks. This is useful when you are + /// doing a lot of operations that would trigger integrity check + /// violations but you know the page will end up in a consistent state. + pub fn pauseIntegrityChecks(self: *Page, v: bool) void { + if (comptime std.debug.runtime_safety) self.pause_integrity_checks = v; + } + /// A helper that can be used to assert the integrity of the page /// when runtime safety is enabled. This is a no-op when runtime /// safety is disabled. This uses the libc allocator. @@ -220,6 +235,10 @@ pub const Page = struct { // used for the same reason as styles above. // + if (comptime std.debug.runtime_safety) { + if (self.pause_integrity_checks) return; + } + if (self.size.rows == 0) { log.warn("page integrity violation zero row count", .{}); return IntegrityError.ZeroRowCount; @@ -282,6 +301,53 @@ pub const Page = struct { if (!gop.found_existing) gop.value_ptr.* = 0; gop.value_ptr.* += 1; } + + switch (cell.wide) { + .narrow => {}, + .wide => {}, + + .spacer_tail => { + // Spacer tails can't be at the start because they follow + // a wide char. + if (x == 0) { + log.warn( + "page integrity violation y={} x={} spacer tail at start", + .{ y, x }, + ); + return IntegrityError.InvalidSpacerTailLocation; + } + + // Spacer tails must follow a wide char + const prev = cells[x - 1]; + if (prev.wide != .wide) { + log.warn( + "page integrity violation y={} x={} spacer tail not following wide", + .{ y, x }, + ); + return IntegrityError.InvalidSpacerTailLocation; + } + }, + + .spacer_head => { + // Spacer heads must be at the end + if (x != self.size.cols - 1) { + log.warn( + "page integrity violation y={} x={} spacer head not at end", + .{ y, x }, + ); + return IntegrityError.InvalidSpacerHeadLocation; + } + + // The row must be wrapped + if (!row.wrap) { + log.warn( + "page integrity violation y={} spacer head not wrapped", + .{y}, + ); + return IntegrityError.UnwrappedSpacerHead; + } + }, + } } // Check row grapheme data