From 7774287fdca1b56e12a420c446e83191c05fb468 Mon Sep 17 00:00:00 2001 From: mlugg Date: Thu, 23 Jan 2025 04:48:01 +0000 Subject: [PATCH 1/5] tests: enable stack trace tests for x86_64-selfhosted Allows the stack trace tests to be additionally compiled and run with `.use_llvm = false, .use_lld = false` depending on the host target. This is currently enabled for x86_64 targets emitting ELF. Self-hosted backends emit slightly different DWARF info to the LLVM backend, so the checking logic (and the tests themselves) had to be tweaked slightly to support both backends at once. --- test/src/StackTrace.zig | 31 ++++++++++++++---- test/src/check-stack-trace.zig | 21 ++++++++---- test/stack_traces.zig | 60 +++++++++++++++++----------------- 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/test/src/StackTrace.zig b/test/src/StackTrace.zig index 37e390c78d..5151447a43 100644 --- a/test/src/StackTrace.zig +++ b/test/src/StackTrace.zig @@ -21,17 +21,34 @@ const Config = struct { }; pub fn addCase(self: *StackTrace, config: Config) void { + self.addCaseInner(config, true); + if (shouldTestNonLlvm(self.b.graph.host.result)) { + self.addCaseInner(config, false); + } +} + +fn addCaseInner(self: *StackTrace, config: Config, use_llvm: bool) void { if (config.Debug) |per_mode| - self.addExpect(config.name, config.source, .Debug, per_mode); + self.addExpect(config.name, config.source, .Debug, use_llvm, per_mode); if (config.ReleaseSmall) |per_mode| - self.addExpect(config.name, config.source, .ReleaseSmall, per_mode); + self.addExpect(config.name, config.source, .ReleaseSmall, use_llvm, per_mode); if (config.ReleaseFast) |per_mode| - self.addExpect(config.name, config.source, .ReleaseFast, per_mode); + self.addExpect(config.name, config.source, .ReleaseFast, use_llvm, per_mode); if (config.ReleaseSafe) |per_mode| - self.addExpect(config.name, config.source, .ReleaseSafe, per_mode); + self.addExpect(config.name, config.source, .ReleaseSafe, use_llvm, per_mode); +} + +fn shouldTestNonLlvm(target: std.Target) bool { + return switch (target.cpu.arch) { + .x86_64 => switch (target.ofmt) { + .elf => true, + else => false, + }, + else => false, + }; } fn addExpect( @@ -39,13 +56,14 @@ fn addExpect( name: []const u8, source: []const u8, optimize_mode: OptimizeMode, + use_llvm: bool, mode_config: Config.PerMode, ) void { for (mode_config.exclude_os) |tag| if (tag == builtin.os.tag) return; const b = self.b; - const annotated_case_name = b.fmt("check {s} ({s})", .{ - name, @tagName(optimize_mode), + const annotated_case_name = b.fmt("check {s} ({s} {s})", .{ + name, @tagName(optimize_mode), if (use_llvm) "llvm" else "selfhosted", }); for (self.test_filters) |test_filter| { if (mem.indexOf(u8, annotated_case_name, test_filter)) |_| break; @@ -61,6 +79,7 @@ fn addExpect( .target = b.graph.host, .error_tracing = mode_config.error_tracing, }), + .use_llvm = use_llvm, }); const run = b.addRunArtifact(exe); diff --git a/test/src/check-stack-trace.zig b/test/src/check-stack-trace.zig index 9856b5738e..fccbbe3609 100644 --- a/test/src/check-stack-trace.zig +++ b/test/src/check-stack-trace.zig @@ -58,14 +58,23 @@ pub fn main() !void { try buf.appendSlice(line[pos + 1 .. marks[2] + delims[2].len]); try buf.appendSlice(" [address]"); if (optimize_mode == .Debug) { - // On certain platforms (windows) or possibly depending on how we choose to link main - // the object file extension may be present so we simply strip any extension. - if (mem.indexOfScalar(u8, line[marks[4]..marks[5]], '.')) |idot| { - try buf.appendSlice(line[marks[3] .. marks[4] + idot]); - try buf.appendSlice(line[marks[5]..]); + try buf.appendSlice(line[marks[3] .. marks[4] + delims[4].len]); + + const file_name = line[marks[4] + delims[4].len .. marks[5]]; + // The LLVM backend currently uses the object file name in the debug info here. + // This actually violates the DWARF specification (DWARF5 ยง 3.1.1, lines 24-27). + // The self-hosted backend uses the root Zig source file of the module (in compilance with the spec). + if (std.mem.eql(u8, file_name, "test") or + std.mem.eql(u8, file_name, "test.exe.obj") or + std.mem.endsWith(u8, file_name, ".zig")) + { + try buf.appendSlice("[main_file]"); } else { - try buf.appendSlice(line[marks[3]..]); + // Something unexpected; include it verbatim. + try buf.appendSlice(file_name); } + + try buf.appendSlice(line[marks[5]..]); } else { try buf.appendSlice(line[marks[3] .. marks[3] + delims[3].len]); try buf.appendSlice("[function]"); diff --git a/test/stack_traces.zig b/test/stack_traces.zig index 5126e3a538..85b682ab87 100644 --- a/test/stack_traces.zig +++ b/test/stack_traces.zig @@ -13,7 +13,7 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: TheSkyIsFalling - \\source.zig:2:5: [address] in main (test) + \\source.zig:2:5: [address] in main ([main_file]) \\ return error.TheSkyIsFalling; \\ ^ \\ @@ -61,10 +61,10 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: TheSkyIsFalling - \\source.zig:2:5: [address] in foo (test) + \\source.zig:2:5: [address] in foo ([main_file]) \\ return error.TheSkyIsFalling; \\ ^ - \\source.zig:6:5: [address] in main (test) + \\source.zig:6:5: [address] in main ([main_file]) \\ try foo(); \\ ^ \\ @@ -120,7 +120,7 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: UnrelatedError - \\source.zig:13:5: [address] in main (test) + \\source.zig:13:5: [address] in main ([main_file]) \\ return error.UnrelatedError; \\ ^ \\ @@ -172,7 +172,7 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: UnrelatedError - \\source.zig:10:5: [address] in main (test) + \\source.zig:10:5: [address] in main ([main_file]) \\ return error.UnrelatedError; \\ ^ \\ @@ -224,10 +224,10 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: TheSkyIsFalling - \\source.zig:2:5: [address] in foo (test) + \\source.zig:2:5: [address] in foo ([main_file]) \\ return error.TheSkyIsFalling; \\ ^ - \\source.zig:10:5: [address] in main (test) + \\source.zig:10:5: [address] in main ([main_file]) \\ try foo(); \\ ^ \\ @@ -284,7 +284,7 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: BadTime - \\source.zig:12:5: [address] in main (test) + \\source.zig:12:5: [address] in main ([main_file]) \\ return error.BadTime; \\ ^ \\ @@ -332,10 +332,10 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: AndMyCarIsOutOfGas - \\source.zig:2:5: [address] in foo (test) + \\source.zig:2:5: [address] in foo ([main_file]) \\ return error.TheSkyIsFalling; \\ ^ - \\source.zig:6:5: [address] in main (test) + \\source.zig:6:5: [address] in main ([main_file]) \\ return foo() catch error.AndMyCarIsOutOfGas; \\ ^ \\ @@ -391,7 +391,7 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: SomethingUnrelatedWentWrong - \\source.zig:11:5: [address] in main (test) + \\source.zig:11:5: [address] in main ([main_file]) \\ return error.SomethingUnrelatedWentWrong; \\ ^ \\ @@ -456,13 +456,13 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: StillUnresolved - \\source.zig:1:18: [address] in foo (test) + \\source.zig:1:18: [address] in foo ([main_file]) \\fn foo() !void { return error.TheSkyIsFalling; } \\ ^ - \\source.zig:2:18: [address] in bar (test) + \\source.zig:2:18: [address] in bar ([main_file]) \\fn bar() !void { return error.InternalError; } \\ ^ - \\source.zig:23:5: [address] in main (test) + \\source.zig:23:5: [address] in main ([main_file]) \\ return error.StillUnresolved; \\ ^ \\ @@ -527,13 +527,13 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: TestExpectedError - \\source.zig:9:18: [address] in foo (test) + \\source.zig:9:18: [address] in foo ([main_file]) \\fn foo() !void { return error.Foo; } \\ ^ - \\source.zig:5:5: [address] in expectError (test) + \\source.zig:5:5: [address] in expectError ([main_file]) \\ return error.TestExpectedError; \\ ^ - \\source.zig:17:5: [address] in main (test) + \\source.zig:17:5: [address] in main ([main_file]) \\ try expectError(error.Bar, foo()); \\ ^ \\ @@ -592,13 +592,13 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: AndMyCarIsOutOfGas - \\source.zig:2:5: [address] in foo (test) + \\source.zig:2:5: [address] in foo ([main_file]) \\ return error.TheSkyIsFalling; \\ ^ - \\source.zig:6:5: [address] in bar (test) + \\source.zig:6:5: [address] in bar ([main_file]) \\ return error.AndMyCarIsOutOfGas; \\ ^ - \\source.zig:11:9: [address] in main (test) + \\source.zig:11:9: [address] in main ([main_file]) \\ try bar(); \\ ^ \\ @@ -657,13 +657,13 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: AndMyCarIsOutOfGas - \\source.zig:2:5: [address] in foo (test) + \\source.zig:2:5: [address] in foo ([main_file]) \\ return error.TheSkyIsFalling; \\ ^ - \\source.zig:6:5: [address] in bar (test) + \\source.zig:6:5: [address] in bar ([main_file]) \\ return error.AndMyCarIsOutOfGas; \\ ^ - \\source.zig:11:9: [address] in main (test) + \\source.zig:11:9: [address] in main ([main_file]) \\ try bar(); \\ ^ \\ @@ -724,16 +724,16 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: TheSkyIsFalling - \\source.zig:10:5: [address] in make_error (test) + \\source.zig:10:5: [address] in make_error ([main_file]) \\ return error.TheSkyIsFalling; \\ ^ - \\source.zig:6:5: [address] in bar (test) + \\source.zig:6:5: [address] in bar ([main_file]) \\ return make_error(); \\ ^ - \\source.zig:2:5: [address] in foo (test) + \\source.zig:2:5: [address] in foo ([main_file]) \\ try bar(); \\ ^ - \\source.zig:14:5: [address] in main (test) + \\source.zig:14:5: [address] in main ([main_file]) \\ try foo(); \\ ^ \\ @@ -797,10 +797,10 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .windows, // TODO intermittent failures }, .expect = - \\source.zig:7:8: [address] in foo (test) + \\source.zig:7:8: [address] in foo ([main_file]) \\ bar(); \\ ^ - \\source.zig:10:8: [address] in main (test) + \\source.zig:10:8: [address] in main ([main_file]) \\ foo(); \\ ^ \\ @@ -829,7 +829,7 @@ pub fn addCases(cases: *tests.StackTracesContext) void { .Debug = .{ .expect = \\error: TheSkyIsFalling - \\source.zig:3:5: [address] in main (test) + \\source.zig:3:5: [address] in main ([main_file]) \\ return error.TheSkyIsFalling; \\ ^ \\ From b46a40ff1db6c4d467925a9a0d72436cdb3a6a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Wed, 22 Jan 2025 21:43:51 +0100 Subject: [PATCH 2/5] compiler: Handle --no-eh-frame-hdr as a regular zig build-* flag too. For some reason we accepted --eh-frame-hdr, but not --no-eh-frame-hdr, despite accepting the latter as a -Wl linker flag. --- src/main.zig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main.zig b/src/main.zig index 7d035ab135..cbc1c33319 100644 --- a/src/main.zig +++ b/src/main.zig @@ -575,6 +575,7 @@ const usage_build_generic = \\ 0x[hexstring] Maximum 32 bytes \\ none (default) Disable build-id \\ --eh-frame-hdr Enable C++ exception handling by passing --eh-frame-hdr to linker + \\ --no-eh-frame-hdr Disable C++ exception handling by passing --no-eh-frame-hdr to linker \\ --emit-relocs Enable output of relocation sections for post build tools \\ -z [arg] Set linker extension flags \\ nodelete Indicate that the object cannot be deleted from a process @@ -1582,6 +1583,8 @@ fn buildOutputType( fatal("unable to parse '{s}': {s}", .{ arg, @errorName(err) }); } else if (mem.eql(u8, arg, "--eh-frame-hdr")) { link_eh_frame_hdr = true; + } else if (mem.eql(u8, arg, "--no-eh-frame-hdr")) { + link_eh_frame_hdr = false; } else if (mem.eql(u8, arg, "--dynamicbase")) { linker_dynamicbase = true; } else if (mem.eql(u8, arg, "--no-dynamicbase")) { From ef4d7f01a5c26d8ed6fa8236d1b64e4091a51be3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Thu, 23 Jan 2025 01:38:20 +0100 Subject: [PATCH 3/5] compiler: Fix computation of Compilation.Config.any_unwind_tables. This moves the default value logic to Package.Module.create() instead and makes it so that Compilation.Config.any_unwind_tables is computed similarly to any_sanitize_thread, any_fuzz, etc. It turns out that for any_unwind_tables, we only actually care if unwind tables are enabled at all, not at what level. --- src/Compilation.zig | 9 ++++----- src/Compilation/Config.zig | 19 ++++++------------- src/Package/Module.zig | 16 ++++++++++++---- src/libcxx.zig | 12 ++++++------ src/libunwind.zig | 7 +++++-- src/main.zig | 14 ++------------ src/target.zig | 2 +- 7 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index ac8fb8a59b..81d95df9ef 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -1274,15 +1274,12 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil // The "any" values provided by resolved config only account for // explicitly-provided settings. We now make them additionally account // for default setting resolution. - const any_unwind_tables = switch (options.config.any_unwind_tables) { - .none => options.root_mod.unwind_tables, - .sync, .@"async" => |uwt| uwt, - }; + const any_unwind_tables = options.config.any_unwind_tables or options.root_mod.unwind_tables != .none; const any_non_single_threaded = options.config.any_non_single_threaded or !options.root_mod.single_threaded; const any_sanitize_thread = options.config.any_sanitize_thread or options.root_mod.sanitize_thread; const any_fuzz = options.config.any_fuzz or options.root_mod.fuzz; - const link_eh_frame_hdr = options.link_eh_frame_hdr or any_unwind_tables != .none; + const link_eh_frame_hdr = options.link_eh_frame_hdr or any_unwind_tables; const build_id = options.build_id orelse .none; const link_libc = options.config.link_libc; @@ -6459,6 +6456,7 @@ fn buildOutputFromZig( .root_optimize_mode = optimize_mode, .root_strip = strip, .link_libc = comp.config.link_libc, + .any_unwind_tables = comp.root_mod.unwind_tables != .none, }); const root_mod = try Package.Module.create(arena, .{ @@ -6595,6 +6593,7 @@ pub fn build_crt_file( .root_optimize_mode = comp.compilerRtOptMode(), .root_strip = comp.compilerRtStrip(), .link_libc = false, + .any_unwind_tables = options.unwind_tables != .none, .lto = switch (output_mode) { .Lib => comp.config.lto, .Obj, .Exe => .none, diff --git a/src/Compilation/Config.zig b/src/Compilation/Config.zig index e91055709c..c6ded8cfb2 100644 --- a/src/Compilation/Config.zig +++ b/src/Compilation/Config.zig @@ -12,14 +12,13 @@ link_libunwind: bool, /// True if and only if the c_source_files field will have nonzero length when /// calling Compilation.create. any_c_source_files: bool, -/// This is not `.none` if any `Module` has `unwind_tables` set explicitly to a +/// This is `true` if any `Module` has `unwind_tables` set explicitly to a /// value other than `.none`. Until `Compilation.create()` is called, it is -/// possible for this to be `.none` while in fact all `Module` instances have +/// possible for this to be `false` while in fact all `Module` instances have /// `unwind_tables != .none` due to the default. After `Compilation.create()` is /// called, this will also take into account the default setting, making this -/// value `.sync` or `.@"async"` if and only if any `Module` has -/// `unwind_tables != .none`. -any_unwind_tables: std.builtin.UnwindTables, +/// value `true` if and only if any `Module` has `unwind_tables != .none`. +any_unwind_tables: bool, /// This is true if any Module has single_threaded set explicitly to false. Until /// Compilation.create is called, it is possible for this to be false while in /// fact all Module instances have single_threaded=false due to the default @@ -88,7 +87,7 @@ pub const Options = struct { any_non_single_threaded: bool = false, any_sanitize_thread: bool = false, any_fuzz: bool = false, - any_unwind_tables: std.builtin.UnwindTables = .none, + any_unwind_tables: bool = false, any_dyn_libs: bool = false, any_c_source_files: bool = false, any_non_stripped: bool = false, @@ -359,12 +358,6 @@ pub fn resolve(options: Options) ResolveError!Config { break :b false; }; - const any_unwind_tables = b: { - if (options.any_unwind_tables != .none) break :b options.any_unwind_tables; - - break :b target_util.needUnwindTables(target, link_libunwind, options.any_sanitize_thread); - }; - const link_mode = b: { const explicitly_exe_or_dyn_lib = switch (options.output_mode) { .Obj => false, @@ -496,7 +489,7 @@ pub fn resolve(options: Options) ResolveError!Config { .link_libc = link_libc, .link_libcpp = link_libcpp, .link_libunwind = link_libunwind, - .any_unwind_tables = any_unwind_tables, + .any_unwind_tables = options.any_unwind_tables, .any_c_source_files = options.any_c_source_files, .any_non_single_threaded = options.any_non_single_threaded, .any_error_tracing = any_error_tracing, diff --git a/src/Package/Module.zig b/src/Package/Module.zig index 433a9b6ade..62b7f075dd 100644 --- a/src/Package/Module.zig +++ b/src/Package/Module.zig @@ -112,7 +112,7 @@ pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module { if (options.inherited.sanitize_thread == true) assert(options.global.any_sanitize_thread); if (options.inherited.fuzz == true) assert(options.global.any_fuzz); if (options.inherited.single_threaded == false) assert(options.global.any_non_single_threaded); - if (options.inherited.unwind_tables) |uwt| if (uwt != .none) assert(options.global.any_unwind_tables != .none); + if (options.inherited.unwind_tables) |uwt| if (uwt != .none) assert(options.global.any_unwind_tables); if (options.inherited.error_tracing == true) assert(options.global.any_error_tracing); const resolved_target = options.inherited.resolved_target orelse options.parent.?.resolved_target; @@ -121,9 +121,6 @@ pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module { const optimize_mode = options.inherited.optimize_mode orelse if (options.parent) |p| p.optimize_mode else .Debug; - const unwind_tables = options.inherited.unwind_tables orelse - if (options.parent) |p| p.unwind_tables else options.global.any_unwind_tables; - const strip = b: { if (options.inherited.strip) |x| break :b x; if (options.parent) |p| break :b p.strip; @@ -220,6 +217,17 @@ pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module { break :b false; }; + const unwind_tables = b: { + if (options.inherited.unwind_tables) |x| break :b x; + if (options.parent) |p| break :b p.unwind_tables; + + break :b target_util.defaultUnwindTables( + target, + options.global.link_libunwind, + sanitize_thread or options.global.any_sanitize_thread, + ); + }; + const fuzz = b: { if (options.inherited.fuzz) |x| break :b x; if (options.parent) |p| break :b p.fuzz; diff --git a/src/libcxx.zig b/src/libcxx.zig index 8b9ce044a4..06890531bc 100644 --- a/src/libcxx.zig +++ b/src/libcxx.zig @@ -397,6 +397,10 @@ pub fn buildLibCxxAbi(comp: *Compilation, prog_node: std.Progress.Node) BuildErr const optimize_mode = comp.compilerRtOptMode(); const strip = comp.compilerRtStrip(); + // See the `-fno-exceptions` logic for WASI. + // The old 32-bit x86 variant of SEH doesn't use tables. + const unwind_tables: std.builtin.UnwindTables = + if (target.os.tag == .wasi or (target.cpu.arch == .x86 and target.os.tag == .windows)) .none else .@"async"; const config = Compilation.Config.resolve(.{ .output_mode = output_mode, @@ -408,6 +412,7 @@ pub fn buildLibCxxAbi(comp: *Compilation, prog_node: std.Progress.Node) BuildErr .root_optimize_mode = optimize_mode, .root_strip = strip, .link_libc = true, + .any_unwind_tables = unwind_tables != .none, .lto = comp.config.lto, .any_sanitize_thread = comp.config.any_sanitize_thread, }) catch |err| { @@ -438,12 +443,7 @@ pub fn buildLibCxxAbi(comp: *Compilation, prog_node: std.Progress.Node) BuildErr .valgrind = false, .optimize_mode = optimize_mode, .structured_cfg = comp.root_mod.structured_cfg, - // See the `-fno-exceptions` logic for WASI. - // The old 32-bit x86 variant of SEH doesn't use tables. - .unwind_tables = if (target.os.tag == .wasi or (target.cpu.arch == .x86 and target.os.tag == .windows)) - .none - else - .@"async", + .unwind_tables = unwind_tables, .pic = comp.root_mod.pic, }, .global = config, diff --git a/src/libunwind.zig b/src/libunwind.zig index c52b579890..c7753b9587 100644 --- a/src/libunwind.zig +++ b/src/libunwind.zig @@ -27,6 +27,9 @@ pub fn buildStaticLib(comp: *Compilation, prog_node: std.Progress.Node) BuildErr const arena = arena_allocator.allocator(); const output_mode = .Lib; + const target = comp.root_mod.resolved_target.result; + const unwind_tables: std.builtin.UnwindTables = + if (target.cpu.arch == .x86 and target.os.tag == .windows) .none else .@"async"; const config = Compilation.Config.resolve(.{ .output_mode = .Lib, .resolved_target = comp.root_mod.resolved_target, @@ -36,6 +39,7 @@ pub fn buildStaticLib(comp: *Compilation, prog_node: std.Progress.Node) BuildErr .root_optimize_mode = comp.compilerRtOptMode(), .root_strip = comp.compilerRtStrip(), .link_libc = true, + .any_unwind_tables = unwind_tables != .none, .lto = comp.config.lto, }) catch |err| { comp.setMiscFailure( @@ -45,7 +49,6 @@ pub fn buildStaticLib(comp: *Compilation, prog_node: std.Progress.Node) BuildErr ); return error.SubCompilationFailed; }; - const target = comp.root_mod.resolved_target.result; const root_mod = Module.create(arena, .{ .global_cache_directory = comp.global_cache_directory, .paths = .{ @@ -65,7 +68,7 @@ pub fn buildStaticLib(comp: *Compilation, prog_node: std.Progress.Node) BuildErr .sanitize_thread = false, // necessary so that libunwind can unwind through its own stack frames // The old 32-bit x86 variant of SEH doesn't use tables. - .unwind_tables = if (target.cpu.arch == .x86 and target.os.tag == .windows) .none else .@"async", + .unwind_tables = unwind_tables, .pic = if (target_util.supports_fpic(target)) true else null, .optimize_mode = comp.compilerRtOptMode(), }, diff --git a/src/main.zig b/src/main.zig index cbc1c33319..ae7c2229aa 100644 --- a/src/main.zig +++ b/src/main.zig @@ -2849,12 +2849,7 @@ fn buildOutputType( create_module.opts.any_fuzz = true; if (mod_opts.unwind_tables) |uwt| switch (uwt) { .none => {}, - .sync => if (create_module.opts.any_unwind_tables == .none) { - create_module.opts.any_unwind_tables = .sync; - }, - .@"async" => { - create_module.opts.any_unwind_tables = .@"async"; - }, + .sync, .@"async" => create_module.opts.any_unwind_tables = true, }; if (mod_opts.strip == false) create_module.opts.any_non_stripped = true; @@ -7566,12 +7561,7 @@ fn handleModArg( create_module.opts.any_fuzz = true; if (mod_opts.unwind_tables) |uwt| switch (uwt) { .none => {}, - .sync => if (create_module.opts.any_unwind_tables == .none) { - create_module.opts.any_unwind_tables = .sync; - }, - .@"async" => { - create_module.opts.any_unwind_tables = .@"async"; - }, + .sync, .@"async" => create_module.opts.any_unwind_tables = true, }; if (mod_opts.strip == false) create_module.opts.any_non_stripped = true; diff --git a/src/target.zig b/src/target.zig index d3fb77e1b5..99e0fd1faa 100644 --- a/src/target.zig +++ b/src/target.zig @@ -407,7 +407,7 @@ pub fn clangSupportsNoImplicitFloatArg(target: std.Target) bool { }; } -pub fn needUnwindTables(target: std.Target, libunwind: bool, libtsan: bool) std.builtin.UnwindTables { +pub fn defaultUnwindTables(target: std.Target, libunwind: bool, libtsan: bool) std.builtin.UnwindTables { if (target.os.tag == .windows) { // The old 32-bit x86 variant of SEH doesn't use tables. return if (target.cpu.arch != .x86) .@"async" else .none; From 41185d297ffcaf61776aa8e7610aea9d00fce3a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Thu, 23 Jan 2025 01:43:57 +0100 Subject: [PATCH 4/5] Package.Module: Make create() fall back on options.global.root_optimize_mode. As is done for root_strip and root_error_tracing. --- src/Compilation/Config.zig | 2 ++ src/Package/Module.zig | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Compilation/Config.zig b/src/Compilation/Config.zig index c6ded8cfb2..41a8077203 100644 --- a/src/Compilation/Config.zig +++ b/src/Compilation/Config.zig @@ -56,6 +56,7 @@ export_memory: bool, shared_memory: bool, is_test: bool, debug_format: DebugFormat, +root_optimize_mode: std.builtin.OptimizeMode, root_strip: bool, root_error_tracing: bool, dll_export_fns: bool, @@ -508,6 +509,7 @@ pub fn resolve(options: Options) ResolveError!Config { .use_lld = use_lld, .wasi_exec_model = wasi_exec_model, .debug_format = debug_format, + .root_optimize_mode = root_optimize_mode, .root_strip = root_strip, .dll_export_fns = dll_export_fns, .rdynamic = rdynamic, diff --git a/src/Package/Module.zig b/src/Package/Module.zig index 62b7f075dd..5b3a487a49 100644 --- a/src/Package/Module.zig +++ b/src/Package/Module.zig @@ -119,7 +119,7 @@ pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module { const target = resolved_target.result; const optimize_mode = options.inherited.optimize_mode orelse - if (options.parent) |p| p.optimize_mode else .Debug; + if (options.parent) |p| p.optimize_mode else options.global.root_optimize_mode; const strip = b: { if (options.inherited.strip) |x| break :b x; From 180db2bf23f05a02876d4567cac3b04842c11acb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Thu, 23 Jan 2025 20:55:20 +0100 Subject: [PATCH 5/5] std.debug: Fall back to .eh_frame/.debug_frame if .eh_frame_hdr is incomplete. When using the self-hosted backends, especially in incremental mode, the .eh_frame_hdr section may be incomplete, so we can't treat it as authoritative. Instead, if we started out intending to use .eh_frame_hdr but find that it's incomplete, load .eh_frame/.debug_frame on demand and use that info going forward. --- lib/std/debug.zig | 12 ++++- lib/std/debug/Dwarf.zig | 18 +++++-- lib/std/debug/SelfInfo.zig | 97 +++++++++++++++++++++++++------------- 3 files changed, 88 insertions(+), 39 deletions(-) diff --git a/lib/std/debug.zig b/lib/std/debug.zig index 9f66599aa0..3624d6d3e7 100644 --- a/lib/std/debug.zig +++ b/lib/std/debug.zig @@ -732,11 +732,12 @@ pub const StackIterator = struct { // via DWARF before attempting to use the compact unwind info will produce incorrect results. if (module.unwind_info) |unwind_info| { if (SelfInfo.unwindFrameMachO( + unwind_state.debug_info.allocator, + module.base_address, &unwind_state.dwarf_context, &it.ma, unwind_info, module.eh_frame, - module.base_address, )) |return_address| { return return_address; } else |err| { @@ -748,7 +749,14 @@ pub const StackIterator = struct { } if (try module.getDwarfInfoForAddress(unwind_state.debug_info.allocator, unwind_state.dwarf_context.pc)) |di| { - return SelfInfo.unwindFrameDwarf(di, &unwind_state.dwarf_context, &it.ma, null); + return SelfInfo.unwindFrameDwarf( + unwind_state.debug_info.allocator, + di, + module.base_address, + &unwind_state.dwarf_context, + &it.ma, + null, + ); } else return error.MissingDebugInfo; } diff --git a/lib/std/debug/Dwarf.zig b/lib/std/debug/Dwarf.zig index 73b1871c46..3469deaae9 100644 --- a/lib/std/debug/Dwarf.zig +++ b/lib/std/debug/Dwarf.zig @@ -48,6 +48,8 @@ compile_unit_list: std.ArrayListUnmanaged(CompileUnit) = .empty, /// Filled later by the initializer func_list: std.ArrayListUnmanaged(Func) = .empty, +/// Starts out non-`null` if the `.eh_frame_hdr` section is present. May become `null` later if we +/// find that `.eh_frame_hdr` is incomplete. eh_frame_hdr: ?ExceptionFrameHeader = null, /// These lookup tables are only used if `eh_frame_hdr` is null cie_map: std.AutoArrayHashMapUnmanaged(u64, CommonInformationEntry) = .empty, @@ -1754,10 +1756,12 @@ fn readDebugAddr(di: Dwarf, compile_unit: CompileUnit, index: u64) !u64 { }; } -/// If .eh_frame_hdr is present, then only the header needs to be parsed. +/// If `.eh_frame_hdr` is present, then only the header needs to be parsed. Otherwise, `.eh_frame` +/// and `.debug_frame` are scanned and a sorted list of FDEs is built for binary searching during +/// unwinding. Even if `.eh_frame_hdr` is used, we may find during unwinding that it's incomplete, +/// in which case we build the sorted list of FDEs at that point. /// -/// Otherwise, .eh_frame and .debug_frame are scanned and a sorted list -/// of FDEs is built for binary searching during unwinding. +/// See also `scanCieFdeInfo`. pub fn scanAllUnwindInfo(di: *Dwarf, allocator: Allocator, base_address: usize) !void { if (di.section(.eh_frame_hdr)) |eh_frame_hdr| blk: { var fbr: FixedBufferReader = .{ .buf = eh_frame_hdr, .endian = native_endian }; @@ -1797,6 +1801,12 @@ pub fn scanAllUnwindInfo(di: *Dwarf, allocator: Allocator, base_address: usize) return; } + try di.scanCieFdeInfo(allocator, base_address); +} + +/// Scan `.eh_frame` and `.debug_frame` and build a sorted list of FDEs for binary searching during +/// unwinding. +pub fn scanCieFdeInfo(di: *Dwarf, allocator: Allocator, base_address: usize) !void { const frame_sections = [2]Section.Id{ .eh_frame, .debug_frame }; for (frame_sections) |frame_section| { if (di.section(frame_section)) |section_data| { @@ -2125,7 +2135,7 @@ pub const ElfModule = struct { return self.dwarf.getSymbol(allocator, relocated_address); } - pub fn getDwarfInfoForAddress(self: *@This(), allocator: Allocator, address: usize) !?*const Dwarf { + pub fn getDwarfInfoForAddress(self: *@This(), allocator: Allocator, address: usize) !?*Dwarf { _ = allocator; _ = address; return &self.dwarf; diff --git a/lib/std/debug/SelfInfo.zig b/lib/std/debug/SelfInfo.zig index 4dd0b4e842..a2cea70d37 100644 --- a/lib/std/debug/SelfInfo.zig +++ b/lib/std/debug/SelfInfo.zig @@ -707,7 +707,7 @@ pub const Module = switch (native_os) { } } - pub fn getDwarfInfoForAddress(self: *@This(), allocator: Allocator, address: usize) !?*const Dwarf { + pub fn getDwarfInfoForAddress(self: *@This(), allocator: Allocator, address: usize) !?*Dwarf { return if ((try self.getOFileInfoForAddress(allocator, address)).o_file_info) |o_file_info| &o_file_info.di else null; } }, @@ -784,7 +784,7 @@ pub const Module = switch (native_os) { return .{}; } - pub fn getDwarfInfoForAddress(self: *@This(), allocator: Allocator, address: usize) !?*const Dwarf { + pub fn getDwarfInfoForAddress(self: *@This(), allocator: Allocator, address: usize) !?*Dwarf { _ = allocator; _ = address; @@ -808,7 +808,7 @@ pub const Module = switch (native_os) { return .{}; } - pub fn getDwarfInfoForAddress(self: *@This(), allocator: Allocator, address: usize) !?*const Dwarf { + pub fn getDwarfInfoForAddress(self: *@This(), allocator: Allocator, address: usize) !?*Dwarf { _ = self; _ = allocator; _ = address; @@ -1156,11 +1156,12 @@ test machoSearchSymbols { /// If the compact encoding can't encode a way to unwind a frame, it will /// defer unwinding to DWARF, in which case `.eh_frame` will be used if available. pub fn unwindFrameMachO( + allocator: Allocator, + base_address: usize, context: *UnwindContext, ma: *std.debug.MemoryAccessor, unwind_info: []const u8, eh_frame: ?[]const u8, - module_base_address: usize, ) !usize { const header = std.mem.bytesAsValue( macho.unwind_info_section_header, @@ -1172,7 +1173,7 @@ pub fn unwindFrameMachO( ); if (indices.len == 0) return error.MissingUnwindInfo; - const mapped_pc = context.pc - module_base_address; + const mapped_pc = context.pc - base_address; const second_level_index = blk: { var left: usize = 0; var len: usize = indices.len; @@ -1351,7 +1352,7 @@ pub fn unwindFrameMachO( else stack_size: { // In .STACK_IND, the stack size is inferred from the subq instruction at the beginning of the function. const sub_offset_addr = - module_base_address + + base_address + entry.function_offset + encoding.value.x86_64.frameless.stack.indirect.sub_offset; if (ma.load(usize, sub_offset_addr) == null) return error.InvalidUnwindInfo; @@ -1416,7 +1417,7 @@ pub fn unwindFrameMachO( break :blk new_ip; }, .DWARF => { - return unwindFrameMachODwarf(context, ma, eh_frame orelse return error.MissingEhFrame, @intCast(encoding.value.x86_64.dwarf)); + return unwindFrameMachODwarf(allocator, base_address, context, ma, eh_frame orelse return error.MissingEhFrame, @intCast(encoding.value.x86_64.dwarf)); }, }, .aarch64, .aarch64_be => switch (encoding.mode.arm64) { @@ -1430,7 +1431,7 @@ pub fn unwindFrameMachO( break :blk new_ip; }, .DWARF => { - return unwindFrameMachODwarf(context, ma, eh_frame orelse return error.MissingEhFrame, @intCast(encoding.value.arm64.dwarf)); + return unwindFrameMachODwarf(allocator, base_address, context, ma, eh_frame orelse return error.MissingEhFrame, @intCast(encoding.value.arm64.dwarf)); }, .FRAME => blk: { const fp = (try regValueNative(context.thread_context, fpRegNum(reg_context), reg_context)).*; @@ -1555,13 +1556,16 @@ pub inline fn stripInstructionPtrAuthCode(ptr: usize) usize { /// Unwind a stack frame using DWARF unwinding info, updating the register context. /// -/// If `.eh_frame_hdr` is available, it will be used to binary search for the FDE. -/// Otherwise, a linear scan of `.eh_frame` and `.debug_frame` is done to find the FDE. +/// If `.eh_frame_hdr` is available and complete, it will be used to binary search for the FDE. +/// Otherwise, a linear scan of `.eh_frame` and `.debug_frame` is done to find the FDE. The latter +/// may require lazily loading the data in those sections. /// /// `explicit_fde_offset` is for cases where the FDE offset is known, such as when __unwind_info /// defers unwinding to DWARF. This is an offset into the `.eh_frame` section. pub fn unwindFrameDwarf( - di: *const Dwarf, + allocator: Allocator, + di: *Dwarf, + base_address: usize, context: *UnwindContext, ma: *std.debug.MemoryAccessor, explicit_fde_offset: ?usize, @@ -1570,10 +1574,7 @@ pub fn unwindFrameDwarf( if (context.pc == 0) return 0; // Find the FDE and CIE - var cie: Dwarf.CommonInformationEntry = undefined; - var fde: Dwarf.FrameDescriptionEntry = undefined; - - if (explicit_fde_offset) |fde_offset| { + const cie, const fde = if (explicit_fde_offset) |fde_offset| blk: { const dwarf_section: Dwarf.Section.Id = .eh_frame; const frame_section = di.section(dwarf_section) orelse return error.MissingFDE; if (fde_offset >= frame_section.len) return error.MissingFDE; @@ -1594,7 +1595,7 @@ pub fn unwindFrameDwarf( const cie_entry_header = try Dwarf.EntryHeader.read(&fbr, null, dwarf_section); if (cie_entry_header.type != .cie) return Dwarf.bad(); - cie = try Dwarf.CommonInformationEntry.parse( + const cie = try Dwarf.CommonInformationEntry.parse( cie_entry_header.entry_bytes, 0, true, @@ -1604,8 +1605,7 @@ pub fn unwindFrameDwarf( @sizeOf(usize), native_endian, ); - - fde = try Dwarf.FrameDescriptionEntry.parse( + const fde = try Dwarf.FrameDescriptionEntry.parse( fde_entry_header.entry_bytes, 0, true, @@ -1613,17 +1613,44 @@ pub fn unwindFrameDwarf( @sizeOf(usize), native_endian, ); - } else if (di.eh_frame_hdr) |header| { - const eh_frame_len = if (di.section(.eh_frame)) |eh_frame| eh_frame.len else null; - try header.findEntry( - ma, - eh_frame_len, - @intFromPtr(di.section(.eh_frame_hdr).?.ptr), - context.pc, - &cie, - &fde, - ); - } else { + + break :blk .{ cie, fde }; + } else blk: { + // `.eh_frame_hdr` may be incomplete. We'll try it first, but if the lookup fails, we fall + // back to loading `.eh_frame`/`.debug_frame` and using those from that point on. + + if (di.eh_frame_hdr) |header| hdr: { + const eh_frame_len = if (di.section(.eh_frame)) |eh_frame| eh_frame.len else null; + + var cie: Dwarf.CommonInformationEntry = undefined; + var fde: Dwarf.FrameDescriptionEntry = undefined; + + header.findEntry( + ma, + eh_frame_len, + @intFromPtr(di.section(.eh_frame_hdr).?.ptr), + context.pc, + &cie, + &fde, + ) catch |err| switch (err) { + error.InvalidDebugInfo => { + // `.eh_frame_hdr` appears to be incomplete, so go ahead and populate `cie_map` + // and `fde_list`, and fall back to the binary search logic below. + try di.scanCieFdeInfo(allocator, base_address); + + // Since `.eh_frame_hdr` is incomplete, we're very likely to get more lookup + // failures using it, and we've just built a complete, sorted list of FDEs + // anyway, so just stop using `.eh_frame_hdr` altogether. + di.eh_frame_hdr = null; + + break :hdr; + }, + else => return err, + }; + + break :blk .{ cie, fde }; + } + const index = std.sort.binarySearch(Dwarf.FrameDescriptionEntry, di.fde_list.items, context.pc, struct { pub fn compareFn(pc: usize, item: Dwarf.FrameDescriptionEntry) std.math.Order { if (pc < item.pc_begin) return .lt; @@ -1635,9 +1662,11 @@ pub fn unwindFrameDwarf( } }.compareFn); - fde = if (index) |i| di.fde_list.items[i] else return error.MissingFDE; - cie = di.cie_map.get(fde.cie_length_offset) orelse return error.MissingCIE; - } + const fde = if (index) |i| di.fde_list.items[i] else return error.MissingFDE; + const cie = di.cie_map.get(fde.cie_length_offset) orelse return error.MissingCIE; + + break :blk .{ cie, fde }; + }; var expression_context: Dwarf.expression.Context = .{ .format = cie.format, @@ -1802,6 +1831,8 @@ pub fn supportsUnwinding(target: std.Target) bool { } fn unwindFrameMachODwarf( + allocator: Allocator, + base_address: usize, context: *UnwindContext, ma: *std.debug.MemoryAccessor, eh_frame: []const u8, @@ -1818,7 +1849,7 @@ fn unwindFrameMachODwarf( .owned = false, }; - return unwindFrameDwarf(&di, context, ma, fde_offset); + return unwindFrameDwarf(allocator, &di, base_address, context, ma, fde_offset); } /// This is a virtual machine that runs DWARF call frame instructions.