Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exporting bigint slices panics #3461

Closed
daxpedda opened this issue Jun 5, 2023 · 3 comments · Fixed by #3463
Closed

Exporting bigint slices panics #3461

daxpedda opened this issue Jun 5, 2023 · 3 comments · Fixed by #3463
Labels

Comments

@daxpedda
Copy link
Collaborator

daxpedda commented Jun 5, 2023

As of Rust v1.70, exporting a function like this:

#[wasm_bindgen]
pub fn test(_: &[u64]) {}

... will make it panic when called from JS:

test([]);
panicked at 'misaligned pointer dereference: address must be a multiple of 0x8 but is 0x4'

Here is a minimal reproducible example:
https://github.com/daxpedda/wasm-bindgen-bigint-slice-issue

I confirmed the following scenarios:

  • Only u64 and i64 integer types cause this issue.
  • Rust v1.69 does not have this issue.
  • The recent Rust changelog doesn't note any significant changes other then the upgrade to LLVM 16, which added default target features to Wasm:
    • Compiling with -Ctarget-feature=-sign-ext,-mutable-globals and -Zbuild-std on nightly-2023-06-03 does not fix the issue.
    • Compiling with -Ctarget-feature=+sign-ext,+mutable-globals on Rust v1.69 does not cause this issue.

This issue was originally discovered here: https://github.com/rustwasm/wasm-bindgen/actions/runs/5176047134/jobs/9324382988#step:7:409.

Update: apparently the bug is coming from rust-lang/rust#98112.
It's possible to circumvent the check by just disabling debug_asserts.

@daxpedda daxpedda added the bug label Jun 5, 2023
@daxpedda daxpedda mentioned this issue Jun 5, 2023
@ranile
Copy link
Collaborator

ranile commented Jun 5, 2023

Can you reproduce this error? It would be good to file an issue for Rust so the regression can be addressed.

@daxpedda
Copy link
Collaborator Author

daxpedda commented Jun 5, 2023

It's pretty easy to reproduce, I made a whole example here: https://github.com/daxpedda/wasm-bindgen-bigint-slice-issue.

I don't think it's a Rust issue, but it definitely goes over my head. I don't know enough about the Rust/wasm-bindgen ABI surface to understand what's going on here let alone what needs to be done.

@Liamolucko
Copy link
Collaborator

It looks like this is happening because __wbindgen_malloc, the function that all the JS glue uses to allocate memory, always aligns everything to the alignment of a usize (so, 4 bytes):

wasm-bindgen/src/lib.rs

Lines 1571 to 1588 in 5453e33

#[no_mangle]
pub extern "C" fn __wbindgen_malloc(size: usize) -> *mut u8 {
let align = mem::align_of::<usize>();
if let Ok(layout) = Layout::from_size_align(size, align) {
unsafe {
if layout.size() > 0 {
let ptr = alloc(layout);
if !ptr.is_null() {
return ptr
}
} else {
return align as *mut u8
}
}
}
malloc_failure();
}

The best fix is probably to add an align argument to __wbindgen_malloc (and __wbindgen_realloc and __wbindgen_free) instead of hard-coding it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants