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

Runtime error using f64::sin in wasm32 #128105

Closed
hexane360 opened this issue Jul 23, 2024 · 12 comments
Closed

Runtime error using f64::sin in wasm32 #128105

hexane360 opened this issue Jul 23, 2024 · 12 comments
Labels
C-gub Category: the reverse of a compiler bug is generally UB O-wasm Target: WASM (WebAssembly), http://webassembly.org/

Comments

@hexane360
Copy link

I'm working on a WASM library using wasm_bindgen which wraps ndarray. I have the following basic structure:

pub enum DynArray {
    Float32(ArrayD<f32>),
    Float64(ArrayD<f64>),
    ...
}

impl DynArray {
    pub fn sin(&self) -> DynArray {
        match self {
            DynArray::Float32(arr) => arr.map(|e| e.sin()).into(),
            DynArray::Float64(arr) => arr.map(|e| e.sin()).into(),
            ...
        }
    }
}

i.e. sin() is implemented by first dispatching on type and then mapping.
However, calling sin on a Float64 array results in the following runtime error:

RuntimeError: unreachable
    at wasm_array_bug.wasm.signature_mismatch:sin (5e6ff9aa49a8693eccd3.module.wasm:0x59426)
    at wasm_array_bug.wasm.std::f64::<impl f64>::sin::hb3cd0e4a298bd1b1 (5e6ff9aa49a8693eccd3.module.wasm:0x55d96)
    at wasm_array_bug.wasm.wasm_array_bug::DynArray::sin::{{closure}}::hc86147a10cddb9c5 (5e6ff9aa49a8693eccd3.module.wasm:0x560a6)
    at wasm_array_bug.wasm.ndarray::iterators::to_vec_mapped::{{closure}}::h0f8b6901051cb335 (5e6ff9aa49a8693eccd3.module.wasm:0x414f7)
    at wasm_array_bug.wasm.<core::slice::iter::Iter<T> as core::iter::traits::iterator::Iterator>::fold::h83c1cb924c78a7c1 (5e6ff9aa49a8693eccd3.module.wasm:0x31beb)
    at wasm_array_bug.wasm.ndarray::iterators::to_vec_mapped::h2d7cca19531985df (5e6ff9aa49a8693eccd3.module.wasm:0x2c54f)
    at wasm_array_bug.wasm.ndarray::impl_constructors::<impl ndarray::ArrayBase<S,D>>::from_shape_trusted_iter_unchecked::hd85ecc6b56c06e67 (5e6ff9aa49a8693eccd3.module.wasm:0x2b1db)
    at wasm_array_bug.wasm.ndarray::impl_methods::<impl ndarray::ArrayBase<S,D>>::map::h753b54be2654f42f (5e6ff9aa49a8693eccd3.module.wasm:0x35707)
    at wasm_array_bug.wasm.wasm_array_bug::DynArray::sin::h634584090a112859 (5e6ff9aa49a8693eccd3.module.wasm:0x3a986)
    at wasm_array_bug.wasm.wasm_array_bug::sin::h90417ccd7ffb0d96 (5e6ff9aa49a8693eccd3.module.wasm:0x5208f)

This error originates in the implementation of f64::sin().
Interestingly, I don't get the same error for f32. I also don't get the same error if I remove the enum & match:

#[wasm_bindgen]
pub struct Float64Array {
    inner: ArrayD<f64>
}

#[wasm_bindgen]
pub fn sin_float64(arr: &Float64Array) -> Float64Array {
    arr.inner.map(|e| e.sin()).into()
}

This makes me think there's likely some strange miscompilation. For reference, here's the f64::sin() function compiled in the broken case (with enum/match):

  (func $std::f64::<impl f64>::sin::hb3cd0e4a298bd1b1 (;960;) (param $var0 f64) (result f64)
    (local $var1 i32)
    (local $var2 i32)
    (local $var3 i32)
    (local $var4 i32)
    (local $var5 i32)
    (local $var6 f64)
    (local $var7 f64)
    global.get $global0
    local.set $var1
    i32.const 16
    local.set $var2
    local.get $var1
    local.get $var2
    i32.sub
    local.set $var3
    local.get $var3
    global.set $global0
    local.get $var3
    local.get $var0
    f64.store
    local.get $var0
    call $signature_mismatch:sin
    local.set $var6
    local.get $var3
    local.get $var6
    f64.store offset=8
    local.get $var3
    f64.load offset=8
    local.set $var7
    i32.const 16
    local.set $var4
    local.get $var3
    local.get $var4
    i32.add
    local.set $var5
    local.get $var5
    global.set $global0
    local.get $var7
    return
  )

This appears to push $var0 to the stack and then immediately calls $signature_mismatch:sin, which halts.

The working case (without enum/match) compiles to the following:

  (func $std::f64::<impl f64>::sin::hb3cd0e4a298bd1b1 (;743;) (param $var0 f64) (result f64)
    (local $var1 i32)
    (local $var2 i32)
    (local $var3 i32)
    (local $var4 i32)
    (local $var5 i32)
    (local $var6 f64)
    (local $var7 f64)
    global.get $global0
    local.set $var1
    i32.const 16
    local.set $var2
    local.get $var1
    local.get $var2
    i32.sub
    local.set $var3
    local.get $var3
    global.set $global0
    local.get $var3
    local.get $var0
    f64.store
    local.get $var0
    call $sin
    local.set $var6
    local.get $var3
    local.get $var6
    f64.store offset=8
    local.get $var3
    f64.load offset=8
    local.set $var7
    i32.const 16
    local.set $var4
    local.get $var3
    local.get $var4
    i32.add
    local.set $var5
    local.get $var5
    global.set $global0
    local.get $var7
    return
  )

I've uploaded the complete code to reproduce the issue here: https://github.com/hexane360/wasm-array-bug

I'm filing this issue here because it appears to be a miscompilation. It's possible this is actually a problem in wasm-bindgen or wasm-pack. Please let me know and I'll open an issue there instead.

Meta

rustc --version --verbose:

rustc 1.79.0 (129f3b996 2024-06-10)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: aarch64-apple-darwin
release: 1.79.0
LLVM version: 18.1.7
  • wasm-pack version 0.12.1
  • wasm-bindgen version 0.2.92

@hexane360 hexane360 added the C-bug Category: This is a bug. label Jul 23, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 23, 2024
@hexane360 hexane360 changed the title Runtime error using f64::sin Runtime error using f64::sin in wasm32-wasi Jul 23, 2024
@hexane360 hexane360 changed the title Runtime error using f64::sin in wasm32-wasi Runtime error using f64::sin in wasm32 Jul 23, 2024
@workingjubilee
Copy link
Member

Your reproducer is incomplete because it does not include the build command you used with wasm-pack. It is possible to compile wasm code without wasm pack.

@workingjubilee workingjubilee added the O-wasm Target: WASM (WebAssembly), http://webassembly.org/ label Jul 23, 2024
@hexane360
Copy link
Author

You can build the reproduction using npm run build or npm run serve. I know it's possible to build without wasm-pack, I'll work on a reproduction without wasm-pack or wasm_bindgen but may take me a couple days to get to.

@workingjubilee
Copy link
Member

It is fine if you didn't, but e.g. every time I have touched wasm-pack, nothing flowed through npm, so I am actually surprised to hear you are using it.

@hexane360
Copy link
Author

hexane360 commented Jul 23, 2024

I've significantly reduced the reproduction and identified the problem as a symbol conflict.

Here's the reproduction:

#![no_main]

#[no_mangle]
pub extern "C" fn sin_(ptr: i64) -> i64 {
    ptr
}

#[no_mangle]
pub extern "C" fn cos(ptr: i64) -> i64 {
    ptr
}

#[no_mangle]
pub extern "C" fn _start() {
    println!("{}", 5.0f64.sin()); // runs fine
    println!("{}", 5.0f64.cos()); // errors
}

Build with cargo build --target wasm32-unknown-unknown, and it can be run with wasmer run -v target/wasm32-unknown-unknown/debug/wasm-array-bug.wasm

When compiling f64::cos(), a function named cos is generated (un-mangled):

(func $cos (type 4) (param f64) (result f64)
    local.get 0
    call $_ZN17compiler_builtins4math3cos17h1c9c941a8073602cE)

This generated function conflicts with the exported cos function (which takes precedence). During linking, LLVM notices the signature mismatch, and generates the signature_mismatch:sin stub. In my case, the error only happens with f64 because the f32 intrinsic is called cosf instead of cos.

Using codegen-units = 1, the signature mismatch isn't noticed, and the generated WASM file has type errors:

~$ wasm-validate target/wasm32-unknown-unknown/debug/wasm-array-bug.wasm
target/wasm32-unknown-unknown/debug/wasm-array-bug.wasm:00001e5: error: type mismatch in call, expected [i64] but got [f64]
target/wasm32-unknown-unknown/debug/wasm-array-bug.wasm:00001e7: error: type mismatch in local.set, expected [f64] but got [i64]

Remaining questions:

  • Why aren't the sin/cos intrinsics mangled?
  • Why isn't the linking error reported to rustc?

@workingjubilee
Copy link
Member

oh neat!

@workingjubilee
Copy link
Member

f32 sin should also be sinf???

@hexane360
Copy link
Author

hexane360 commented Jul 24, 2024

Yeah, these are the functions generated by f32::sin, f32::cos, f64::sin, f64::cos respectively:

  (func $sinf (type 5) (param f32) (result f32)
    local.get 0
    call $_ZN17compiler_builtins4math4sinf17ha493429580d1ed0cE)
  (func $cosf (type 5) (param f32) (result f32)
    local.get 0
    call $_ZN17compiler_builtins4math4cosf17h2f95fcc8c4a732edE)
  (func $sin (type 7) (param f64) (result f64)
    local.get 0
    call $_ZN17compiler_builtins4math3sin17h958cb507a9dbd375E)
  (func $cos (type 7) (param f64) (result f64)
    local.get 0
    call $_ZN17compiler_builtins4math3cos17h1c9c941a8073602cE)

I believe these symbol names correspond to those exported by libm. I was only running into a problem with sin::f64 and cos::f64 because I was only exporting functions named sin and cos.

@workingjubilee
Copy link
Member

I see.

Hm, so, shadowing the libm names causing problems sounds like an unfortunate and expected hazard of using no_mangle.

I think the rest is a wasm_bindgen bug then...?

@hexane360
Copy link
Author

I'm out of my depth as to whether this is expected behavior or not. Naively, I'd assume this would generate a linker error rather than generating code broken at runtime, though I'm not sure how easy it'd be to fix. It does look like something similar happens with the same code compiled to arm64 (in this case, silent failure at runtime).

I don't think there's any bug in wasm_bindgen, as I get the same behavior with and without. However, in theory it should be possible to change a function name in the wasm module without changing the name exported in the JS shim.

related: #28179

@tgross35
Copy link
Contributor

tgross35 commented Jul 24, 2024

It is easy to cause UB with no_mangle if your symbol names happen to conflict, even with no unsafe anywhere

#[no_mangle]
extern "C" fn malloc() {
    panic!(
        "malloc is a good name for a function that finds \
        Original Content at the mall, right?"
    );
}

// doesn't even do anything but the program crashes
fn main() {}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a550cf9f28b03c86187c46e61888d090

For that reason, starting in edition 2024 it will be #[unsafe(no_mangle)].

Symbols can be either strong (always gets picked, duplicates raise errors) or weak (yield to a strong symbol if available, if there are >1 weak symbols it just picks one). Symbols exported by Rust are always strong (the way to configure this is unstable), and I think libm symbols are always weak, as is malloc from this example, so it's just picking the strong ones. (I'm not 100% sure this is the same for wasm but I figure it's similar).

Your best bet here might be to change the names you export to JS, then just wrap them on the JS side with the correct name. Assuming this is coming from wasm-bindgen and there aren't knobs to twiddle.

@workingjubilee
Copy link
Member

workingjubilee commented Jul 24, 2024

adding #![forbid(unsafe_code)] means it won't even compile, regardless of edition:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f78e430f3275cdcfbe35a641e6b25ecd

@hexane360
Copy link
Author

Yeah, it looks like this is a dupe of #28179, and of rustwasm/wasm-bindgen#2338

Thanks for the help debugging.

@tgross35 tgross35 added C-gub Category: the reverse of a compiler bug is generally UB and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-gub Category: the reverse of a compiler bug is generally UB O-wasm Target: WASM (WebAssembly), http://webassembly.org/
Projects
None yet
Development

No branches or pull requests

4 participants