Merge pull request #16559 from kcbanner/improve_compiler_rt_stack_trace

Unwinding follow up: Don't strip compiler_rt symbols, enable unwind tables on supported platforms

Author: andrewrk

lib/std/Build/Step/CheckObject.zig

@@ -478,8 +478,7 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
                 },
                 .not_present => {
                     while (it.next()) |line| {
-                        if (act.notPresent(b, step, line)) break;
-                    } else {
+                        if (act.notPresent(b, step, line)) continue;
                         return step.fail(
                             \\
                             \\========= expected not to find: ===================

lib/std/debug.zig

@@ -242,8 +242,7 @@ pub fn dumpStackTraceFromBase(context: *const ThreadContext) void {
         printSourceAtAddress(debug_info, stderr, it.unwind_state.?.dwarf_context.pc, tty_config) catch return;
 
         while (it.next()) |return_address| {
-            if (it.getLastError()) |unwind_error|
-                printUnwindError(debug_info, stderr, unwind_error.address, unwind_error.err, tty_config) catch {};
+            printLastUnwindError(&it, debug_info, stderr, tty_config);
 
             // On arm64 macOS, the address of the last frame is 0x0 rather than 0x1 as on x86_64 macOS,
             // therefore, we do a check for `return_address == 0` before subtracting 1 from it to avoid
@@ -252,7 +251,7 @@ pub fn dumpStackTraceFromBase(context: *const ThreadContext) void {
             // same behaviour for x86-windows-msvc
             const address = if (return_address == 0) return_address else return_address - 1;
             printSourceAtAddress(debug_info, stderr, address, tty_config) catch return;
-        }
+        } else printLastUnwindError(&it, debug_info, stderr, tty_config);
     }
 }
 
@@ -731,8 +730,7 @@ pub fn writeCurrentStackTrace(
     defer it.deinit();
 
     while (it.next()) |return_address| {
-        if (it.getLastError()) |unwind_error|
-            try printUnwindError(debug_info, out_stream, unwind_error.address, unwind_error.err, tty_config);
+        printLastUnwindError(&it, debug_info, out_stream, tty_config);
 
         // On arm64 macOS, the address of the last frame is 0x0 rather than 0x1 as on x86_64 macOS,
         // therefore, we do a check for `return_address == 0` before subtracting 1 from it to avoid
@@ -741,7 +739,7 @@ pub fn writeCurrentStackTrace(
         // same behaviour for x86-windows-msvc
         const address = if (return_address == 0) return_address else return_address - 1;
         try printSourceAtAddress(debug_info, out_stream, address, tty_config);
-    }
+    } else printLastUnwindError(&it, debug_info, out_stream, tty_config);
 }
 
 pub noinline fn walkStackWindows(addresses: []usize, existing_context: ?*const windows.CONTEXT) usize {
@@ -879,10 +877,21 @@ fn printUnknownSource(debug_info: *DebugInfo, out_stream: anytype, address: usiz
     );
 }
 
-pub fn printUnwindError(debug_info: *DebugInfo, out_stream: anytype, address: usize, err: UnwindError, tty_config: io.tty.Config) !void {
+fn printLastUnwindError(it: *StackIterator, debug_info: *DebugInfo, out_stream: anytype, tty_config: io.tty.Config) void {
+    if (!have_ucontext) return;
+    if (it.getLastError()) |unwind_error| {
+        printUnwindError(debug_info, out_stream, unwind_error.address, unwind_error.err, tty_config) catch {};
+    }
+}
+
+fn printUnwindError(debug_info: *DebugInfo, out_stream: anytype, address: usize, err: UnwindError, tty_config: io.tty.Config) !void {
     const module_name = debug_info.getModuleNameForAddress(address) orelse "???";
     try tty_config.setColor(out_stream, .dim);
-    try out_stream.print("Unwind information for `{s}:0x{x}` was not available ({}), trace may be incomplete\n\n", .{ module_name, address, err });
+    if (err == error.MissingDebugInfo) {
+        try out_stream.print("Unwind information for `{s}:0x{x}` was not available, trace may be incomplete\n\n", .{ module_name, address });
+    } else {
+        try out_stream.print("Unwind error at address `{s}:0x{x}` ({}), trace may be incomplete\n\n", .{ module_name, address, err });
+    }
     try tty_config.setColor(out_stream, .reset);
 }
 

lib/std/dwarf.zig

@@ -1656,7 +1656,7 @@ pub const DwarfInfo = struct {
     /// `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 unwindFrame(di: *const DwarfInfo, context: *UnwindContext, explicit_fde_offset: ?usize) !usize {
-        if (!comptime abi.isSupportedArch(builtin.target.cpu.arch)) return error.UnsupportedCpuArchitecture;
+        if (!comptime abi.supportsUnwinding(builtin.target)) return error.UnsupportedCpuArchitecture;
         if (context.pc == 0) return 0;
 
         // Find the FDE and CIE

lib/std/dwarf/abi.zig

@@ -3,13 +3,24 @@ const std = @import("../std.zig");
 const os = std.os;
 const mem = std.mem;
 
-pub fn isSupportedArch(arch: std.Target.Cpu.Arch) bool {
-    return switch (arch) {
-        .x86,
-        .x86_64,
-        .arm,
-        .aarch64,
-        => true,
+pub fn supportsUnwinding(target: std.Target) bool {
+    return switch (target.cpu.arch) {
+        .x86 => switch (target.os.tag) {
+            .linux, .netbsd, .solaris => true,
+            else => false,
+        },
+        .x86_64 => switch (target.os.tag) {
+            .linux, .netbsd, .freebsd, .openbsd, .macos, .solaris => true,
+            else => false,
+        },
+        .arm => switch (target.os.tag) {
+            .linux => true,
+            else => false,
+        },
+        .aarch64 => switch (target.os.tag) {
+            .linux, .netbsd, .freebsd, .macos => true,
+            else => false,
+        },
         else => false,
     };
 }

src/Compilation.zig

@@ -5491,6 +5491,7 @@ fn buildOutputFromZig(
         .omit_frame_pointer = comp.bin_file.options.omit_frame_pointer,
         .want_valgrind = false,
         .want_tsan = false,
+        .want_unwind_tables = comp.bin_file.options.eh_frame_hdr,
         .want_pic = comp.bin_file.options.pic,
         .want_pie = comp.bin_file.options.pie,
         .emit_h = null,
@@ -5639,9 +5640,5 @@ pub fn compilerRtOptMode(comp: Compilation) std.builtin.Mode {
 /// This decides whether to strip debug info for all zig-provided libraries, including
 /// compiler-rt, libcxx, libc, libunwind, etc.
 pub fn compilerRtStrip(comp: Compilation) bool {
-    if (comp.debug_compiler_runtime_libs) {
-        return comp.bin_file.options.strip;
-    } else {
-        return true;
-    }
+    return comp.bin_file.options.strip;
 }

src/target.zig

@@ -510,7 +510,7 @@ pub fn clangAssemblerSupportsMcpuArg(target: std.Target) bool {
 }
 
 pub fn needUnwindTables(target: std.Target) bool {
-    return target.os.tag == .windows or target.isDarwin();
+    return target.os.tag == .windows or target.isDarwin() or std.dwarf.abi.supportsUnwinding(target);
 }
 
 pub fn defaultAddressSpace(

test/standalone.zig

@@ -241,6 +241,10 @@ pub const build_cases = [_]BuildCase{
         .build_root = "test/standalone/coff_dwarf",
         .import = @import("standalone/coff_dwarf/build.zig"),
     },
+    .{
+        .build_root = "test/standalone/compiler_rt_panic",
+        .import = @import("standalone/compiler_rt_panic/build.zig"),
+    },
 };
 
 const std = @import("std");

test/standalone/compiler_rt_panic/build.zig

@@ -0,0 +1,26 @@
+const std = @import("std");
+
+pub fn build(b: *std.Build) void {
+    const test_step = b.step("test", "Test it");
+    b.default_step = test_step;
+
+    const target = b.standardTargetOptions(.{});
+    const optimize = b.standardOptimizeOption(.{});
+
+    if (target.getObjectFormat() != .elf) return;
+
+    const exe = b.addExecutable(.{
+        .name = "main",
+        .optimize = optimize,
+        .target = target,
+    });
+    exe.addCSourceFile("main.c", &.{});
+    exe.link_gc_sections = false;
+    exe.bundle_compiler_rt = true;
+
+    // Verify compiler_rt hasn't pulled in any debug handlers
+    const check_exe = exe.checkObject();
+    check_exe.checkInSymtab();
+    check_exe.checkNotPresent("debug.readElfDebugInfo");
+    test_step.dependOn(&check_exe.step);
+}

test/standalone/compiler_rt_panic/main.c

@@ -0,0 +1,11 @@
+#include <stddef.h>
+
+void* __memset(void* dest, char c, size_t n, size_t dest_n);
+
+char foo[128];
+
+int main() {
+    __memset(&foo[0], 0xff, 128, 128);
+    return foo[64];
+}
+