From d2ec05a102dbd4c06a6fe9debbe9757e6a58b77d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 18 Jul 2025 11:36:41 -0700 Subject: [PATCH] terminal: viewport_pin must be initialized Even though the viewport pin isn't used unless the `viewport` is `pin`, it's still possible to access undefined data through `clone`. Valgrind found this: ``` ==107091== Conditional jump or move depends on uninitialised value(s) ==107091== at 0x392B96A: terminal.PageList.clone (PageList.zig:540) ==107091== by 0x392C9A0: terminal.Screen.clonePool (Screen.zig:348) ==107091== by 0x392DF7A: terminal.Screen.clone (Screen.zig:330) ==107091== by 0x394E6D4: renderer.generic.Renderer(renderer.OpenGL).updateFrame (generic.zig:1129) ==107091== by 0x3919BF8: renderer.Thread.renderCallback (Thread.zig:607) ==107091== by 0x3919A6F: renderer.Thread.wakeupCallback (Thread.zig:524) ==107091== by 0x394FA6E: callback (async.zig:679) ==107091== by 0x394FA6E: watcher.async.AsyncEventFd(api.Xev(.io_uring,backend.io_uring)).waitPoll__anon_436371__struct_440666.callback (async.zig:181) ==107091== by 0x38F781E: backend.io_uring.Completion.invoke (io_uring.zig:804) ==107091== by 0x38FA448: backend.io_uring.Loop.tick___anon_431479 (io_uring.zig:193) ==107091== by 0x38FA53D: backend.io_uring.Loop.run (io_uring.zig:84) ==107091== by 0x38FEFE3: dynamic.Xev(&.{ .io_uring, .epoll }[0..2]).Loop.run (dynamic.zig:172) ==107091== by 0x38FF2E2: renderer.Thread.threadMain_ (Thread.zig:263) ==107091== by 0x38DDF80: renderer.Thread.threadMain (Thread.zig:202) ==107091== by 0x38B5C0A: Thread.callFn__anon_421402 (Thread.zig:488) ==107091== by 0x3888604: Thread.PosixThreadImpl.spawn__anon_418943.Instance.entryFn (Thread.zig:757) ==107091== by 0x6C6E7EA: start_thread (pthread_create.c:448) ==107091== by 0x6CF1FB3: clone (clone.S:100) ==107091== ``` --- src/terminal/PageList.zig | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 660949c9c..a7563ac8c 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -244,6 +244,7 @@ pub fn init( // We always track our viewport pin to ensure this is never an allocation const viewport_pin = try pool.pins.create(); + viewport_pin.* = .{ .node = page_list.first.? }; var tracked_pins: PinSet = .{}; errdefer tracked_pins.deinit(pool.alloc); try tracked_pins.putNoClobber(pool.alloc, viewport_pin, {}); @@ -3808,6 +3809,10 @@ test "PageList" { try testing.expect(s.pages.first != null); try testing.expectEqual(@as(usize, s.rows), s.totalRows()); + // Our viewport pin must be defined. It isn't used until the + // viewport is a pin but it prevents undefined access on clone. + try testing.expect(s.viewport_pin.node == s.pages.first.?); + // Active area should be the top try testing.expectEqual(Pin{ .node = s.pages.first.?,