Skip to content

Conversation

mlugg
Copy link
Member

@mlugg mlugg commented Oct 9, 2025

I broke this when porting this logic for the std.debug rework in #25227. The offset that I copied was actually being treated as relative to the address of the saved base pointer. I think it makes more sense to do what I did and just treat all offsets as relative to this frame's base.

I broke this when porting this logic for the `std.debug` rework in
ziglang#25227. The offset that I copied was
actually being treated as relative to the address of the *saved* base
pointer. I think it makes more sense to do what I did and just treat all
offsets as relative to this frame's base.
@mlugg mlugg requested a review from alexrp October 9, 2025 13:19
Copy link
Member

@alexrp alexrp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that the RISC-V change works, but the SPARC change doesn't seem to make a difference on my end:

❯ zig4 build-exe crash.zig -target sparc64-linux && qemu-sparc64 ./crash
Cannot print stack trace: safe unwind unavailable for target
Segmentation fault at address 0x4
fish: Job 1, 'qemu-sparc64 ./crash' terminated by signal SIGABRT (Abort)
❯ cat crash.zig
const std = @import("std");
pub fn main() void {
    foo();
    // @panic("oh no");
    @as(*u32, @ptrFromInt(4)).* = 42;
}

noinline fn foo() void {
    std.debug.dumpCurrentStackTrace(.{});
}

@mlugg
Copy link
Member Author

mlugg commented Oct 9, 2025

I'll be honest, I didn't test the SPARC one; I was just trying to mirror the old logic on the assumption that it was more correct.

Was SPARC tracing working prior to my std.debug PR (i.e. did I regress it)? If so, I'll take a look now, but if not, I think this should be merged as an improvement.

@alexrp
Copy link
Member

alexrp commented Oct 9, 2025

Actually no, it doesn't seem like it; there's no stack crash stack trace on 0.15.1 either. Could've sworn I managed to make it produce one previously though...?

In any case, seems fine.

@mlugg
Copy link
Member Author

mlugg commented Oct 9, 2025

Interesting... I might take a look at SPARC in a bit. How hard could it be?! :):):):)

@mlugg mlugg enabled auto-merge (rebase) October 9, 2025 17:42
@mlugg mlugg merged commit 80f6b8c into ziglang:master Oct 9, 2025
9 checks passed
@mlugg mlugg deleted the riscv-sparc-fp-unwind branch October 10, 2025 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants