Commit b60fc16b4f

Jacob Young <jacobly0@users.noreply.github.com>
2024-03-01 16:46:48
compiler: audit debug mode checks
* Introduce `-Ddebug-extensions` for enabling compiler debug helpers * Replace safety mode checks with `std.debug.runtime_safety` * Replace debugger helper checks with `!builtin.strip_debug_info` Sometimes, you just have to debug optimized compilers...
1 parent 155f527
lib/std/hash_map.zig
@@ -1601,7 +1601,7 @@ pub fn HashMapUnmanaged(
         }
 
         comptime {
-            if (builtin.mode == .Debug) {
+            if (!builtin.strip_debug_info) {
                 _ = &dbHelper;
             }
         }
lib/std/multi_array_list.zig
@@ -574,7 +574,7 @@ pub fn MultiArrayList(comptime T: type) type {
         }
 
         comptime {
-            if (builtin.mode == .Debug) {
+            if (!builtin.strip_debug_info) {
                 _ = &dbHelper;
                 _ = &Slice.dbHelper;
             }
src/arch/aarch64/Mir.zig
@@ -484,9 +484,9 @@ pub const Inst = struct {
     };
 
     // Make sure we don't accidentally make instructions bigger than expected.
-    // Note that in Debug builds, Zig is allowed to insert a secret field for safety checks.
+    // Note that in safety builds, Zig is allowed to insert a secret field for safety checks.
     comptime {
-        if (builtin.mode != .Debug and builtin.mode != .ReleaseSafe) {
+        if (!std.debug.runtime_safety) {
             assert(@sizeOf(Data) == 8);
         }
     }
src/arch/arm/Mir.zig
@@ -264,9 +264,9 @@ pub const Inst = struct {
     };
 
     // Make sure we don't accidentally make instructions bigger than expected.
-    // Note that in Debug builds, Zig is allowed to insert a secret field for safety checks.
+    // Note that in safety builds, Zig is allowed to insert a secret field for safety checks.
     comptime {
-        if (builtin.mode != .Debug and builtin.mode != .ReleaseSafe) {
+        if (!std.debug.runtime_safety) {
             assert(@sizeOf(Data) == 8);
         }
     }
src/arch/riscv64/Mir.zig
@@ -112,9 +112,9 @@ pub const Inst = struct {
     };
 
     // Make sure we don't accidentally make instructions bigger than expected.
-    // Note that in Debug builds, Zig is allowed to insert a secret field for safety checks.
+    // Note that in safety builds, Zig is allowed to insert a secret field for safety checks.
     // comptime {
-    //     if (builtin.mode != .Debug) {
+    //     if (!std.debug.runtime_safety) {
     //         assert(@sizeOf(Inst) == 8);
     //     }
     // }
src/arch/sparc64/Mir.zig
@@ -356,9 +356,9 @@ pub const Inst = struct {
     };
 
     // Make sure we don't accidentally make instructions bigger than expected.
-    // Note that in Debug builds, Zig is allowed to insert a secret field for safety checks.
+    // Note that in safety builds, Zig is allowed to insert a secret field for safety checks.
     comptime {
-        if (builtin.mode != .Debug and builtin.mode != .ReleaseSafe) {
+        if (!std.debug.runtime_safety) {
             assert(@sizeOf(Data) == 8);
         }
     }
src/arch/wasm/CodeGen.zig
@@ -735,7 +735,7 @@ free_locals_v128: std.ArrayListUnmanaged(u32) = .{},
 /// stored in our `values` map and therefore cause bugs.
 air_bookkeeping: @TypeOf(bookkeeping_init) = bookkeeping_init,
 
-const bookkeeping_init = if (builtin.mode == .Debug) @as(usize, 0) else {};
+const bookkeeping_init = if (std.debug.runtime_safety) @as(usize, 0) else {};
 
 const InnerError = error{
     OutOfMemory,
@@ -830,7 +830,7 @@ fn finishAir(func: *CodeGen, inst: Air.Inst.Index, result: WValue, operands: []c
         branch.values.putAssumeCapacityNoClobber(inst.toRef(), result);
     }
 
-    if (builtin.mode == .Debug) {
+    if (std.debug.runtime_safety) {
         func.air_bookkeeping += 1;
     }
 }
@@ -866,7 +866,7 @@ const BigTomb = struct {
             bt.gen.currentBranch().values.putAssumeCapacityNoClobber(bt.inst.toRef(), result);
         }
 
-        if (builtin.mode == .Debug) {
+        if (std.debug.runtime_safety) {
             bt.gen.air_bookkeeping += 1;
         }
     }
@@ -2079,7 +2079,7 @@ fn genBody(func: *CodeGen, body: []const Air.Inst.Index) InnerError!void {
         try func.currentBranch().values.ensureUnusedCapacity(func.gpa, Liveness.bpi);
         try func.genInst(inst);
 
-        if (builtin.mode == .Debug and func.air_bookkeeping < old_bookkeeping_value + 1) {
+        if (std.debug.runtime_safety and func.air_bookkeeping < old_bookkeeping_value + 1) {
             std.debug.panic("Missing call to `finishAir` in AIR instruction %{d} ('{}')", .{
                 inst,
                 func.air.instructions.items(.tag)[@intFromEnum(inst)],
src/arch/x86_64/Mir.zig
@@ -1012,9 +1012,9 @@ pub const Inst = struct {
     };
 
     // Make sure we don't accidentally make instructions bigger than expected.
-    // Note that in Debug builds, Zig is allowed to insert a secret field for safety checks.
+    // Note that in safety builds, Zig is allowed to insert a secret field for safety checks.
     comptime {
-        if (builtin.mode != .Debug and builtin.mode != .ReleaseSafe) {
+        if (!std.debug.runtime_safety) {
             assert(@sizeOf(Data) == 8);
         }
     }
src/Air.zig
@@ -1102,10 +1102,10 @@ pub const Inst = struct {
         };
 
         // Make sure we don't accidentally add a field to make this union
-        // bigger than expected. Note that in Debug builds, Zig is allowed
+        // bigger than expected. Note that in safety builds, Zig is allowed
         // to insert a secret field for safety checks.
         comptime {
-            if (builtin.mode != .Debug and builtin.mode != .ReleaseSafe) {
+            if (!std.debug.runtime_safety) {
                 assert(@sizeOf(Data) == 8);
             }
         }
src/Compilation.zig
@@ -2191,14 +2191,14 @@ pub fn update(comp: *Compilation, main_progress_node: *std.Progress.Node) !void
     try comp.performAllTheWork(main_progress_node);
 
     if (comp.module) |module| {
-        if (builtin.mode == .Debug and comp.verbose_intern_pool) {
+        if (build_options.enable_debug_extensions and comp.verbose_intern_pool) {
             std.debug.print("intern pool stats for '{s}':\n", .{
                 comp.root_name,
             });
             module.intern_pool.dump();
         }
 
-        if (builtin.mode == .Debug and comp.verbose_generic_instances) {
+        if (build_options.enable_debug_extensions and comp.verbose_generic_instances) {
             std.debug.print("generic instances for '{s}:0x{x}':\n", .{
                 comp.root_name,
                 @as(usize, @intFromPtr(module)),
src/crash_report.zig
@@ -1,5 +1,6 @@
 const std = @import("std");
 const builtin = @import("builtin");
+const build_options = @import("build_options");
 const debug = std.debug;
 const os = std.os;
 const io = std.io;
@@ -11,37 +12,26 @@ const Sema = @import("Sema.zig");
 const Zir = std.zig.Zir;
 const Decl = Module.Decl;
 
-pub const is_enabled = builtin.mode == .Debug;
-
 /// To use these crash report diagnostics, publish this panic in your main file
 /// and add `pub const enable_segfault_handler = false;` to your `std_options`.
 /// You will also need to call initialize() on startup, preferably as the very first operation in your program.
-pub const panic = if (is_enabled) compilerPanic else std.builtin.default_panic;
+pub const panic = if (build_options.enable_debug_extensions) compilerPanic else std.builtin.default_panic;
 
 /// Install signal handlers to identify crashes and report diagnostics.
 pub fn initialize() void {
-    if (is_enabled and debug.have_segfault_handling_support) {
+    if (build_options.enable_debug_extensions and debug.have_segfault_handling_support) {
         attachSegfaultHandler();
     }
 }
 
-fn En(comptime T: type) type {
-    return if (is_enabled) T else void;
-}
-
-fn en(val: anytype) En(@TypeOf(val)) {
-    return if (is_enabled) val else {};
-}
-
-pub const AnalyzeBody = struct {
-    parent: if (is_enabled) ?*AnalyzeBody else void,
-    sema: En(*Sema),
-    block: En(*Sema.Block),
-    body: En([]const Zir.Inst.Index),
-    body_index: En(usize),
+pub const AnalyzeBody = if (build_options.enable_debug_extensions) struct {
+    parent: ?*AnalyzeBody,
+    sema: *Sema,
+    block: *Sema.Block,
+    body: []const Zir.Inst.Index,
+    body_index: usize,
 
     pub fn push(self: *@This()) void {
-        if (!is_enabled) return;
         const head = &zir_state;
         debug.assert(self.parent == null);
         self.parent = head.*;
@@ -49,7 +39,6 @@ pub const AnalyzeBody = struct {
     }
 
     pub fn pop(self: *@This()) void {
-        if (!is_enabled) return;
         const head = &zir_state;
         const old = head.*.?;
         debug.assert(old == self);
@@ -57,27 +46,24 @@ pub const AnalyzeBody = struct {
     }
 
     pub fn setBodyIndex(self: *@This(), index: usize) void {
-        if (!is_enabled) return;
         self.body_index = index;
     }
+} else struct {
+    pub inline fn push(_: @This()) void {}
+    pub inline fn pop(_: @This()) void {}
+    pub inline fn setBodyIndex(_: @This(), _: usize) void {}
 };
 
-threadlocal var zir_state: ?*AnalyzeBody = if (is_enabled) null else @compileError("Cannot use zir_state if crash_report is disabled.");
+threadlocal var zir_state: ?*AnalyzeBody = if (build_options.enable_debug_extensions) null else @compileError("Cannot use zir_state without debug extensions.");
 
 pub fn prepAnalyzeBody(sema: *Sema, block: *Sema.Block, body: []const Zir.Inst.Index) AnalyzeBody {
-    if (is_enabled) {
-        return .{
-            .parent = null,
-            .sema = sema,
-            .block = block,
-            .body = body,
-            .body_index = 0,
-        };
-    } else {
-        if (@sizeOf(AnalyzeBody) != 0)
-            @compileError("AnalyzeBody must have zero size when crash reports are disabled");
-        return undefined;
-    }
+    return if (build_options.enable_debug_extensions) .{
+        .parent = null,
+        .sema = sema,
+        .block = block,
+        .body = body,
+        .body_index = 0,
+    } else .{};
 }
 
 fn dumpStatusReport() !void {
src/InternPool.zig
@@ -2593,7 +2593,7 @@ pub const Index = enum(u32) {
     }
 
     comptime {
-        if (builtin.mode == .Debug) {
+        if (!builtin.strip_debug_info) {
             _ = &dbHelper;
         }
     }
src/main.zig
@@ -67,8 +67,6 @@ pub fn fatal(comptime format: []const u8, args: anytype) noreturn {
     process.exit(1);
 }
 
-const debug_extensions_enabled = builtin.mode == .Debug;
-
 const normal_usage =
     \\Usage: zig [command] [options]
     \\
@@ -120,7 +118,7 @@ const debug_usage = normal_usage ++
     \\
 ;
 
-const usage = if (debug_extensions_enabled) debug_usage else normal_usage;
+const usage = if (build_options.enable_debug_extensions) debug_usage else normal_usage;
 
 var log_scopes: std.ArrayListUnmanaged([]const u8) = .{};
 
@@ -334,9 +332,9 @@ fn mainArgs(gpa: Allocator, arena: Allocator, args: []const []const u8) !void {
         return io.getStdOut().writeAll(usage);
     } else if (mem.eql(u8, cmd, "ast-check")) {
         return cmdAstCheck(gpa, arena, cmd_args);
-    } else if (debug_extensions_enabled and mem.eql(u8, cmd, "changelist")) {
+    } else if (build_options.enable_debug_extensions and mem.eql(u8, cmd, "changelist")) {
         return cmdChangelist(gpa, arena, cmd_args);
-    } else if (debug_extensions_enabled and mem.eql(u8, cmd, "dump-zir")) {
+    } else if (build_options.enable_debug_extensions and mem.eql(u8, cmd, "dump-zir")) {
         return cmdDumpZir(gpa, arena, cmd_args);
     } else {
         std.log.info("{s}", .{usage});
@@ -1591,10 +1589,10 @@ fn buildOutputType(
                             });
                         };
                     } else if (mem.eql(u8, arg, "--debug-compile-errors")) {
-                        if (!crash_report.is_enabled) {
-                            warn("Zig was compiled in a release mode. --debug-compile-errors has no effect.", .{});
-                        } else {
+                        if (build_options.enable_debug_extensions) {
                             debug_compile_errors = true;
+                        } else {
+                            warn("Zig was compiled without debug extensions. --debug-compile-errors has no effect.", .{});
                         }
                     } else if (mem.eql(u8, arg, "--verbose-link")) {
                         verbose_link = true;
@@ -5076,10 +5074,10 @@ fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !void {
                     }
                     continue;
                 } else if (mem.eql(u8, arg, "--debug-compile-errors")) {
-                    if (!crash_report.is_enabled) {
-                        warn("Zig was compiled in a release mode. --debug-compile-errors has no effect.", .{});
-                    } else {
+                    if (build_options.enable_debug_extensions) {
                         debug_compile_errors = true;
+                    } else {
+                        warn("Zig was compiled without debug extensions. --debug-compile-errors has no effect.", .{});
                     }
                 } else if (mem.eql(u8, arg, "--verbose-link")) {
                     verbose_link = true;
@@ -6317,8 +6315,8 @@ fn cmdAstCheck(
     if (!want_output_text) {
         return cleanExit();
     }
-    if (!debug_extensions_enabled) {
-        fatal("-t option only available in debug builds of zig", .{});
+    if (!build_options.enable_debug_extensions) {
+        fatal("-t option only available in builds of zig with debug extensions", .{});
     }
 
     {
src/Module.zig
@@ -3203,8 +3203,8 @@ pub fn ensureFuncBodyAnalyzed(zcu: *Zcu, func_index: InternPool.Index) SemaError
 
     const comp = zcu.comp;
 
-    const dump_air = builtin.mode == .Debug and comp.verbose_air;
-    const dump_llvm_ir = builtin.mode == .Debug and (comp.verbose_llvm_ir != null or comp.verbose_llvm_bc != null);
+    const dump_air = build_options.enable_debug_extensions and comp.verbose_air;
+    const dump_llvm_ir = build_options.enable_debug_extensions and (comp.verbose_llvm_ir != null or comp.verbose_llvm_bc != null);
 
     if (comp.bin_file == null and zcu.llvm_object == null and !dump_air and !dump_llvm_ir) {
         return;
src/Sema.zig
@@ -2505,7 +2505,7 @@ fn failWithOwnedErrorMsg(sema: *Sema, block: ?*Block, err_msg: *Module.ErrorMsg)
     ref: {
         errdefer err_msg.destroy(gpa);
 
-        if (crash_report.is_enabled and mod.comp.debug_compile_errors) {
+        if (build_options.enable_debug_extensions and mod.comp.debug_compile_errors) {
             var wip_errors: std.zig.ErrorBundle.Wip = undefined;
             wip_errors.init(gpa) catch unreachable;
             Compilation.addModuleErrorMsg(mod, &wip_errors, err_msg.*) catch unreachable;
@@ -5758,7 +5758,7 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr
     const body = sema.code.bodySlice(extra.end, extra.data.body_len);
 
     // we check this here to avoid undefined symbols
-    if (!@import("build_options").have_llvm)
+    if (!build_options.have_llvm)
         return sema.fail(parent_block, src, "C import unavailable; Zig compiler built without LLVM extensions", .{});
 
     var c_import_buf = std.ArrayList(u8).init(gpa);
src/Value.zig
@@ -4069,7 +4069,7 @@ fn dbHelper(self: *Value, tag_to_payload_map: *map: {
 }
 
 comptime {
-    if (builtin.mode == .Debug) {
+    if (!builtin.strip_debug_info) {
         _ = &dbHelper;
     }
 }
stage1/config.zig.in
@@ -5,11 +5,12 @@ pub const llvm_has_arc = false;
 pub const llvm_has_xtensa = false;
 pub const version: [:0]const u8 = "@RESOLVED_ZIG_VERSION@";
 pub const semver = @import("std").SemanticVersion.parse(version) catch unreachable;
-pub const enable_logging: bool = false;
-pub const enable_link_snapshots: bool = false;
+pub const enable_debug_extensions = false;
+pub const enable_logging = false;
+pub const enable_link_snapshots = false;
 pub const enable_tracy = false;
 pub const value_tracing = false;
 pub const skip_non_native = false;
-pub const only_c = false;
 pub const force_gpa = false;
+pub const only_c = false;
 pub const only_core_functionality = true;
bootstrap.c
@@ -131,15 +131,15 @@ int main(int argc, char **argv) {
             "pub const llvm_has_xtensa = false;\n"
             "pub const version: [:0]const u8 = \"%s\";\n"
             "pub const semver = @import(\"std\").SemanticVersion.parse(version) catch unreachable;\n"
-            "pub const enable_logging: bool = false;\n"
-            "pub const enable_link_snapshots: bool = false;\n"
+            "pub const enable_debug_extensions = false;\n"
+            "pub const enable_logging = false;\n"
+            "pub const enable_link_snapshots = false;\n"
             "pub const enable_tracy = false;\n"
             "pub const value_tracing = false;\n"
             "pub const skip_non_native = false;\n"
-            "pub const only_c = false;\n"
             "pub const force_gpa = false;\n"
+            "pub const only_c = false;\n"
             "pub const only_core_functionality = true;\n"
-            "pub const only_reduce = false;\n"
         , zig_version);
         if (written < 100)
             panic("unable to write to config.zig file");
build.zig
@@ -250,6 +250,7 @@ pub fn build(b: *std.Build) !void {
     }
 
     const is_debug = optimize == .Debug;
+    const enable_debug_extensions = b.option(bool, "debug-extensions", "Enable commands and options useful for debugging the compiler") orelse is_debug;
     const enable_logging = b.option(bool, "log", "Enable debug logging with --debug-log") orelse is_debug;
     const enable_link_snapshots = b.option(bool, "link-snapshot", "Whether to enable linker state snapshots") orelse false;
 
@@ -357,6 +358,7 @@ pub fn build(b: *std.Build) !void {
     const semver = try std.SemanticVersion.parse(version);
     exe_options.addOption(std.SemanticVersion, "semver", semver);
 
+    exe_options.addOption(bool, "enable_debug_extensions", enable_debug_extensions);
     exe_options.addOption(bool, "enable_logging", enable_logging);
     exe_options.addOption(bool, "enable_link_snapshots", enable_link_snapshots);
     exe_options.addOption(bool, "enable_tracy", tracy != null);
@@ -393,6 +395,7 @@ pub fn build(b: *std.Build) !void {
     check_case_exe.root_module.addOptions("build_options", test_cases_options);
 
     test_cases_options.addOption(bool, "enable_tracy", false);
+    test_cases_options.addOption(bool, "enable_debug_extensions", enable_debug_extensions);
     test_cases_options.addOption(bool, "enable_logging", enable_logging);
     test_cases_options.addOption(bool, "enable_link_snapshots", enable_link_snapshots);
     test_cases_options.addOption(bool, "skip_non_native", skip_non_native);
@@ -588,6 +591,7 @@ fn addWasiUpdateStep(b: *std.Build, version: [:0]const u8) !void {
     exe_options.addOption(bool, "only_c", true);
     exe_options.addOption([:0]const u8, "version", version);
     exe_options.addOption(std.SemanticVersion, "semver", semver);
+    exe_options.addOption(bool, "enable_debug_extensions", false);
     exe_options.addOption(bool, "enable_logging", false);
     exe_options.addOption(bool, "enable_link_snapshots", false);
     exe_options.addOption(bool, "enable_tracy", false);