From 8e6a62ba10326e48eaefd40f89c9452d92f39c9d Mon Sep 17 00:00:00 2001 From: kcbanner Date: Wed, 19 Jul 2023 02:06:17 -0400 Subject: [PATCH] test: disable omit_frame_pointer unwinding tests on aarch64-macos dwarf: handle signal frame CIE flag --- lib/std/debug.zig | 2 +- lib/std/dwarf.zig | 15 +++++++++------ test/standalone/stack_iterator/build.zig | 13 ++++++------- .../stack_iterator/shared_lib_unwind.zig | 4 ++++ .../stack_iterator/{zig_unwind.zig => unwind.zig} | 3 +++ 5 files changed, 23 insertions(+), 14 deletions(-) rename test/standalone/stack_iterator/{zig_unwind.zig => unwind.zig} (94%) diff --git a/lib/std/debug.zig b/lib/std/debug.zig index 603d651e5c..178274f25f 100644 --- a/lib/std/debug.zig +++ b/lib/std/debug.zig @@ -882,7 +882,7 @@ 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 { const module_name = debug_info.getModuleNameForAddress(address) orelse "???"; try tty_config.setColor(out_stream, .dim); - try out_stream.print("Unwind information for `{s}:{}` was not available ({}), trace may be incomplete\n\n", .{ module_name, address, err }); + try out_stream.print("Unwind information for `{s}:0x{x}` was not available ({}), trace may be incomplete\n\n", .{ module_name, address, err }); try tty_config.setColor(out_stream, .reset); } diff --git a/lib/std/dwarf.zig b/lib/std/dwarf.zig index 968dab094d..8999763823 100644 --- a/lib/std/dwarf.zig +++ b/lib/std/dwarf.zig @@ -1659,8 +1659,6 @@ pub const DwarfInfo = struct { if (!comptime abi.isSupportedArch(builtin.target.cpu.arch)) return error.UnsupportedCpuArchitecture; if (context.pc == 0) return 0; - // TODO: Handle unwinding from a signal frame (ie. use_prev_instr in libunwind) - // Find the FDE and CIE var cie: CommonInformationEntry = undefined; var fde: FrameDescriptionEntry = undefined; @@ -1828,11 +1826,16 @@ pub const DwarfInfo = struct { (try abi.regValueNative(usize, context.thread_context, abi.ipRegNum(), context.reg_context)).* = context.pc; - // The call instruction will have pushed the address of the instruction that follows the call as the return address - // However, this return address may be past the end of the function if the caller was `noreturn`. By subtracting one, - // then `context.pc` will always point to an instruction within the FDE for the previous function. + // The call instruction will have pushed the address of the instruction that follows the call as the return address. + // This next instruction may be past the end of the function if the caller was `noreturn` (ie. the last instruction in + // the function was the call). If we were to look up an FDE entry using the return address directly, it could end up + // either not finding an FDE at all, or using the next FDE in the program, producing incorrect results. To prevent this, + // we subtract one so that the next lookup is guaranteed to land inside the + // + // The exception to this rule is signal frames, where we return execution would be returned to the instruction + // that triggered the handler. const return_address = context.pc; - if (context.pc > 0) context.pc -= 1; + if (context.pc > 0 and !cie.isSignalFrame()) context.pc -= 1; return return_address; } diff --git a/test/standalone/stack_iterator/build.zig b/test/standalone/stack_iterator/build.zig index f264777ae0..1c5a9673ce 100644 --- a/test/standalone/stack_iterator/build.zig +++ b/test/standalone/stack_iterator/build.zig @@ -7,7 +7,7 @@ pub fn build(b: *std.Build) void { const target = b.standardTargetOptions(.{}); const optimize = b.standardOptimizeOption(.{}); - // Unwinding pure zig code, with a frame pointer + // Unwinding with a frame pointer // // getcontext version: zig std // @@ -18,8 +18,8 @@ pub fn build(b: *std.Build) void { // - aarch64: FRAME, DWARF { const exe = b.addExecutable(.{ - .name = "zig_unwind_fp", - .root_source_file = .{ .path = "zig_unwind.zig" }, + .name = "unwind_fp", + .root_source_file = .{ .path = "unwind.zig" }, .target = target, .optimize = optimize, }); @@ -31,7 +31,7 @@ pub fn build(b: *std.Build) void { test_step.dependOn(&run_cmd.step); } - // Unwinding pure zig code, without a frame pointer. + // Unwinding without a frame pointer // // getcontext version: zig std // @@ -42,13 +42,12 @@ pub fn build(b: *std.Build) void { // - aarch64: FRAMELESS, DWARF { const exe = b.addExecutable(.{ - .name = "zig_unwind_nofp", - .root_source_file = .{ .path = "zig_unwind.zig" }, + .name = "unwind_nofp", + .root_source_file = .{ .path = "unwind.zig" }, .target = target, .optimize = optimize, }); - if (target.isDarwin()) exe.unwind_tables = true; exe.omit_frame_pointer = true; exe.unwind_tables = true; diff --git a/test/standalone/stack_iterator/shared_lib_unwind.zig b/test/standalone/stack_iterator/shared_lib_unwind.zig index 22f0081744..50e0421e2a 100644 --- a/test/standalone/stack_iterator/shared_lib_unwind.zig +++ b/test/standalone/stack_iterator/shared_lib_unwind.zig @@ -1,4 +1,5 @@ const std = @import("std"); +const builtin = @import("builtin"); const debug = std.debug; const testing = std.testing; @@ -34,6 +35,9 @@ extern fn frame0( ) void; pub fn main() !void { + // Disabled until the DWARF unwinder bugs on .aarch64 are solved + if (builtin.omit_frame_pointer and comptime builtin.target.isDarwin() and builtin.cpu.arch == .aarch64) return; + if (!std.debug.have_ucontext or !std.debug.have_getcontext) return; var expected: [5]usize = undefined; diff --git a/test/standalone/stack_iterator/zig_unwind.zig b/test/standalone/stack_iterator/unwind.zig similarity index 94% rename from test/standalone/stack_iterator/zig_unwind.zig rename to test/standalone/stack_iterator/unwind.zig index 5421ac052f..1280118173 100644 --- a/test/standalone/stack_iterator/zig_unwind.zig +++ b/test/standalone/stack_iterator/unwind.zig @@ -87,6 +87,9 @@ noinline fn frame0(expected: *[4]usize, unwound: *[4]usize) void { } pub fn main() !void { + // Disabled until the DWARF unwinder bugs on .aarch64 are solved + if (builtin.omit_frame_pointer and comptime builtin.target.isDarwin() and builtin.cpu.arch == .aarch64) return; + if (!std.debug.have_ucontext or !std.debug.have_getcontext) return; var expected: [4]usize = undefined;