From b5398180d6b362346522a6067d54b90b97e23dc2 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 9 Aug 2024 19:49:48 -0700 Subject: std.debug.Coverage.resolveAddressesDwarf: fix broken logic The implementation assumed that compilation units did not overlap, which is not the case. The new implementation uses .debug_ranges to iterate over the requested PCs. This partially resolves #20990. The dump-cov tool is fixed but the same fix needs to be applied to `std.Build.Fuzz.WebServer` (sorting the PC list before passing it to be resolved by debug info). I am observing LLVM emit multiple 8-bit counters for the same PC addresses when enabling `-fsanitize-coverage=inline-8bit-counters`. This seems like a bug in LLVM. I can't fathom why that would be desireable. --- lib/std/debug/Coverage.zig | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) (limited to 'lib/std/debug/Coverage.zig') diff --git a/lib/std/debug/Coverage.zig b/lib/std/debug/Coverage.zig index f341efaffb..bb6f7aabcb 100644 --- a/lib/std/debug/Coverage.zig +++ b/lib/std/debug/Coverage.zig @@ -151,46 +151,35 @@ pub fn resolveAddressesDwarf( d: *Dwarf, ) ResolveAddressesDwarfError!void { assert(sorted_pc_addrs.len == output.len); - assert(d.compile_units_sorted); + assert(d.ranges.items.len != 0); // call `populateRanges` first. - var cu_i: usize = 0; - var line_table_i: usize = 0; - var cu: *Dwarf.CompileUnit = &d.compile_unit_list.items[0]; - var range = cu.pc_range.?; + var range_i: usize = 0; + var range: *std.debug.Dwarf.Range = &d.ranges.items[0]; + var line_table_i: usize = undefined; + var prev_cu: ?*std.debug.Dwarf.CompileUnit = null; // Protects directories and files tables from other threads. cov.mutex.lock(); defer cov.mutex.unlock(); next_pc: for (sorted_pc_addrs, output) |pc, *out| { while (pc >= range.end) { - cu_i += 1; - if (cu_i >= d.compile_unit_list.items.len) { + range_i += 1; + if (range_i >= d.ranges.items.len) { out.* = SourceLocation.invalid; continue :next_pc; } - cu = &d.compile_unit_list.items[cu_i]; - line_table_i = 0; - range = cu.pc_range orelse { - out.* = SourceLocation.invalid; - continue :next_pc; - }; + range = &d.ranges.items[range_i]; } if (pc < range.start) { out.* = SourceLocation.invalid; continue :next_pc; } - if (line_table_i == 0) { - line_table_i = 1; + const cu = &d.compile_unit_list.items[range.compile_unit_index]; + if (cu.src_loc_cache == null) { cov.mutex.unlock(); defer cov.mutex.lock(); d.populateSrcLocCache(gpa, cu) catch |err| switch (err) { error.MissingDebugInfo, error.InvalidDebugInfo => { out.* = SourceLocation.invalid; - cu_i += 1; - if (cu_i < d.compile_unit_list.items.len) { - cu = &d.compile_unit_list.items[cu_i]; - line_table_i = 0; - if (cu.pc_range) |r| range = r; - } continue :next_pc; }, else => |e| return e, @@ -198,6 +187,14 @@ pub fn resolveAddressesDwarf( } const slc = &cu.src_loc_cache.?; const table_addrs = slc.line_table.keys(); + if (cu != prev_cu) { + prev_cu = cu; + line_table_i = std.sort.upperBound(u64, table_addrs, pc, struct { + fn order(context: u64, item: u64) std.math.Order { + return std.math.order(item, context); + } + }.order); + } while (line_table_i < table_addrs.len and table_addrs[line_table_i] < pc) line_table_i += 1; const entry = slc.line_table.values()[line_table_i - 1]; -- cgit v1.2.3 From a9e7fb0e0189e01ebf67b144aca3fd8c318925c3 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 11 Aug 2024 15:08:43 -0700 Subject: avoid a branch in resolveAddressesDwarf --- lib/std/Build/Fuzz/WebServer.zig | 4 ++-- lib/std/debug/Coverage.zig | 30 ++++++++++++++++-------------- 2 files changed, 18 insertions(+), 16 deletions(-) (limited to 'lib/std/debug/Coverage.zig') diff --git a/lib/std/Build/Fuzz/WebServer.zig b/lib/std/Build/Fuzz/WebServer.zig index 17bc2c918f..376322c3ec 100644 --- a/lib/std/Build/Fuzz/WebServer.zig +++ b/lib/std/Build/Fuzz/WebServer.zig @@ -664,8 +664,8 @@ fn addEntryPoint(ws: *WebServer, coverage_id: u64, addr: u64) error{ AlreadyRepo if (false) { const sl = coverage_map.source_locations[index]; const file_name = coverage_map.coverage.stringAt(coverage_map.coverage.fileAt(sl.file).basename); - log.debug("server found entry point {s}:{d}:{d}", .{ - file_name, sl.line, sl.column, + log.debug("server found entry point for 0x{x} at {s}:{d}:{d}", .{ + addr, file_name, sl.line, sl.column, }); } const gpa = ws.gpa; diff --git a/lib/std/debug/Coverage.zig b/lib/std/debug/Coverage.zig index bb6f7aabcb..3971d770f3 100644 --- a/lib/std/debug/Coverage.zig +++ b/lib/std/debug/Coverage.zig @@ -174,28 +174,30 @@ pub fn resolveAddressesDwarf( continue :next_pc; } const cu = &d.compile_unit_list.items[range.compile_unit_index]; - if (cu.src_loc_cache == null) { - cov.mutex.unlock(); - defer cov.mutex.lock(); - d.populateSrcLocCache(gpa, cu) catch |err| switch (err) { - error.MissingDebugInfo, error.InvalidDebugInfo => { - out.* = SourceLocation.invalid; - continue :next_pc; - }, - else => |e| return e, - }; - } - const slc = &cu.src_loc_cache.?; - const table_addrs = slc.line_table.keys(); if (cu != prev_cu) { prev_cu = cu; + if (cu.src_loc_cache == null) { + cov.mutex.unlock(); + defer cov.mutex.lock(); + d.populateSrcLocCache(gpa, cu) catch |err| switch (err) { + error.MissingDebugInfo, error.InvalidDebugInfo => { + out.* = SourceLocation.invalid; + continue :next_pc; + }, + else => |e| return e, + }; + } + const slc = &cu.src_loc_cache.?; + const table_addrs = slc.line_table.keys(); line_table_i = std.sort.upperBound(u64, table_addrs, pc, struct { fn order(context: u64, item: u64) std.math.Order { return std.math.order(item, context); } }.order); } - while (line_table_i < table_addrs.len and table_addrs[line_table_i] < pc) line_table_i += 1; + const slc = &cu.src_loc_cache.?; + const table_addrs = slc.line_table.keys(); + while (line_table_i < table_addrs.len and table_addrs[line_table_i] <= pc) line_table_i += 1; const entry = slc.line_table.values()[line_table_i - 1]; const corrected_file_index = entry.file - @intFromBool(slc.version < 5); -- cgit v1.2.3 From a726e09389aceff39f4478f215f4991a5f148e8d Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 13 Aug 2024 19:29:55 -0700 Subject: std.debug.Coverage.resolveAddressesDwarf: assert sorted --- lib/std/debug/Coverage.zig | 5 +++++ lib/std/debug/Info.zig | 1 + 2 files changed, 6 insertions(+) (limited to 'lib/std/debug/Coverage.zig') diff --git a/lib/std/debug/Coverage.zig b/lib/std/debug/Coverage.zig index 3971d770f3..2d0e054673 100644 --- a/lib/std/debug/Coverage.zig +++ b/lib/std/debug/Coverage.zig @@ -145,6 +145,7 @@ pub const ResolveAddressesDwarfError = Dwarf.ScanError; pub fn resolveAddressesDwarf( cov: *Coverage, gpa: Allocator, + /// Asserts the addresses are in ascending order. sorted_pc_addrs: []const u64, /// Asserts its length equals length of `sorted_pc_addrs`. output: []SourceLocation, @@ -156,11 +157,15 @@ pub fn resolveAddressesDwarf( var range_i: usize = 0; var range: *std.debug.Dwarf.Range = &d.ranges.items[0]; var line_table_i: usize = undefined; + var prev_pc: u64 = 0; var prev_cu: ?*std.debug.Dwarf.CompileUnit = null; // Protects directories and files tables from other threads. cov.mutex.lock(); defer cov.mutex.unlock(); next_pc: for (sorted_pc_addrs, output) |pc, *out| { + assert(pc >= prev_pc); + prev_pc = pc; + while (pc >= range.end) { range_i += 1; if (range_i >= d.ranges.items.len) { diff --git a/lib/std/debug/Info.zig b/lib/std/debug/Info.zig index 512d24bfc5..0a07d9ba15 100644 --- a/lib/std/debug/Info.zig +++ b/lib/std/debug/Info.zig @@ -51,6 +51,7 @@ pub const ResolveAddressesError = Coverage.ResolveAddressesDwarfError; pub fn resolveAddresses( info: *Info, gpa: Allocator, + /// Asserts the addresses are in ascending order. sorted_pc_addrs: []const u64, /// Asserts its length equals length of `sorted_pc_addrs`. output: []SourceLocation, -- cgit v1.2.3