From fc3406a96169ee72c33ba6a0c965ec62fbd44539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sat, 23 May 2026 22:48:11 +0200 Subject: [PATCH] std.debug.Pdb: deduplicate inline source locations Previously, if the same inline function was called multiple times by the same caller, the inline function's frame would be repeated multiple times, once for each prior call. --- lib/std/debug/Pdb.zig | 101 ++++++++++++++++++----------- lib/std/debug/SelfInfo/Windows.zig | 8 +-- test/error_traces.zig | 31 +++++---- test/stack_traces.zig | 23 ++++--- 4 files changed, 97 insertions(+), 66 deletions(-) diff --git a/lib/std/debug/Pdb.zig b/lib/std/debug/Pdb.zig index b242eb3696e2e2fb3c96fb2d5d71e5ce747e1fc5..5cd24287972d79904986d376de15a89ca23530b0 100644 --- a/lib/std/debug/Pdb.zig +++ b/lib/std/debug/Pdb.zig @@ -26,10 +26,11 @@ pub const Module = struct { symbols: []u8, subsect_info: []u8, checksum_offset: ?usize, - /// The inlinee source lines, sorted by inlinee. This saves us from repeatedly doing linear - /// searches over all inlinees. We prefer binary search over a hashmap as LLVM somtimes outputs - /// multiple entries for a single inlinee ID, see `getInlineeSourceLines` for more info. - inlinee_source_lines: []InlineeSourceLine, + /// The inlinee source lines, sorted by inlinee, then file, then line number. + /// This saves us from repeatedly doing linear searches over all inlinees. + /// We prefer binary search over a hashmap as LLVM somtimes outputs multiple entries + /// for a single inlinee ID, see `getInlineeSourceLines` for more info. + inlinee_source_lines: []*align(1) const pdb.InlineeSourceLine, pub fn deinit(self: *Module, allocator: Allocator) void { allocator.free(self.module_name); @@ -669,44 +670,68 @@ pub fn getSymbolName(self: *Pdb, proc_sym: *align(1) const pdb.ProcSym) []const return std.mem.sliceTo(@as([*:0]const u8, @ptrCast(&proc_sym.name[0])), 0); } -pub const InlineeSourceLine = struct { - signature: pdb.InlineeSourceLineSignature, - info: *align(1) const pdb.InlineeSourceLine, +fn inlineeSourceLineLessThan( + _: void, + lhs: *align(1) const pdb.InlineeSourceLine, + rhs: *align(1) const pdb.InlineeSourceLine, +) bool { + if (lhs.inlinee < rhs.inlinee) return true; + if (lhs.inlinee > rhs.inlinee) return false; + if (lhs.file_id < rhs.file_id) return true; + if (lhs.file_id > rhs.file_id) return false; + return lhs.source_line_num < rhs.source_line_num; +} - fn lessThan(_: void, lhs: InlineeSourceLine, rhs: InlineeSourceLine) bool { - return lhs.info.inlinee < rhs.info.inlinee; - } - - fn compare(inlinee: u32, self: InlineeSourceLine) std.math.Order { - return std.math.order(inlinee, self.info.inlinee); - } -}; - -/// Returns all `InlineeSourceLine`s for a given module with the given inlinee. Ideally there would -/// only be one entry per inlinee, but LLVM appears to assign all functions that share a name the -/// same inlinee ID. This appears to be a bug, so the best the caller can do right now is print all -/// the results. -pub fn getInlineeSourceLines( - self: *Pdb, - mod: *Module, +fn compareInlineeSourceLineInlinee( inlinee: u32, -) []const InlineeSourceLine { + inlinee_src_line: *align(1) const pdb.InlineeSourceLine, +) std.math.Order { + return std.math.order(inlinee, inlinee_src_line.inlinee); +} + +pub const InlineeSourceLocationIterator = struct { + /// The iterator assumes that all source lines in the slice are associated + /// with the same inlinee, and that it is sorted by file, then line number. + lines: []*align(1) const pdb.InlineeSourceLine, + + pub const empty: InlineeSourceLocationIterator = .{ .lines = &.{} }; + + pub fn next(iter: *InlineeSourceLocationIterator) ?*align(1) const pdb.InlineeSourceLine { + if (iter.lines.len == 0) return null; + const line = iter.lines[0]; + iter.lines = iter.lines[1..]; + // Filter out duplicate entries + while (iter.lines.len != 0 and + iter.lines[0].file_id == line.file_id and + iter.lines[0].source_line_num == line.source_line_num) + { + iter.lines = iter.lines[1..]; + } + return line; + } +}; + +/// Returns all `pdb.InlineeSourceLine`s for a given module with the given inlinee. Ideally +/// there would only be one entry per inlinee, but LLVM appears to assign all functions that share +/// a name the same inlinee ID. This is a bug: https://github.com/llvm/llvm-project/issues/191787 +/// The best the caller can do right now is print all the results. +pub fn getInlineeSourceLines(self: *Pdb, mod: *Module, inlinee: u32) InlineeSourceLocationIterator { _ = self; // Binary search to an arbitrary match, if there are other matches they will be adjacent const any = std.sort.binarySearch( - InlineeSourceLine, + *align(1) const pdb.InlineeSourceLine, mod.inlinee_source_lines, inlinee, - InlineeSourceLine.compare, - ) orelse return &.{}; + compareInlineeSourceLineInlinee, + ) orelse return .empty; // Linearly scan to the first match const begin = b: { var begin = any; while (begin > 0) { const prev = begin - 1; - if (mod.inlinee_source_lines[prev].info.inlinee != inlinee) break; + if (mod.inlinee_source_lines[prev].inlinee != inlinee) break; begin = prev; } break :b begin; @@ -716,13 +741,13 @@ pub fn getInlineeSourceLines( const end = b: { var end = any + 1; while (end < mod.inlinee_source_lines.len and - mod.inlinee_source_lines[end].info.inlinee == inlinee) : (end += 1) + mod.inlinee_source_lines[end].inlinee == inlinee) : (end += 1) {} break :b end; }; - // Return a slice of all the matches - return mod.inlinee_source_lines[begin..end]; + // Return an iterator over all matches (the iterator filters out duplicate entries) + return .{ .lines = mod.inlinee_source_lines[begin..end] }; } pub fn getLineNumberInfo(self: *Pdb, gpa: Allocator, module: *Module, address: u64) !std.debug.SourceLocation { @@ -845,7 +870,7 @@ pub fn getModule(self: *Pdb, index: usize) !?*Module { mod.subsect_info = try reader.readAlloc(gpa, mod.mod_info.c13_byte_size); errdefer gpa.free(mod.subsect_info); mod.inlinee_source_lines = b: { - var inlinee_source_lines: std.ArrayList(InlineeSourceLine) = .empty; + var inlinee_source_lines: std.ArrayList(*align(1) const pdb.InlineeSourceLine) = .empty; defer inlinee_source_lines.deinit(gpa); var subsects: Io.Reader = .fixed(mod.subsect_info); while (subsects.takeStructPointer(pdb.DebugSubsectionHeader) catch null) |subsect_hdr| { @@ -866,15 +891,17 @@ pub fn getModule(self: *Pdb, index: usize) !?*Module { return error.InvalidDebugInfo; } - try inlinee_source_lines.append(gpa, .{ - .signature = inlinee_source_line_signature, - .info = info, - }); + try inlinee_source_lines.append(gpa, info); } } } - std.mem.sortUnstable(InlineeSourceLine, inlinee_source_lines.items, {}, InlineeSourceLine.lessThan); + std.mem.sortUnstable( + *align(1) const pdb.InlineeSourceLine, + inlinee_source_lines.items, + {}, + inlineeSourceLineLessThan, + ); break :b try inlinee_source_lines.toOwnedSlice(gpa); }; errdefer gpa.free(mod.inlinee_source_lines); diff --git a/lib/std/debug/SelfInfo/Windows.zig b/lib/std/debug/SelfInfo/Windows.zig index 50684e9b2a8d6288dcfba85a170420784756d935..5bc770003a3591481993e45ed003b583ba14af31 100644 --- a/lib/std/debug/SelfInfo/Windows.zig +++ b/lib/std/debug/SelfInfo/Windows.zig @@ -311,15 +311,13 @@ const Module = struct { // If our address points into this site, get the source location(s) it // points at - for (pdb.getInlineeSourceLines( - module, - inline_site.inlinee, - )) |inlinee_src_line| { + var line_iter = pdb.getInlineeSourceLines(module, inline_site.inlinee); + while (line_iter.next()) |inlinee_src_line| { const maybe_loc = pdb.getInlineSiteSourceLocation( text_arena, module, inline_site, - inlinee_src_line.info, + inlinee_src_line, offset_in_func, ) catch continue; const loc = maybe_loc orelse continue; diff --git a/test/error_traces.zig b/test/error_traces.zig index 3e526f1224610eb6c01e34c838b938b84708577b..071e4e8df53b7f017f988f6759db5adc30cebef0 100644 --- a/test/error_traces.zig +++ b/test/error_traces.zig @@ -580,12 +580,15 @@ pub fn addCases(cases: *@import("tests.zig").ErrorTracesContext, os: std.Target. cases.addCase(.{ .name = "trace through inline call", + // The main function has two inline calls to ensure + // that inlinees in PDBs are properly deduplicated. .source = \\pub fn main() !void { - \\ try foo(); + \\ try foo(false); + \\ try foo(true); \\} - \\inline fn foo() !void { - \\ try bar(); + \\inline fn foo(b: bool) !void { + \\ if (b) try bar(); \\} \\fn bar() !void { \\ return error.ThisIsSoSad; @@ -597,25 +600,25 @@ pub fn addCases(cases: *@import("tests.zig").ErrorTracesContext, os: std.Target. // so our expected result is slightly different for Windows than on other operating // systems. .windows => - \\source.zig:8:5: [address] in bar + \\source.zig:9:5: [address] in bar \\ return error.ThisIsSoSad; \\ ^ - \\source.zig:5: [address] in foo - \\ try bar(); + \\source.zig:6: [address] in foo + \\ if (b) try bar(); \\ - \\source.zig:2:5: [address] in main - \\ try foo(); + \\source.zig:3:5: [address] in main + \\ try foo(true); \\ ^ , else => - \\source.zig:8:5: [address] in bar + \\source.zig:9:5: [address] in bar \\ return error.ThisIsSoSad; \\ ^ - \\source.zig:5:5: [address] in foo - \\ try bar(); - \\ ^ - \\source.zig:2:5: [address] in main - \\ try foo(); + \\source.zig:6:12: [address] in foo + \\ if (b) try bar(); + \\ ^ + \\source.zig:3:5: [address] in main + \\ try foo(true); \\ ^ , }, diff --git a/test/stack_traces.zig b/test/stack_traces.zig index 037399d36c7094f80949c49697475292ada364bf..82d7d67863c209f4a5e3825f0eeb2cb100e50ced 100644 --- a/test/stack_traces.zig +++ b/test/stack_traces.zig @@ -226,12 +226,15 @@ pub fn addCases(cases: *@import("tests.zig").StackTracesContext, os: std.Target. cases.addCase(.{ .name = "simple inline panic", + // The main function has two inline calls to ensure + // that inlinees in PDBs are properly deduplicated. .source = \\pub fn main() void { - \\ foo(); + \\ foo(false); + \\ foo(true); \\} - \\inline fn foo() void { - \\ @panic("oh no"); + \\inline fn foo(b: bool) void { + \\ if (b) @panic("oh no"); \\} \\ , @@ -242,11 +245,11 @@ pub fn addCases(cases: *@import("tests.zig").StackTracesContext, os: std.Target. // so the first location has only a row. .windows => \\panic: oh no - \\source.zig:5: [address] in foo - \\ @panic("oh no"); + \\source.zig:6: [address] in foo + \\ if (b) @panic("oh no"); \\ - \\source.zig:2:8: [address] in main - \\ foo(); + \\source.zig:3:8: [address] in main + \\ foo(true); \\ ^ \\ , @@ -254,9 +257,9 @@ pub fn addCases(cases: *@import("tests.zig").StackTracesContext, os: std.Target. // resolve the inline callers. else => \\panic: oh no - \\source.zig:5:5: [address] in foo - \\ @panic("oh no"); - \\ ^ + \\source.zig:6:12: [address] in foo + \\ if (b) @panic("oh no"); + \\ ^ , }, .expect_strip = switch (os) { -- 2.54.0