John Murray (Nov 21 2023 at 02:55): John Murray (Nov 21 2023 at 02:55):Im working on a fix for https://github.com/roc-lang/roc/issues/6027 and I think i traced the issue down to this line https://github.com/roc-lang/roc/blob/ad5ed57c4202f847cf9e215cda970c6ece22f9ff/crates/compiler/builtins/bitcode/src/dec.zig#L372
if i switch it to roc_panic the llvm and wasm backends seem to be erroring as expected, without my change they return non-sense numbers for div by 0
Based on this message it seems like all @panic in zig should be roc_panic calls.
Is that still the case? If So i can update them all on my pr
side question: does that fix make sense for the og issue?
Brendan Hansknecht (Nov 21 2023 at 02:56):Yeah, that sounds accurate.
Brendan Hansknecht (Nov 21 2023 at 02:57):as for replacing them all, maybe... My thought is that generally zig @panic should be used if we have a todo while roc_panic should be used if it is something intentional to expose.
Brendan Hansknecht (Nov 21 2023 at 02:58):But that may not be correct thinking. Making them all roc_panicis probably fine.
Brendan Hansknecht (Nov 21 2023 at 02:59):oh, richard already said they should all be roc_panic. Ok, cool, yeah, change them all!
John Murray (Nov 21 2023 at 03:00):dope will do!
John Murray (Nov 22 2023 at 03:52):while doing some testing i came across this potentially confusing output from the repl
so looks like in the repl when theres a panic or a call to crash we still display some "normal" output
is this expected?
Something similar happens with calling crash from roc
2023-11-21-22-51-28.png
Brendan Hansknecht (Nov 22 2023 at 04:32):Interesting.
Brendan Hansknecht (Nov 22 2023 at 04:32):Definitely a bug
John Murray (Nov 22 2023 at 22:35):Forgot to respond, will filed an issue for that https://github.com/roc-lang/roc/issues/6066
John Murray (Nov 27 2023 at 23:56):Does anyone have any insights to the failing tests on this pr? https://github.com/roc-lang/roc/pull/6062
On the apple silcon test i see
---- cli_run::inspect_gui stdout ----
thread 'cli_run::inspect_gui' panicked at '
___________
The roc command:
"/Users/m1ci/actions-runner2/_work/roc/roc/target/release/roc build --optimize /Users/m1ci/actions-runner2/_work/roc/roc/examples/inspect-gui.roc --"
had unexpected stderr:
🔨 Rebuilding platform...
ld: malformed nlist string offset file '/Users/m1ci/actions-runner2/_work/roc/roc/examples/gui/platform/macos-arm64.o'
/Users/m1ci/actions-runner2/_work/roc/roc/examples/inspect-gui: No such file or directory
thread 'main' panicked at 'not yet implemented: gracefully handle `ld` (or `zig` in the case of wasm with --optimize) returning exit code Some(1)', crates/compiler/build/src/program.rs:1089:17
stack backtrace:
0: rust_begin_unwind
at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs:593:5
1: core::panicking::panic_fmt
at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/panicking.rs:67:14
2: roc_build::program::build_loaded_file
3: roc_build::program::build_file
4: roc_cli::build
5: roc::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
the other is
---- list_concat_empty_list_zero_sized_type stdout ----
thread 'list_concat_empty_list_zero_sized_type' panicked at '`valgrind` exited with exit code 1. valgrind stdout was: ""
valgrind stderr was: "--1766045-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting
--1766045-- si_code=1; Faulting address: 0x251000; sp: 0x1002f7d4c0
valgrind: the 'impossible' happened:
Killed by fatal signal
host stacktrace:
==1766045== at 0x581B3211: getUChar (guest_amd64_toIR.c:524)
==1766045== by 0x581B3211: dis_ESC_NONE.isra.0 (guest_amd64_toIR.c:19967)
==1766045== by 0x581E3A09: disInstr_AMD64_WRK (guest_amd64_toIR.c:32515)
==1766045== by 0x581E41E3: disInstr_AMD64 (guest_amd64_toIR.c:32683)
==1766045== by 0x58166439: disassemble_basic_block_till_stop (guest_generic_bb_to_IR.c:956)
==1766045== by 0x5816748C: bb_to_IR (guest_generic_bb_to_IR.c:1365)
==1766045== by 0x58148834: LibVEX_FrontEnd (main_main.c:583)
==1766045== by 0x581491A6: LibVEX_Translate (main_main.c:1235)
==1766045== by 0x5806384F: vgPlain_translate (m_translate.c:1831)
==1766045== by 0x580AEBBF: handle_tt_miss (scheduler.c:1136)
==1766045== by 0x580AEBBF: vgPlain_scheduler (scheduler.c:1526)
==1766045== by 0x580F9959: thread_wrapper (syswrap-linux.c:102)
==1766045== by 0x580F9959: run_a_thread_NORETURN (syswrap-linux.c:155)
sched status:
running_tid=1
Thread 1: status = VgTs_Runnable (lwpid 1766045)
client stack range: [0x1FFEFFD000 0x1FFF000FFF] client SP: 0x1FFEFFF090
valgrind stack range: [0x1002E7E000 0x1002F7DFFF] top usage: 18664 of 1048576
Both I cant seem to fail on either of my machines (both nixos x64)
Do i need to change the calls to roc_panic to have the second param be non zero? Not sure what that does tbh
John Murray (Nov 28 2023 at 01:49):Did a bit of a wild guess and figured it might be related to me doing a renmae for some existing panics, undid that to only affect "new" code
Brendan Hansknecht (Nov 28 2023 at 04:21):It shouldnt be related to the second param
Brendan Hansknecht (Nov 28 2023 at 04:21):That is just a selector essentially
Brendan Hansknecht (Nov 28 2023 at 04:21):0 is std lib crash
Brendan Hansknecht (Nov 28 2023 at 04:21):1 is userland crash
Brendan Hansknecht (Nov 28 2023 at 04:21):At least IIRC
John Murray (Nov 28 2023 at 04:23):did a bit of digging, i added some unreachable calls after roc_panic since zig didnt know that it will end there.
Looks like unreachable is undefiened behavior when compiled with RelaseFast https://ziglang.org/documentation/master/#try which i think we do
I then just noticed our custom panic handler uses unreachable and does not actually do anything in non test
// Custom panic function, as builtin Zig version errors during LLVM verification
pub fn panic(message: []const u8, stacktrace: ?*std.builtin.StackTrace, _: ?usize) noreturn {
if (builtin.is_test) {
std.debug.print("{s}: {?}", .{ message, stacktrace });
}
unreachable;
}
That comment is 2 years old, ill see if can get that to call out to roc_panic and maybe of this will just work
Brendan Hansknecht (Nov 28 2023 at 04:25):So that version should only ever be called in some tests. Most of the time roc_panic will be a call to a real platform
Brendan Hansknecht (Nov 28 2023 at 04:26):inspect_gui shouldn't be using it. The other test will be though I'm pretty sure.
Brendan Hansknecht (Nov 28 2023 at 04:27):Hmm
.though why is our panic definition just unreachable....I think it should instead exit or call zigs panic during tests or something. Just calling unreachable seems weird, but maybe that is reasonable for tests
John Murray (Nov 28 2023 at 04:31):O it seems me reverting some of renames that were unrelated to this branch fixed the inspect_gui issue, now its just the valgrind error for list_concat_empty_list_zero_sized_type
Brendan Hansknecht (Nov 28 2023 at 04:41):That was an m1 specific error?
John Murray (Nov 28 2023 at 04:45):Somehow...
I realized that the llvm backend equilvants had the panic strings in the lowercased form as well so some of the tests were actually wrong but tbh not sure how running cargo test locally didnt catch it, could be some weirdness with default features.
so maybe cargo test just got in a really weird state but that doesnt make a lot of sense either...
Brendan Hansknecht (Nov 28 2023 at 04:48):hmm, looks to be an x86_64 linux test. Valgrind doesn't run on m1 macs
John Murray (Nov 28 2023 at 04:48):o sorry the inspect_gui error was the error only on m1
Brendan Hansknecht (Nov 28 2023 at 04:49):ah, ok and that is fixed now...got it. That definitely seems like it probably was a flake to me.
Brendan Hansknecht (Nov 28 2023 at 04:51):Can't repro the valgrind failure locally....sad
John Murray (Nov 28 2023 at 04:53):atleast im not crazy lol
My gut is the unreachable undefined behavior, ill see if theres another way to tell zig that roc_panic's return is actuall noreturn
actually now that i think of it, i wonder if just updating
extern fn roc_panic(msg: *const RocStr, tag_id: u32) callconv(.C) void;
to
extern fn roc_panic(msg: *const RocStr, tag_id: u32) callconv(.C) noreturn;
works?
John Murray (Nov 28 2023 at 04:57):o zig seems to compile at least with that.. Ill push a commit with all my unreachables removed and that change
Brendan Hansknecht (Nov 28 2023 at 04:58):That makes a lot of sense. Hopefully a fix
John Murray (Nov 28 2023 at 05:05):pushed up a commit, can you kick off the ci when you get a sec?
John Murray (Nov 28 2023 at 05:22):It made it past that test :party_ball:
Brendan Hansknecht (Nov 28 2023 at 05:23):Awesome :tada:
Anton (Nov 28 2023 at 09:55):I remember we hit some valgrind: the 'impossible' happened before and I gave up on it, awesome work @John Murray!
Last updated: Jul 06 2025 at 12:14 UTC