Commit ef4d7f01a5

Alex Rønne Petersen <alex@alexrp.com>
2025-01-23 01:38:20
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.
1 parent b46a40f
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,
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;
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,
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,
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(),
         },
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;
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;