Cranelift: inconsistent NaN canonicalization behavior for f32.demote_f64

candymate commented 7 months ago

Test Case

// main.rs
use wasmtime::*;

fn main() -> Result<()> {
    let mut config = Config::default();
    config.strategy(Strategy::Cranelift);
    config.cranelift_opt_level(OptLevel::None);
    config.cranelift_nan_canonicalization(true); // doesn't matter: both options show the same result

    let engine = Engine::new(&config)?;
    let wat = r#"
    (module
        (type (;0;) (func (param i64) (result i32)))
        (import "mem" "mem" (memory (;0;) 1))
        (func (;0;) (type 0) (param i64) (result i32)
          local.get 0
          f64.reinterpret_i64
          f32.demote_f64                ;; <---- only this is needed to trigger the bug!
          i32.reinterpret_f32)
        (export "main" (func 0)))
    "#;
    let module = Module::new(&engine, wat)?;
    let mut store = Store::new(&engine, ());

    let memory_ty = MemoryType::new(1, None);
    let memory = Memory::new(&mut store, memory_ty.clone())?;

    let instance = Instance::new(&mut store, &module, &[memory.into()])?;
    let main = instance.get_func(&mut store, "main")
        .expect("`main` was not an exported function");
    let params = vec![
        Val::I64(0xfffefdfccccdcecfu64 as i64)
    ];
    let mut results = vec![Val::F32(0)];
    println!("Opt level None: {:?}", main.call(
        &mut store,
        &params,
        &mut results
    ));
    println!("{:?}", results);

    Ok(())
}

[package]
name = "wasmtime-wrapper"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
# wasmtime = "18.0.3"
wasmtime = { path = "../wasmtime/crates/wasmtime" } # commit: 5b576da4235e3df1bc4385644644157f720e5f21 (current latest, Date: Thu Mar 14 21:34:04 2024 -0500)

Steps to reproduce

Compare the following executions:

cargo run --release
cargo run --release --target=aarch64-unknown-linux-gnu
cargo run --release --target=s390x-unknown-linux-gnu
-----
cargo run --release --target=riscv64gc-unknown-linux-gnu

QEMU run options I'm currently using are the following:

qemu-aarch64 -L /usr/aarch64-linux-gnu -E LD_LIBRARY_PATH=/usr/aarch64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1 target/aarch64-unknown-linux-gnu/release/wasmtime-wrapper
qemu-s390x -L /usr/s390x-linux-gnu -E LD_LIBRARY_PATH=/usr/s390x-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1 target/s390x-unknown-linux-gnu/release/wasmtime-wrapper
qemu-riscv64 -cpu rv64,v=true,vlen=128,vext_spec=v1.0,zba=true,zbb=true,zbs=true,zbc=true,zbkb=true,zcb=true,zicond=true -L /usr/riscv64-linux-gnu -E LD_LIBRARY_PATH=/usr/riscv64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1 target/riscv64gc-unknown-linux-gnu/release/wasmtime-wrapper

Expected Results

Results from x64, aarch64, and s390x should be the value 0x7fc00000 (or 0xffc00000) due to NaN canonicalization

Opt level None: Ok(())
[I32(2143289344)]

Actual Results

Execution results in a wrong value -528410, which is 0xfff7efe6

Opt level None: Ok(())
[I32(-528410)]

However, RISC-V64 shows the correct result 0x7fc00000, which is a canonicalized NaN value:

Opt level None: Ok(())
[I32(2143289344)]

Versions and Environment

wasmtime version
- commit: 5b576da4235e3df1bc4385644644157f720e5f21(current latest, Date: Thu Mar 14 21:34:04 2024 -0500)
- However, also checked on v18.0.3
Operating system & architecture: Ubuntu 22.04.3 LTS, Arch: x86_64
QEMU version: all architectures (aarch64, s390x, riscv64) in version 8.2.1 (v8.2.1)

Extra Info

Having only f32.demote_f64 is enough to expose the semantic difference
cranelift_nan_canonicalization option doesn't change the result
Uploading as public (not a security vuln.) since the bug only exhibits a semantic difference and I don't think this can cause a security impact. Also, this is related to cranelift_nan_canonicalization flag, which is false by default.

candymate commented 7 months ago

I'm not sure which side contains the problem. (RISC-V vs. all the other architectures). According to the WebAssembly specifications, both results look fine. However, I expected cranelift_nan_canonicalization flag to canonicalize the NaN value computed from f32.demote_f64.

Specifications:

alexcrichton commented 7 months ago

Thanks, as always, for the report!

The NaN canonicalization pass in cranelift has an allow-listed set of opcodes which get a canonicalization sequence inserted and demotion is not one of those. I believe the correct answer pops out on risc-v because risc-v doesn't need nan canonicalization as it natively does it, but that explains why the other platforms are in error here.

Looks like we need to add another opcode to that list...

bytecodealliance / wasmtime