Merge pull request #25227 from mlugg/capture-stack

The Great `std.debug` Refactor

4 files changed, 396 insertions, 144 deletions

diff --git a/test/src/ErrorTrace.zig b/test/src/ErrorTrace.zig
new file mode 100644
index 0000000000..d8ff380332
--- /dev/null
+++ b/test/src/ErrorTrace.zig
@@ -0,0 +1,130 @@
+b: *std.Build,
+step: *Step,
+test_filters: []const []const u8,
+targets: []const std.Build.ResolvedTarget,
+optimize_modes: []const OptimizeMode,
+convert_exe: *std.Build.Step.Compile,
+
+pub const Case = struct {
+    name: []const u8,
+    source: []const u8,
+    expect_error: []const u8,
+    expect_trace: []const u8,
+    /// On these arch/OS pairs we will not test the error trace on optimized LLVM builds because the
+    /// optimizations break the error trace. We will test the binary with error tracing disabled,
+    /// just to ensure that the expected error is still returned from `main`.
+    ///
+    /// LLVM ReleaseSmall builds always have the trace disabled regardless of this field, because it
+    /// seems that LLVM is particularly good at optimizing traces away in those.
+    disable_trace_optimized: []const DisableConfig = &.{},
+
+    pub const DisableConfig = struct { std.Target.Cpu.Arch, std.Target.Os.Tag };
+    pub const Backend = enum { llvm, selfhosted };
+};
+
+pub fn addCase(self: *ErrorTrace, case: Case) void {
+    for (self.targets) |*target| {
+        const triple: ?[]const u8 = if (target.query.isNative()) null else t: {
+            break :t target.query.zigTriple(self.b.graph.arena) catch @panic("OOM");
+        };
+        for (self.optimize_modes) |optimize| {
+            self.addCaseConfig(case, target, triple, optimize, .llvm);
+        }
+        if (shouldTestNonLlvm(&target.result)) {
+            for (self.optimize_modes) |optimize| {
+                self.addCaseConfig(case, target, triple, optimize, .selfhosted);
+            }
+        }
+    }
+}
+
+fn shouldTestNonLlvm(target: *const std.Target) bool {
+    return switch (target.cpu.arch) {
+        .x86_64 => switch (target.ofmt) {
+            .elf => true,
+            else => false,
+        },
+        else => false,
+    };
+}
+
+fn addCaseConfig(
+    self: *ErrorTrace,
+    case: Case,
+    target: *const std.Build.ResolvedTarget,
+    triple: ?[]const u8,
+    optimize: OptimizeMode,
+    backend: Case.Backend,
+) void {
+    const b = self.b;
+
+    const error_tracing: bool = tracing: {
+        if (optimize == .Debug) break :tracing true;
+        if (backend != .llvm) break :tracing true;
+        if (optimize == .ReleaseSmall) break :tracing false;
+        for (case.disable_trace_optimized) |disable| {
+            const d_arch, const d_os = disable;
+            if (target.result.cpu.arch == d_arch and target.result.os.tag == d_os) {
+                // This particular configuration cannot do error tracing in optimized LLVM builds.
+                break :tracing false;
+            }
+        }
+        break :tracing true;
+    };
+
+    const annotated_case_name = b.fmt("check {s} ({s}{s}{s} {s})", .{
+        case.name,
+        triple orelse "",
+        if (triple != null) " " else "",
+        @tagName(optimize),
+        @tagName(backend),
+    });
+    if (self.test_filters.len > 0) {
+        for (self.test_filters) |test_filter| {
+            if (mem.indexOf(u8, annotated_case_name, test_filter)) |_| break;
+        } else return;
+    }
+
+    const write_files = b.addWriteFiles();
+    const source_zig = write_files.add("source.zig", case.source);
+    const exe = b.addExecutable(.{
+        .name = "test",
+        .root_module = b.createModule(.{
+            .root_source_file = source_zig,
+            .optimize = optimize,
+            .target = target.*,
+            .error_tracing = error_tracing,
+            .strip = false,
+        }),
+        .use_llvm = switch (backend) {
+            .llvm => true,
+            .selfhosted => false,
+        },
+    });
+    exe.bundle_ubsan_rt = false;
+
+    const run = b.addRunArtifact(exe);
+    run.removeEnvironmentVariable("CLICOLOR_FORCE");
+    run.setEnvironmentVariable("NO_COLOR", "1");
+    run.expectExitCode(1);
+    run.expectStdOutEqual("");
+
+    const expected_stderr = switch (error_tracing) {
+        true => b.fmt("error: {s}\n{s}\n", .{ case.expect_error, case.expect_trace }),
+        false => b.fmt("error: {s}\n", .{case.expect_error}),
+    };
+
+    const check_run = b.addRunArtifact(self.convert_exe);
+    check_run.setName(annotated_case_name);
+    check_run.addFileArg(run.captureStdErr(.{}));
+    check_run.expectStdOutEqual(expected_stderr);
+
+    self.step.dependOn(&check_run.step);
+}
+
+const ErrorTrace = @This();
+const std = @import("std");
+const builtin = @import("builtin");
+const Step = std.Build.Step;
+const OptimizeMode = std.builtin.OptimizeMode;
+const mem = std.mem;
diff --git a/test/src/StackTrace.zig b/test/src/StackTrace.zig
index 9b51f4e4b2..3d35a4f935 100644
--- a/test/src/StackTrace.zig
+++ b/test/src/StackTrace.zig
@@ -1,75 +1,171 @@
 b: *std.Build,
 step: *Step,
-test_index: usize,
 test_filters: []const []const u8,
-optimize_modes: []const OptimizeMode,
-check_exe: *std.Build.Step.Compile,
+targets: []const std.Build.ResolvedTarget,
+convert_exe: *std.Build.Step.Compile,
 
 const Config = struct {
     name: []const u8,
     source: []const u8,
-    Debug: ?PerMode = null,
-    ReleaseSmall: ?PerMode = null,
-    ReleaseSafe: ?PerMode = null,
-    ReleaseFast: ?PerMode = null,
-
-    const PerMode = struct {
-        expect: []const u8,
-        exclude_arch: []const std.Target.Cpu.Arch = &.{},
-        exclude_os: []const std.Target.Os.Tag = &.{},
-        error_tracing: ?bool = null,
-    };
+    /// Whether this test case expects to have unwind tables / frame pointers.
+    unwind: enum {
+        /// This case assumes that some unwind strategy, safe or unsafe, is available.
+        any,
+        /// This case assumes that no unwinding strategy is available.
+        none,
+        /// This case assumes that a safe unwind strategy, like DWARF unwinding, is available.
+        safe,
+        /// This case assumes that at most, unsafe FP unwinding is available.
+        no_safe,
+    },
+    /// If `true`, the expected exit code is that of the default panic handler, rather than 0.
+    expect_panic: bool,
+    /// When debug info is not stripped, stdout is expected to **contain** (not equal!) this string.
+    expect: []const u8,
+    /// When debug info *is* stripped, stdout is expected to **contain** (not equal!) this string.
+    expect_strip: []const u8,
 };
 
 pub fn addCase(self: *StackTrace, config: Config) void {
-    self.addCaseInner(config, true);
-    if (shouldTestNonLlvm(&self.b.graph.host.result)) {
-        self.addCaseInner(config, false);
+    for (self.targets) |*target| {
+        addCaseTarget(
+            self,
+            config,
+            target,
+            if (target.query.isNative()) null else t: {
+                break :t target.query.zigTriple(self.b.graph.arena) catch @panic("OOM");
+            },
+        );
     }
 }
-
-fn addCaseInner(self: *StackTrace, config: Config, use_llvm: bool) void {
-    if (config.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, use_llvm, per_mode);
-
-    if (config.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, use_llvm, per_mode);
-}
-
-fn shouldTestNonLlvm(target: *const std.Target) bool {
-    return switch (target.cpu.arch) {
-        .x86_64 => switch (target.ofmt) {
-            .elf => !target.os.tag.isBSD(),
+fn addCaseTarget(
+    self: *StackTrace,
+    config: Config,
+    target: *const std.Build.ResolvedTarget,
+    triple: ?[]const u8,
+) void {
+    const both_backends = switch (target.result.cpu.arch) {
+        .x86_64 => switch (target.result.ofmt) {
+            .elf => true,
             else => false,
         },
         else => false,
     };
+    const both_pie = switch (target.result.os.tag) {
+        .fuchsia, .openbsd => false,
+        else => true,
+    };
+    const both_libc = switch (target.result.os.tag) {
+        .freebsd, .netbsd => false,
+        else => !target.result.requiresLibC(),
+    };
+
+    // On aarch64-macos, FP unwinding is blessed by Apple to always be reliable, and std.debug knows this.
+    const fp_unwind_is_safe = target.result.cpu.arch == .aarch64 and target.result.os.tag.isDarwin();
+    const supports_unwind_tables = switch (target.result.os.tag) {
+        // x86-windows just has no way to do stack unwinding other then using frame pointers.
+        .windows => target.result.cpu.arch != .x86,
+        // We do not yet implement support for the AArch32 exception table section `.ARM.exidx`.
+        else => !target.result.cpu.arch.isArm(),
+    };
+
+    const use_llvm_vals: []const bool = if (both_backends) &.{ true, false } else &.{true};
+    const pie_vals: []const ?bool = if (both_pie) &.{ true, false } else &.{null};
+    const link_libc_vals: []const ?bool = if (both_libc) &.{ true, false } else &.{null};
+    const strip_debug_vals: []const bool = &.{ true, false };
+
+    const UnwindInfo = packed struct(u2) {
+        tables: bool,
+        fp: bool,
+        const none: @This() = .{ .tables = false, .fp = false };
+        const both: @This() = .{ .tables = true, .fp = true };
+        const only_tables: @This() = .{ .tables = true, .fp = false };
+        const only_fp: @This() = .{ .tables = false, .fp = true };
+    };
+    const unwind_info_vals: []const UnwindInfo = switch (config.unwind) {
+        .none => &.{.none},
+        .any => &.{ .only_tables, .only_fp, .both },
+        .safe => if (fp_unwind_is_safe) &.{ .only_tables, .only_fp, .both } else &.{ .only_tables, .both },
+        .no_safe => if (fp_unwind_is_safe) &.{.none} else &.{ .none, .only_fp },
+    };
+
+    for (use_llvm_vals) |use_llvm| {
+        for (pie_vals) |pie| {
+            for (link_libc_vals) |link_libc| {
+                for (strip_debug_vals) |strip_debug| {
+                    for (unwind_info_vals) |unwind_info| {
+                        if (unwind_info.tables and !supports_unwind_tables) continue;
+                        self.addCaseInstance(
+                            target,
+                            triple,
+                            config.name,
+                            config.source,
+                            use_llvm,
+                            pie,
+                            link_libc,
+                            strip_debug,
+                            !unwind_info.tables and supports_unwind_tables,
+                            !unwind_info.fp,
+                            config.expect_panic,
+                            if (strip_debug) config.expect_strip else config.expect,
+                        );
+                    }
+                }
+            }
+        }
+    }
 }
 
-fn addExpect(
+fn addCaseInstance(
     self: *StackTrace,
+    target: *const std.Build.ResolvedTarget,
+    triple: ?[]const u8,
     name: []const u8,
     source: []const u8,
-    optimize_mode: OptimizeMode,
     use_llvm: bool,
-    mode_config: Config.PerMode,
+    pie: ?bool,
+    link_libc: ?bool,
+    strip_debug: bool,
+    strip_unwind: bool,
+    omit_frame_pointer: bool,
+    expect_panic: bool,
+    expect_stderr: []const u8,
 ) void {
-    for (mode_config.exclude_arch) |tag| if (tag == builtin.cpu.arch) return;
-    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} {s})", .{
-        name, @tagName(optimize_mode), if (use_llvm) "llvm" else "selfhosted",
+
+    if (strip_debug) {
+        // To enable this coverage, one of two things needs to happen:
+        // * The compiler needs to gain the ability to strip only debug info (not symbols)
+        // * `std.Build.Step.ObjCopy` needs to be un-regressed
+        return;
+    }
+
+    if (strip_unwind) {
+        // To enable this coverage, `std.Build.Step.ObjCopy` needs to be un-regressed and gain the
+        // ability to remove individual sections. `-fno-unwind-tables` is insufficient because it
+        // does not prevent `.debug_frame` from being emitted. If we could, we would remove the
+        // following sections:
+        // * `.eh_frame`, `.eh_frame_hdr`, `.debug_frame` (Linux)
+        // * `__TEXT,__eh_frame`, `__TEXT,__unwind_info` (macOS)
+        return;
+    }
+
+    const annotated_case_name = b.fmt("check {s} ({s}{s}{s}{s}{s}{s}{s}{s})", .{
+        name,
+        triple orelse "",
+        if (triple != null) " " else "",
+        if (use_llvm) "llvm" else "selfhosted",
+        if (pie == true) " pie" else "",
+        if (link_libc == true) " libc" else "",
+        if (strip_debug) " strip" else "",
+        if (strip_unwind) " no_unwind" else "",
+        if (omit_frame_pointer) " no_fp" else "",
     });
-    for (self.test_filters) |test_filter| {
-        if (mem.indexOf(u8, annotated_case_name, test_filter)) |_| break;
-    } else if (self.test_filters.len > 0) return;
+    if (self.test_filters.len > 0) {
+        for (self.test_filters) |test_filter| {
+            if (mem.indexOf(u8, annotated_case_name, test_filter)) |_| break;
+        } else return;
+    }
 
     const write_files = b.addWriteFiles();
     const source_zig = write_files.add("source.zig", source);
@@ -77,27 +173,34 @@ fn addExpect(
         .name = "test",
         .root_module = b.createModule(.{
             .root_source_file = source_zig,
-            .optimize = optimize_mode,
-            .target = b.graph.host,
-            .error_tracing = mode_config.error_tracing,
+            .optimize = .Debug,
+            .target = target.*,
+            .omit_frame_pointer = omit_frame_pointer,
+            .link_libc = link_libc,
+            .unwind_tables = if (strip_unwind) .none else null,
+            // make panics single-threaded so that they don't include a thread ID
+            .single_threaded = expect_panic,
         }),
         .use_llvm = use_llvm,
     });
+    exe.pie = pie;
     exe.bundle_ubsan_rt = false;
 
     const run = b.addRunArtifact(exe);
     run.removeEnvironmentVariable("CLICOLOR_FORCE");
     run.setEnvironmentVariable("NO_COLOR", "1");
-    run.expectExitCode(1);
+    run.addCheck(.{ .expect_term = term: {
+        if (!expect_panic) break :term .{ .Exited = 0 };
+        if (target.result.os.tag == .windows) break :term .{ .Exited = 3 };
+        break :term .{ .Signal = 6 };
+    } });
     run.expectStdOutEqual("");
 
-    const check_run = b.addRunArtifact(self.check_exe);
+    const check_run = b.addRunArtifact(self.convert_exe);
     check_run.setName(annotated_case_name);
     check_run.addFileArg(run.captureStdErr(.{}));
-    check_run.addArgs(&.{
-        @tagName(optimize_mode),
-    });
-    check_run.expectStdOutEqual(mode_config.expect);
+    check_run.expectExitCode(0);
+    check_run.addCheck(.{ .expect_stdout_match = expect_stderr });
 
     self.step.dependOn(&check_run.step);
 }
diff --git a/test/src/check-stack-trace.zig b/test/src/check-stack-trace.zig
deleted file mode 100644
index 411a2ab53e..0000000000
--- a/test/src/check-stack-trace.zig
+++ /dev/null
@@ -1,88 +0,0 @@
-const builtin = @import("builtin");
-const std = @import("std");
-const mem = std.mem;
-const fs = std.fs;
-
-pub fn main() !void {
-    var arena_instance = std.heap.ArenaAllocator.init(std.heap.page_allocator);
-    defer arena_instance.deinit();
-    const arena = arena_instance.allocator();
-
-    const args = try std.process.argsAlloc(arena);
-
-    const input_path = args[1];
-    const optimize_mode_text = args[2];
-
-    const input_bytes = try std.fs.cwd().readFileAlloc(input_path, arena, .limited(5 * 1024 * 1024));
-    const optimize_mode = std.meta.stringToEnum(std.builtin.OptimizeMode, optimize_mode_text).?;
-
-    var stderr = input_bytes;
-
-    // process result
-    // - keep only basename of source file path
-    // - replace address with symbolic string
-    // - replace function name with symbolic string when optimize_mode != .Debug
-    // - skip empty lines
-    const got: []const u8 = got_result: {
-        var buf = std.array_list.Managed(u8).init(arena);
-        defer buf.deinit();
-        if (stderr.len != 0 and stderr[stderr.len - 1] == '\n') stderr = stderr[0 .. stderr.len - 1];
-        var it = mem.splitScalar(u8, stderr, '\n');
-        process_lines: while (it.next()) |line| {
-            if (line.len == 0) continue;
-
-            // offset search past `[drive]:` on windows
-            var pos: usize = if (builtin.os.tag == .windows) 2 else 0;
-            // locate delims/anchor
-            const delims = [_][]const u8{ ":", ":", ":", " in ", "(", ")" };
-            var marks = [_]usize{0} ** delims.len;
-            for (delims, 0..) |delim, i| {
-                marks[i] = mem.indexOfPos(u8, line, pos, delim) orelse {
-                    // unexpected pattern: emit raw line and cont
-                    try buf.appendSlice(line);
-                    try buf.appendSlice("\n");
-                    continue :process_lines;
-                };
-                pos = marks[i] + delim.len;
-            }
-            // locate source basename
-            pos = mem.lastIndexOfScalar(u8, line[0..marks[0]], fs.path.sep) orelse {
-                // unexpected pattern: emit raw line and cont
-                try buf.appendSlice(line);
-                try buf.appendSlice("\n");
-                continue :process_lines;
-            };
-            // end processing if source basename changes
-            if (!mem.eql(u8, "source.zig", line[pos + 1 .. marks[0]])) break;
-            // emit substituted line
-            try buf.appendSlice(line[pos + 1 .. marks[2] + delims[2].len]);
-            try buf.appendSlice(" [address]");
-            if (optimize_mode == .Debug) {
-                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_zcu.obj") or
-                    std.mem.endsWith(u8, file_name, ".zig"))
-                {
-                    try buf.appendSlice("[main_file]");
-                } else {
-                    // 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]");
-            }
-            try buf.appendSlice("\n");
-        }
-        break :got_result try buf.toOwnedSlice();
-    };
-
-    try std.fs.File.stdout().writeAll(got);
-}
diff --git a/test/src/convert-stack-trace.zig b/test/src/convert-stack-trace.zig
new file mode 100644
index 0000000000..c7cd01a460
--- /dev/null
+++ b/test/src/convert-stack-trace.zig
@@ -0,0 +1,107 @@
+//! Accepts a stack trace in a file (whose path is given as argv[1]), and removes all
+//! non-reproducible information from it, including addresses, module names, and file
+//! paths. All module names are removed, file paths become just their basename, and
+//! addresses are replaced with a fixed string. So, lines like this:
+//!
+//!   /something/foo.zig:1:5: 0x12345678 in bar (main.o)
+//!       doThing();
+//!              ^
+//!   ???:?:?: 0x12345678 in qux (other.o)
+//!   ???:?:?: 0x12345678 in ??? (???)
+//!
+//! ...are turned into lines like this:
+//!
+//!   foo.zig:1:5: [address] in bar
+//!       doThing();
+//!              ^
+//!   ???:?:?: [address] in qux
+//!   ???:?:?: [address] in ???
+//!
+//! Additionally, lines reporting unwind errors are removed:
+//!
+//!   Unwind error at address `/proc/self/exe:0x1016533` (unwind info unavailable), remaining frames may be incorrect
+//!   Cannot print stack trace: safe unwind unavilable for target
+//!
+//! With these transformations, the test harness can safely do string comparisons.
+
+pub fn main() !void {
+    var arena_instance: std.heap.ArenaAllocator = .init(std.heap.page_allocator);
+    defer arena_instance.deinit();
+    const arena = arena_instance.allocator();
+
+    const args = try std.process.argsAlloc(arena);
+    if (args.len != 2) std.process.fatal("usage: convert-stack-trace path/to/test/output", .{});
+
+    var read_buf: [1024]u8 = undefined;
+    var write_buf: [1024]u8 = undefined;
+
+    const in_file = try std.fs.cwd().openFile(args[1], .{});
+    defer in_file.close();
+
+    const out_file: std.fs.File = .stdout();
+
+    var in_fr = in_file.reader(&read_buf);
+    var out_fw = out_file.writer(&write_buf);
+
+    const w = &out_fw.interface;
+
+    while (in_fr.interface.takeDelimiterInclusive('\n')) |in_line| {
+        if (std.mem.eql(u8, in_line, "Cannot print stack trace: safe unwind unavailable for target\n") or
+            std.mem.startsWith(u8, in_line, "Unwind error at address `"))
+        {
+            // Remove these lines from the output.
+            continue;
+        }
+
+        const src_col_end = std.mem.indexOf(u8, in_line, ": 0x") orelse {
+            try w.writeAll(in_line);
+            continue;
+        };
+        const src_row_end = std.mem.lastIndexOfScalar(u8, in_line[0..src_col_end], ':') orelse {
+            try w.writeAll(in_line);
+            continue;
+        };
+        const src_path_end = std.mem.lastIndexOfScalar(u8, in_line[0..src_row_end], ':') orelse {
+            try w.writeAll(in_line);
+            continue;
+        };
+
+        const addr_end = std.mem.indexOfPos(u8, in_line, src_col_end, " in ") orelse {
+            try w.writeAll(in_line);
+            continue;
+        };
+        const symbol_end = std.mem.indexOfPos(u8, in_line, addr_end, " (") orelse {
+            try w.writeAll(in_line);
+            continue;
+        };
+        if (!std.mem.endsWith(u8, std.mem.trimEnd(u8, in_line, "\n"), ")")) {
+            try w.writeAll(in_line);
+            continue;
+        }
+
+        // Where '_' is a placeholder for an arbitrary string, we now know the line looks like:
+        //
+        //   _:_:_: 0x_ in _ (_)
+        //
+        // That seems good enough to assume it's a stack trace frame! We'll rewrite it to:
+        //
+        //   _:_:_: [address] in _
+        //
+        // ...with that first '_' being replaced by its basename.
+
+        const src_path = in_line[0..src_path_end];
+        const basename_start = if (std.mem.lastIndexOfAny(u8, src_path, "/\\")) |i| i + 1 else 0;
+        const symbol_start = addr_end + " in ".len;
+        try w.writeAll(in_line[basename_start..src_col_end]);
+        try w.writeAll(": [address] in ");
+        try w.writeAll(in_line[symbol_start..symbol_end]);
+        try w.writeByte('\n');
+    } else |err| switch (err) {
+        error.EndOfStream => {},
+        else => |e| return e,
+    }
+
+    try w.flush();
+}
+
+const std = @import("std");