safety: show error return trace when unwrapping error in switch

[Vexu]

A small quality-of-life improvement I thought of while trying to figure out where an error.GenericPoison was coming from.

Example situation:

fn foo() !void {
    return error.Foo;
}
fn bar() !void {
    try foo();
}
fn baz() void {
    // bar() catch unreachable;
    // bar() catch @panic("foo");
    // const S = struct {
    //     const S = struct {
    //         const fooo = "foo";
    //     };
    // };
    bar() catch |err| switch (err) {
        // error.Foo => @panic(S.S.fooo),
        error.Foo => unreachable,
    };
}
test {
    baz();
}

before:

Test [1/1] test_0... thread 18432 panic: reached unreachable code
./a.zig:17:22: 0x20f0fc in baz (test)
        error.Foo => unreachable,
                     ^
./a.zig:21:8: 0x20f078 in test_0 (test)
    baz();
       ^
/home/vexu/Documents/zig/zig/lib/test_runner.zig:79:28: 0x2105b5 in main (test)
        } else test_fn.func();
                           ^
/home/vexu/Documents/zig/zig/lib/std/start.zig:568:22: 0x20f9bd in posixCallMainAndExit (test)
            root.main();
                     ^
/home/vexu/Documents/zig/zig/lib/std/start.zig:340:5: 0x20f3c2 in _start (test)
    @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{});
    ^
error: the following test command crashed:

after:

Test [1/1] test_0... thread 18500 panic: attempt to unwrap error: Foo
./a.zig:2:5: 0x20f998 in foo (test)
    return error.Foo;
    ^
./a.zig:5:5: 0x20fa83 in bar (test)
    try foo();
    ^
/home/vexu/Documents/zig/zig/lib/std/debug.zig:311:22: 0x210b34 in panicExtra__anon_2885 (test)
    std.builtin.panic(msg, trace);
                     ^
/home/vexu/Documents/zig/zig/lib/std/builtin.zig:864:25: 0x20fad3 in panicUnwrapError (test)
    std.debug.panicExtra(st, "attempt to unwrap error: {s}", .{@errorName(err)});
                        ^
./a.zig:17:22: 0x20f097 in baz (test)
        error.Foo => unreachable,
                     ^
./a.zig:21:8: 0x20f018 in test_0 (test)
    baz();
       ^
/home/vexu/Documents/zig/zig/lib/test_runner.zig:79:28: 0x210595 in main (test)
        } else test_fn.func();
                           ^
/home/vexu/Documents/zig/zig/lib/std/start.zig:568:22: 0x20f95d in posixCallMainAndExit (test)
            root.main();
                     ^
/home/vexu/Documents/zig/zig/lib/std/start.zig:340:5: 0x20f362 in _start (test)
    @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{});
    ^
error: the following test command crashed:

This also makes @panic show error return trace in these same situations which you can test by uncommenting lines in the example.

[andrewrk]

Looks great. It would be nice to get rid of these frames from the stack trace too:

/home/vexu/Documents/zig/zig/lib/std/debug.zig:311:22: 0x210b34 in panicExtra__anon_2885 (test)
    std.builtin.panic(msg, trace);
                     ^
/home/vexu/Documents/zig/zig/lib/std/builtin.zig:864:25: 0x20fad3 in panicUnwrapError (test)
    std.debug.panicExtra(st, "attempt to unwrap error: {s}", .{@errorName(err)});
                        ^

[andrewrk]

I rebased this against master branch, updated the CI tarballs for windows and macos, and force pushed.

[Vexu]

I suggest instead of inline calls, to have these functions use @returnAddress() and then pass this forward into the panic system so that this value ends up passed to panicImpl as the first_trace_addr parameter. Maybe this means we have to make a breaking change to the panic function signature to add this parameter. If that is the case, I'm fine with it.

That would require changing PanicFn from fn ([]const u8, ?*StackTrace) noreturn to fn ([]const u8, ?*StackTrace, ?usize) noreturn but I also think that's an improvement.

As a side note, I'm a little uncomfortable with how much std lib formatting is being dragged in by builtin panic helper functions, and by the fact that overriding the panic handler still results in a bunch of formatting stuff being outputted. But, I don't have any concrete suggestions for improvement on that, and it's not relevant to this PR, and I think that the helpful debugging messages are worth it in the end.

How about a @import("root").simple_panics option?

[andrewrk]

That would require changing PanicFn from fn ([]const u8, ?*StackTrace) noreturn to fn ([]const u8, ?*StackTrace, ?usize) noreturn but I also think that's an improvement.

Let's do it.

How about a @import("root").simple_panics option?

Worth considering. Now that you mention it, I can think of a couple other options worth considering as well. Sounds like we should discuss in a separate proposal that does not block this PR, yeah?

[Vexu]

Call parameter type does not match function signature!
%"?usize" { i64 undef, i1 false }
 ptr  call fastcc void @std.builtin.default_panic(ptr @11218, ptr null, %"?usize" { i64 undef, i1 false }), !dbg !1431870

Any ideas to what could be causing this? I don't think panic is being called in any other place in stage1 and my change there seems correct enough to build stage1?

[andrewrk]

Can I see the corresponding function signature from the LLVM module?

[Vexu]

How do I pass -femit-llvm-ir to CMake?

[andrewrk]

-femit-llvm-ir gives you the LLVM IR after optimization passes (even in debug builds there is still some processing). In this case --verbose-llvm-ir is the more helpful flag.

My suggestion is to invoke make like this: make VERBOSE=1 which will make it print the commands it runs before it runs them, then you can copy paste the command, adding --verbose-llvm-ir 2>stderr.txt to the end.