From dd2edd760da6b90b01ef88f75e1bea8a71b31e3f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Sep 2026 20:41:55 -0700 Subject: [PATCH] libvt: return safe pointers for empty output Normalize empty buffers and borrowed strings at the libghostty-vt C boundary to null pointers. Zig can use sentinel addresses such as 0x1 for empty slices. Returning these pointers to Go can cause a fatal invalid-pointer error when the runtime relocates a goroutine's stack, even though the length is zero. --- include/ghostty/vt/allocator.h | 3 +- include/ghostty/vt/formatter.h | 2 ++ include/ghostty/vt/selection.h | 2 ++ include/ghostty/vt/terminal.h | 3 +- include/ghostty/vt/types.h | 2 ++ src/lib/types.zig | 16 +++++++++- src/terminal/c/allocator.zig | 5 ++- src/terminal/c/formatter.zig | 45 +++++++++++++++++++++++++- src/terminal/c/selection.zig | 34 +++++++++++++++++++- src/terminal/c/sys.zig | 2 +- src/terminal/c/terminal.zig | 58 +++++++++++++++++++--------------- 11 files changed, 140 insertions(+), 32 deletions(-) diff --git a/include/ghostty/vt/allocator.h b/include/ghostty/vt/allocator.h index e12d10e47..f68ffa2f4 100644 --- a/include/ghostty/vt/allocator.h +++ b/include/ghostty/vt/allocator.h @@ -224,7 +224,8 @@ typedef struct GhosttyAllocator { * * @param allocator Pointer to the allocator to use, or NULL for the default * @param len Number of bytes to allocate - * @return Pointer to the allocated buffer, or NULL if allocation failed + * @return Pointer to the allocated buffer, or NULL if len is zero or + * allocation failed * * @ingroup allocator */ diff --git a/include/ghostty/vt/formatter.h b/include/ghostty/vt/formatter.h index 210f42284..fd593b139 100644 --- a/include/ghostty/vt/formatter.h +++ b/include/ghostty/vt/formatter.h @@ -196,6 +196,8 @@ GHOSTTY_API GhosttyResult ghostty_formatter_format_buf(GhosttyFormatter formatte * The caller is responsible for freeing the returned buffer with * ghostty_free(), passing the same allocator (or NULL for the default) * that was used for the allocation. + * Empty output returns GHOSTTY_SUCCESS with *out_ptr set to NULL and + * *out_len set to zero. This result can be passed to ghostty_free(). * * @param formatter The formatter handle (must not be NULL) * @param allocator Pointer to allocator, or NULL to use the default allocator diff --git a/include/ghostty/vt/selection.h b/include/ghostty/vt/selection.h index 27d3c47b2..0158965e9 100644 --- a/include/ghostty/vt/selection.h +++ b/include/ghostty/vt/selection.h @@ -907,6 +907,8 @@ GHOSTTY_API GhosttyResult ghostty_terminal_selection_format_buf( * The returned buffer is allocated using allocator, or the default allocator * if NULL is passed. The caller owns the returned buffer and must free it with * ghostty_free(), passing the same allocator and returned length. + * Empty output returns GHOSTTY_SUCCESS with *out_ptr set to NULL and + * *out_len set to zero. This result can be passed to ghostty_free(). * * The returned bytes are not NUL-terminated. This supports plain text, VT, and * HTML uniformly as byte output. diff --git a/include/ghostty/vt/terminal.h b/include/ghostty/vt/terminal.h index 172674bf5..db5420d2f 100644 --- a/include/ghostty/vt/terminal.h +++ b/include/ghostty/vt/terminal.h @@ -2176,7 +2176,8 @@ GHOSTTY_API GhosttyResult ghostty_terminal_continuation_buf( * The returned bytes are allocated with allocator, or the default allocator * when allocator is NULL. The caller must release them with ghostty_free(), * passing the same allocator and returned length. An empty continuation is a - * successful zero-length allocation. + * successful result with *out_ptr set to NULL and *out_len set to zero, + * which can also be passed to ghostty_free(). * Continuation tracking must have been enabled by setting * GHOSTTY_TERMINAL_OPT_CONTINUATION_MAX_BYTES to a nonzero value before the * input that produced the continuation was written. diff --git a/include/ghostty/vt/types.h b/include/ghostty/vt/types.h index 8c5f35f21..059bc5c8f 100644 --- a/include/ghostty/vt/types.h +++ b/include/ghostty/vt/types.h @@ -262,6 +262,8 @@ typedef enum GHOSTTY_ENUM_TYPED { * * The memory is not owned by this struct. The pointer is only valid * for the lifetime documented by the API that produces or consumes it. + * Empty strings produced by the library have a non-NULL pointer to valid + * storage. */ typedef struct { /** Pointer to the string bytes. */ diff --git a/src/lib/types.zig b/src/lib/types.zig index 010522496..fbb50af8c 100644 --- a/src/lib/types.zig +++ b/src/lib/types.zig @@ -5,7 +5,10 @@ pub const String = extern struct { pub fn init(zig: anytype) String { return switch (@TypeOf(zig)) { []u8, []const u8 => .{ - .ptr = zig.ptr, + // Borrowed strings keep a non-null pointer, but it must point + // to real storage: empty Zig slices can use invalid sentinels + // that foreign runtimes reject even without dereferencing. + .ptr = if (zig.len == 0) "" else zig.ptr, .len = zig.len, }, else => @compileError("unsupported String.init type: " ++ @typeName(@TypeOf(zig))), @@ -18,3 +21,14 @@ pub const Buffer = extern struct { cap: usize = 0, len: usize = 0, }; + +test "String.init empty output" { + const std = @import("std"); + const empty = try std.testing.allocator.alloc(u8, 0); + defer std.testing.allocator.free(empty); + const str = String.init(empty); + try std.testing.expectEqual(@as(usize, 0), str.len); + // Empty borrowed strings use the same real storage as an empty literal, + // never Zig's zero-length allocation sentinel. + try std.testing.expectEqual(@as([*]const u8, ""), str.ptr); +} diff --git a/src/terminal/c/allocator.zig b/src/terminal/c/allocator.zig index ca698f3c8..c6732375d 100644 --- a/src/terminal/c/allocator.zig +++ b/src/terminal/c/allocator.zig @@ -7,11 +7,13 @@ const CAllocator = lib.alloc.Allocator; /// (or the default allocator if NULL). /// /// Returns a pointer to the allocated buffer, or NULL if the -/// allocation failed. +/// allocation failed or `len` is zero. pub fn alloc( alloc_: ?*const CAllocator, len: usize, ) callconv(lib.calling_conv) ?[*]u8 { + // Zig's empty allocation pointer is not safe to expose to foreign runtimes. + if (len == 0) return null; const allocator = lib.alloc.default(alloc_); const buf = allocator.alloc(u8, len) catch return null; return buf.ptr; @@ -47,6 +49,7 @@ test "alloc with null allocator" { test "alloc zero length" { const ptr = alloc(&lib.alloc.test_allocator, 0); defer free(&lib.alloc.test_allocator, ptr, 0); + try testing.expectEqual(null, ptr); } test "free null pointer" { diff --git a/src/terminal/c/formatter.zig b/src/terminal/c/formatter.zig index aad33a14e..4ab41cd47 100644 --- a/src/terminal/c/formatter.zig +++ b/src/terminal/c/formatter.zig @@ -234,7 +234,9 @@ pub fn format_alloc( } const buf = aw.toOwnedSlice() catch return .out_of_memory; - out_ptr.* = buf.ptr; + // Empty Zig slices may contain sentinel pointers that foreign runtimes + // reject even when the length is zero (for example Go's stack scanner). + out_ptr.* = if (buf.len == 0) null else buf.ptr; out_len.* = buf.len; return .success; } @@ -281,6 +283,47 @@ test "free null" { free(null); } +test "format_alloc empty output" { + const failing: CAllocator = .fromZig(&std.mem.Allocator.failing); + var t: terminal_c.Terminal = null; + try testing.expectEqual(Result.success, terminal_c.new( + &lib.alloc.test_allocator, + &t, + 100, + 40, + )); + defer terminal_c.free(t); + + // Match a zero-initialized C options struct, including trim = false. + const opts: TerminalOptions = std.mem.zeroInit(TerminalOptions, .{}); + var f: Formatter = null; + try testing.expectEqual(Result.success, terminal_new( + &lib.alloc.test_allocator, + &f, + t, + opts, + )); + defer free(f); + + for ([_]?*const CAllocator{ null, &failing }) |allocator| { + var sentinel: u8 = 0; + var ptr: ?[*]u8 = @ptrCast(&sentinel); + var len: usize = 123; + try testing.expectEqual(Result.success, format_alloc(f, allocator, &ptr, &len)); + defer @import("allocator.zig").free(allocator, ptr, len); + try testing.expectEqual(@as(usize, 0), len); + try testing.expectEqual(null, ptr); + } + + // Reusing the same formatter must still allocate and return real output. + terminal_c.vt_write(t, "hello", 5); + var ptr: ?[*]u8 = null; + var len: usize = 0; + try testing.expectEqual(Result.success, format_alloc(f, null, &ptr, &len)); + defer @import("allocator.zig").free(null, ptr, len); + try testing.expectEqualStrings("hello", ptr.?[0..len]); +} + test "format plain" { var t: terminal_c.Terminal = null; try testing.expectEqual(Result.success, terminal_c.new( diff --git a/src/terminal/c/selection.zig b/src/terminal/c/selection.zig index 77d518d1f..2830149f7 100644 --- a/src/terminal/c/selection.zig +++ b/src/terminal/c/selection.zig @@ -246,7 +246,8 @@ pub fn format_alloc( }; const buf = aw.toOwnedSlice() catch return .out_of_memory; - out_ptr.* = buf.ptr; + // Do not expose Zig's empty slice sentinel through the C ABI. + out_ptr.* = if (buf.len == 0) null else buf.ptr; out_len.* = buf.len; return .success; } @@ -404,6 +405,37 @@ pub fn equal( return .success; } +test "selection_format_alloc empty output" { + const failing: CAllocator = .fromZig(&std.mem.Allocator.failing); + var t: terminal_c.Terminal = null; + try testing.expectEqual(Result.success, terminal_c.new( + &lib.alloc.test_allocator, + &t, + 80, + 24, + )); + defer terminal_c.free(t); + + var ref: grid_ref.CGridRef = .{}; + try testing.expectEqual(Result.success, terminal_c.grid_ref(t, .{ + .tag = .active, + .value = .{ .active = .{ .x = 0, .y = 0 } }, + }, &ref)); + const sel: CSelection = .{ .start = ref, .end = ref }; + try testing.expectEqual(Result.success, terminal_c.set(t, .selection, &sel)); + + var ptr: ?[*]u8 = null; + var len: usize = 123; + try testing.expectEqual(Result.success, format_alloc(t, &failing, .{ + .emit = .plain, + .unwrap = true, + .trim = true, + }, &ptr, &len)); + defer @import("allocator.zig").free(&failing, ptr, len); + try testing.expectEqual(@as(usize, 0), len); + try testing.expectEqual(null, ptr); +} + test "selection_format_alloc uses active selection" { var t: terminal_c.Terminal = null; try testing.expectEqual(Result.success, terminal_c.new( diff --git a/src/terminal/c/sys.zig b/src/terminal/c/sys.zig index 40e7e1c84..f622e8fdb 100644 --- a/src/terminal/c/sys.zig +++ b/src/terminal/c/sys.zig @@ -97,7 +97,7 @@ fn decodePngWrapper( const c_alloc = CAllocator.fromZig(&alloc); var out: Image = undefined; - if (!func(global.userdata, &c_alloc, data.ptr, data.len, &out)) return error.InvalidData; + if (!func(global.userdata, &c_alloc, lib.String.init(data).ptr, data.len, &out)) return error.InvalidData; const result_data = out.data orelse return error.InvalidData; diff --git a/src/terminal/c/terminal.zig b/src/terminal/c/terminal.zig index 4f72bcbdd..04c3e242b 100644 --- a/src/terminal/c/terminal.zig +++ b/src/terminal/c/terminal.zig @@ -415,14 +415,8 @@ const Effects = struct { for (contents, write.contents) |*c_content, content| { c_content.* = .{ - .mime = .{ - .ptr = content.mime.ptr, - .len = content.mime.len, - }, - .data = .{ - .ptr = content.data.ptr, - .len = content.data.len, - }, + .mime = .init(content.mime), + .data = .init(content.data), }; } @@ -559,14 +553,8 @@ const Effects = struct { const func = wrapper.effects.desktop_notification orelse return; const request: DesktopNotification = .{ .size = @sizeOf(DesktopNotification), - .title = .{ - .ptr = notification.title.ptr, - .len = notification.title.len, - }, - .body = .{ - .ptr = notification.body.ptr, - .len = notification.body.len, - }, + .title = .init(notification.title), + .body = .init(notification.body), }; func(@ptrCast(wrapper), wrapper.effects.userdata, &request); } @@ -661,10 +649,7 @@ const Effects = struct { .apc => |apc_value| .{ .apc = .{ .truncated = apc_value.truncated, - .content = .{ - .ptr = apc_value.content.ptr, - .len = apc_value.content.len, - }, + .content = .init(apc_value.content), }, }, }); @@ -1108,7 +1093,9 @@ pub fn continuation_alloc( // Ownership crosses the ABI here; callers release this exact pointer and // length with ghostty_free and the same allocator selection. - out_ptr.* = bytes.ptr; + // An idle parser has no continuation. Never export the empty Zig slice's + // sentinel pointer to a foreign runtime. + out_ptr.* = if (bytes.len == 0) null else bytes.ptr; out_len.* = bytes.len; return .success; } @@ -1752,7 +1739,7 @@ fn getTyped( .enabled => |d| d.directory, .disabled => "", }; - out.* = .{ .ptr = dir.ptr, .len = dir.len }; + out.* = .init(dir); }, .kitty_image_medium_shared_mem => { if (comptime !build_options.kitty_graphics) return .no_value; @@ -2018,9 +2005,9 @@ test "continuation buffer and allocator export exact suffix" { &out_ptr, &out_len, )); - const allocated = out_ptr orelse return error.TestExpectedEqual; - defer lib.alloc.default(&lib.alloc.test_allocator).free(allocated[0..out_len]); - try testing.expectEqualStrings("\x1b[31", allocated[0..out_len]); + const allocated = (out_ptr orelse return error.TestExpectedEqual)[0..out_len]; + defer lib.alloc.default(&lib.alloc.test_allocator).free(allocated); + try testing.expectEqualStrings("\x1b[31", allocated); vt_write(t, "m", 1); try testing.expectEqual( @@ -2028,6 +2015,13 @@ test "continuation buffer and allocator export exact suffix" { continuation_buf(t, null, 0, &required), ); try testing.expectEqual(@as(usize, 0), required); + + // Completing the sequence must return an empty, allocation-free result. + const failing: CAllocator = .fromZig(&std.mem.Allocator.failing); + try testing.expectEqual(Result.success, continuation_alloc(t, &failing, &out_ptr, &out_len)); + defer @import("allocator.zig").free(&failing, out_ptr, out_len); + try testing.expectEqual(null, out_ptr); + try testing.expectEqual(@as(usize, 0), out_len); } test "continuation export tracks split UTF-8" { @@ -4366,6 +4360,20 @@ test "title_changed without callback is silent" { vt_write(t, "\x1B]2;Hello\x1B\\", 10); } +test "kitty_image_medium_temp_file empty output" { + if (comptime !build_options.kitty_graphics) return error.SkipZigTest; + var t: Terminal = null; + try testing.expectEqual(Result.success, new(&lib.alloc.test_allocator, &t, 80, 24)); + defer free(t); + + const empty: lib.String = .{ .ptr = "", .len = 0 }; + try testing.expectEqual(Result.success, set(t, .kitty_image_medium_temp_file, &empty)); + var result: lib.String = undefined; + try testing.expectEqual(Result.success, get(t, .kitty_image_medium_temp_file, &result)); + try testing.expectEqual(@as(usize, 0), result.len); + try testing.expectEqual(@as([*]const u8, ""), result.ptr); +} + test "set desktop_notification callback" { var t: Terminal = null; try testing.expectEqual(Result.success, new(