str_to_u128 segfault · compiler development

Stream: compiler development

Topic: str_to_u128 segfault

Anton (Apr 16 2024 at 18:10):

After upgrading to a newer rust version (PR#6644) gen_str::str_to_u128 is now segfaulting on x64 linux. It doesn't fail on macos x64, there it gets a wrong answer:

LLVM test failed
  left: RocOk(ManuallyDrop { value: 1 })
 right: RocOk(ManuallyDrop { value: 2593737180395252978950973914349568 })

These also go wrong on macos x64:

SIGSEGV [   1.594s] test_gen::test_gen gen_num::add_checked_u128
SIGSEGV [   1.560s] test_gen::test_gen gen_num::mul_checked_u128
SIGSEGV [   1.489s] test_gen::test_gen gen_num::sub_checked_u128
SIGABRT [   1.294s] test_gen::test_gen gen_str::str_to_i128

I've got the llvm IR but the roc_builtins.str.to_int.u128 function is pretty big.

The valgrind errors are:

Use of uninitialised value of size 8
Invalid read of size 8
Address 0x1 is not stack'd, malloc'd or (recently) free'd
Process terminating with default action of signal 11 (SIGSEGV): dumping core
Access not within mapped region at address 0x1

I plan on stepping though the macos assembly tomorrow. Any other suggestions/tips?

Folkert de Vries (Apr 16 2024 at 18:16):

did u128 alignment update already?

Brendan Hansknecht (Apr 16 2024 at 18:51):

Oh yeah, I think rust was fixing it on frontend first.

Brendan Hansknecht (Apr 16 2024 at 18:51):

With some backend workaround

Brendan Hansknecht (Apr 16 2024 at 18:51):

Then when llvm is fixed update to that

Brendan Hansknecht (Apr 16 2024 at 18:51):

At least if I recall correctly, so that would make sense

Anton (Apr 17 2024 at 13:40):

Alignment is still the same, I checked with std::mem::align_of::<u128>() it will change with 1.78.

Brendan Hansknecht (Apr 17 2024 at 14:19):

Is our internal U128 the same size and alignment still?

Anton (Apr 17 2024 at 14:58):

Rust 1.75 is the first one to segfault on str_to_u128 but there is no difference compared to the LLVM IR using rust 1.74

gdb is not working well with the dynamic invocation in try_run_lib_function, do we have an easy way to turn that into a standalone binary? I also tried Str.toU128 with several platforms but none of them segfaulted.

Anton (Apr 19 2024 at 15:45):

Just including a dbg here fixes the issue :thinking:

#[allow(dead_code)]
pub fn try_run_lib_function<T>(
    main_fn_name: &str,
    lib: &libloading::Library,
) -> Result<T, (String, CrashTag)> {
    dbg!(main_fn_name);
    unsafe {
        let main: libloading::Symbol<unsafe extern "C" fn(*mut RocCallResult<T>)> = lib
            .get(main_fn_name.as_bytes())
            .ok()
            .ok_or(format!("Unable to JIT compile `{main_fn_name}`"))
            .expect("errored");

        let mut main_result = MaybeUninit::uninit();
        main(main_result.as_mut_ptr());

        main_result.assume_init().into()
    }
}

Anton (Apr 19 2024 at 16:13):

Is our internal U128 the same size and alignment still?

How can I check that @Brendan Hansknecht?

Brendan Hansknecht (Apr 19 2024 at 17:10):

Should be the same way you checked u128. std::mem::align_of and size_of. Just in the U128 defined in roc_std

Anton (Apr 19 2024 at 18:03):

They alignment of U128 has not changed but I do think I found the bug:

#[test]
#[cfg(feature = "gen-llvm")]
fn str_to_u128() {
    assert_evals_to!(
        indoc!(
            r#"
            Str.toU128 "1"
            "#
        ),
        RocResult::ok(1),
        RocResult<u128, ()>
    );
}

I believe RocResult<u128, ()> should be RocResult<roc_std::U128, ()>.
Now it also makes sense why it failed only in the gen tests and nowhere else.