Skip to content

Commit 9760068

Browse files
committed
std.debug: prefer FP unwinding on targets where it is ideal
If the ABI requires a backchain pointer, FP unwinding is always possible, safe, and fast, so there's really no reason not to use it.
1 parent d280061 commit 9760068

File tree

1 file changed

+37
-26
lines changed

1 file changed

+37
-26
lines changed

lib/std/debug.zig

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,10 @@ pub fn writeCurrentStackTrace(options: StackUnwindOptions, writer: *Writer, tty_
697697
var printed_any_frame = false;
698698
while (true) switch (it.next()) {
699699
.switch_to_fp => |unwind_error| {
700-
if (StackIterator.abi_requires_backchain or StackIterator.fp_unwind_is_safe) continue; // no need to even warn
700+
switch (StackIterator.fp_usability) {
701+
.useless, .unsafe => {},
702+
.safe, .ideal => continue, // no need to even warn
703+
}
701704
const module_name = di.getModuleName(di_gpa, unwind_error.address) catch "???";
702705
const caption: []const u8 = switch (unwind_error.err) {
703706
error.MissingDebugInfo => "unwind info unavailable",
@@ -840,7 +843,8 @@ const StackIterator = union(enum) {
840843
if (!(builtin.zig_backend == .stage2_c and builtin.target.abi == .msvc) and
841844
SelfInfo != void and
842845
SelfInfo.can_unwind and
843-
cpu_context.Native != noreturn)
846+
cpu_context.Native != noreturn and
847+
fp_usability != .ideal)
844848
{
845849
// We don't need `di_first` here, because our PC is in `std.debug`; we're only interested
846850
// in our caller's frame and above.
@@ -855,32 +859,41 @@ const StackIterator = union(enum) {
855859
}
856860
}
857861

858-
/// Some architectures make FP unwinding too impractical. For example, due to its very silly ABI
859-
/// design decisions, it's not possible to do generic FP unwinding on MIPS; we would need to do
860-
/// a complicated code scanning algorithm instead. At that point, we may as well just use DWARF.
861-
const fp_unwind_is_impossible = switch (builtin.cpu.arch) {
862+
const FpUsability = enum {
863+
/// FP unwinding is impractical on this target. For example, due to its very silly ABI
864+
/// design decisions, it's not possible to do generic FP unwinding on MIPS without a
865+
/// complicated code scanning algorithm.
866+
useless,
867+
/// FP unwinding is unsafe on this target; we may crash when doing so. We will only perform
868+
/// FP unwinding in the case of crashes/panics, or if the user opts in.
869+
unsafe,
870+
/// FP unwinding is guaranteed to be safe on this target. We will do so if unwinding with
871+
/// debug info does not work, and if this compilation has frame pointers enabled.
872+
safe,
873+
/// FP unwinding is the best option on this target. This is usually because the ABI requires
874+
/// a backchain pointer, thus making it always available, safe, and fast.
875+
ideal,
876+
};
877+
878+
const fp_usability: FpUsability = switch (builtin.target.cpu.arch) {
862879
.mips,
863880
.mipsel,
864881
.mips64,
865882
.mips64el,
866-
=> true,
867-
else => false,
868-
};
869-
870-
/// <https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Respect-the-purpose-of-specific-CPU-registers>
871-
const fp_unwind_is_safe = builtin.cpu.arch == .aarch64 and builtin.os.tag.isDarwin();
872-
873-
/// On some architectures, we can do FP unwinding even with `-fomit-frame-pointer` due to ABI
874-
/// requirements. Typically this is because the ABI requires a backchain regardless of whether
875-
/// codegen actually uses a frame pointer for stack frame access.
876-
const abi_requires_backchain = switch (builtin.cpu.arch) {
883+
=> .useless,
877884
.hexagon,
885+
// The PowerPC ABIs don't actually strictly require a backchain pointer; they allow omitting
886+
// it when full unwind info is present. Despite this, both GCC and Clang always enforce the
887+
// presence of the backchain pointer no matter what options they are given. This seems to be
888+
// a case of "the spec is only a polite suggestion", except it works in our favor this time!
878889
.powerpc,
879890
.powerpcle,
880891
.powerpc64,
881892
.powerpc64le,
882-
=> true,
883-
else => false,
893+
=> .ideal,
894+
// https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Respect-the-purpose-of-specific-CPU-registers
895+
.aarch64 => if (builtin.target.os.tag.isDarwin()) .safe else .unsafe,
896+
else => .unsafe,
884897
};
885898

886899
/// Whether the current unwind strategy is allowed given `allow_unsafe`.
@@ -891,13 +904,11 @@ const StackIterator = union(enum) {
891904
// immediately regardless of anything. But FPs could also be omitted from a different
892905
// linked object, so it's not guaranteed to be safe, unless the target specifically
893906
// requires it.
894-
.fp => s: {
895-
if (fp_unwind_is_impossible) break :s false;
896-
if (abi_requires_backchain) break :s true;
897-
if (builtin.omit_frame_pointer) break :s false;
898-
if (fp_unwind_is_safe) break :s true;
899-
if (allow_unsafe) break :s true;
900-
break :s false;
907+
.fp => switch (fp_usability) {
908+
.useless => false,
909+
.unsafe => allow_unsafe and !builtin.omit_frame_pointer,
910+
.safe => !builtin.omit_frame_pointer,
911+
.ideal => true,
901912
},
902913
};
903914
}

0 commit comments

Comments
 (0)