fix up recent Valgrind errors

- ci: valgrind runs now fail when Memcheck finds an unsuppressed error.
- fix real memory issues in unit tests
- add a suppression for Zig's flate compression which documents as
  purposely using undefined memory
This commit is contained in:
Mitchell Hashimoto
2026-08-20 13:54:51 -07:00
parent 1ffa77c90b
commit c5f4f00ed3
6 changed files with 54 additions and 9 deletions
+2
View File
@@ -326,6 +326,7 @@ pub fn build(b: *std.Build) !void {
const run_cmd = b.addSystemCommand(&.{
"valgrind",
"--leak-check=full",
"--error-exitcode=1",
"--num-callers=50",
b.fmt("--suppressions={s}", .{b.pathFromRoot("valgrind.supp")}),
"--gen-suppressions=all",
@@ -392,6 +393,7 @@ pub fn build(b: *std.Build) !void {
const valgrind_run = b.addSystemCommand(&.{
"valgrind",
"--leak-check=full",
"--error-exitcode=1",
"--num-callers=50",
b.fmt("--suppressions={s}", .{b.pathFromRoot("valgrind.supp")}),
"--gen-suppressions=all",
+1 -1
View File
@@ -41,7 +41,7 @@ pub const FileFormatter = struct {
if (field.name[0] == '_') continue;
const value = @field(self.config, field.name);
const do_format = if (default) |d| format: {
const do_format = if (default) |*d| format: {
const key = @field(Key, field.name);
break :format d.changed(self.config, key);
} else true;
+8
View File
@@ -7794,6 +7794,14 @@ test "Screen: resize errors preserve state" {
try testing.expectEqual(before.pages.viewport, s.pages.viewport);
try testing.expectEqual(before_viewport_pin, s.pages.viewport_pin.*);
try testing.expectEqual(before_tracked_pins, s.pages.countTrackedPins());
if (std.valgrind.runningOnValgrind() > 0) {
// This assertion deliberately compares the complete raw page,
// including semantically irrelevant struct padding.
std.valgrind.memcheck.makeMemDefined(before_page);
std.valgrind.memcheck.makeMemDefined(
s.pages.pages.first.?.page().memory,
);
}
try testing.expectEqualSlices(
u8,
before_page,
+14
View File
@@ -106,6 +106,15 @@ pub fn init(
if (scratch.len < required) return error.OutputTooSmall;
if (required == 0) return null;
// Page memory is an opaque snapshot containing structs whose padding has
// no semantic value. Assigning those structs can leave their padding
// undefined in Memcheck even though every byte is addressable and must be
// preserved by compression. Mark the complete byte representation as
// defined at this boundary so that undefined padding does not propagate
// through the codec and obscure real Valgrind failures.
if (std.valgrind.runningOnValgrind() > 0)
std.valgrind.memcheck.makeMemDefined(source.memory);
const encoded_len = lz4.compress(
source.memory,
scratch[0..required],
@@ -209,6 +218,11 @@ test "compressed Page retained mapping round trip" {
const expected = try testing.allocator.dupe(u8, resident.memory);
defer testing.allocator.free(expected);
if (std.valgrind.runningOnValgrind() > 0) {
// This snapshot deliberately includes semantically irrelevant struct
// padding so the round trip can compare the complete representation.
std.valgrind.memcheck.makeMemDefined(expected);
}
const memory_ptr = resident.memory.ptr;
const memory_len = resident.memory.len;
+20 -8
View File
@@ -2811,9 +2811,12 @@ test "kitty_keyboard_query" {
defer t.deinit(testing.allocator);
const S = struct {
var written: ?[:0]const u8 = null;
var written: ?[]const u8 = null;
var written_buf: [64]u8 = undefined;
fn writePty(_: *Handler, data: [:0]const u8) void {
written = data;
std.debug.assert(data.len <= written_buf.len);
@memcpy(written_buf[0..data.len], data);
written = written_buf[0..data.len];
}
};
S.written = null;
@@ -2840,9 +2843,12 @@ test "xtversion default" {
defer t.deinit(testing.allocator);
const S = struct {
var written: ?[:0]const u8 = null;
var written: ?[]const u8 = null;
var written_buf: [64]u8 = undefined;
fn writePty(_: *Handler, data: [:0]const u8) void {
written = data;
std.debug.assert(data.len <= written_buf.len);
@memcpy(written_buf[0..data.len], data);
written = written_buf[0..data.len];
}
};
S.written = null;
@@ -2863,9 +2869,12 @@ test "xtversion with effect" {
defer t.deinit(testing.allocator);
const S = struct {
var written: ?[:0]const u8 = null;
var written: ?[]const u8 = null;
var written_buf: [64]u8 = undefined;
fn writePty(_: *Handler, data: [:0]const u8) void {
written = data;
std.debug.assert(data.len <= written_buf.len);
@memcpy(written_buf[0..data.len], data);
written = written_buf[0..data.len];
}
fn xtversion(_: *Handler) []const u8 {
return "ghostty 1.2.3";
@@ -2889,9 +2898,12 @@ test "xtversion with empty string effect" {
defer t.deinit(testing.allocator);
const S = struct {
var written: ?[:0]const u8 = null;
var written: ?[]const u8 = null;
var written_buf: [64]u8 = undefined;
fn writePty(_: *Handler, data: [:0]const u8) void {
written = data;
std.debug.assert(data.len <= written_buf.len);
@memcpy(written_buf[0..data.len], data);
written = written_buf[0..data.len];
}
fn xtversion(_: *Handler) []const u8 {
return "";
+9
View File
@@ -13,6 +13,15 @@
# You must gracefully exit Ghostty (do not SIGINT) by closing all windows
# and quitting. Otherwise, we leave a number of GTK resources around.
# Zig's DEFLATE encoder intentionally increments an undefined base[0] entry
# for unused Huffman symbols. The corresponding output code is also unused;
# see the comment in std.compress.flate.Compress.huffman.buildValues.
{
Zig std DEFLATE unused Huffman code
Memcheck:Cond
fun:compress.flate.Compress.huffman.buildValues
}
# Reproduction:
# 1. Launch Ghostty (no config)