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) {