1663861 - Crash [@ RustMozCrash] through [@ cranelift_codegen::machinst::compile::compile]

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Christian Holler (:decoder)]

The attached testcase crashes on mozilla-central revision 20200908-dc90a7a18c07 (opt build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off test.js).

Backtrace:

==13847==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x5622c31b36eb bp 0x7ffe47706ad0 sp 0x7ffe47706680 T0)
==13847==The signal is caused by a WRITE memory access.
==13847==Hint: address points to the zero page.
    #0 0x5622c31b36eb in RustMozCrash
    #1 0x5622c2e2c7ec in mozglue_static::panic_hook::h92ca72deb9e91048 mozglue/static/rust/lib.rs:89:8
    #2 0x5622c2e2c6bb in core::ops::function::Fn::call::hc67766f75bab9932 /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550/src/libcore/ops/function.rs:72:4
    #3 0x5622c3003021 in std::panicking::rust_panic_with_hook::hb976084785e50594 /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550/src/libstd/panicking.rs:474:16
    #4 0x5622c3002dd9 in rust_begin_unwind /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550/src/libstd/panicking.rs:378:4
    #5 0x5622c2b783df in core::panicking::panic_fmt::h45f7d6868edb5678 /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550/src/libcore/panicking.rs:85:13
    #6 0x5622c2b7e901 in core::option::expect_none_failed::hd7ba247f271405e7 /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550/src/libcore/option.rs:1211:4
    #7 0x5622c2cd8b00 in core::result::Result$LT$T$C$E$GT$::expect::h7d64754d9298545f /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550/src/libcore/result.rs:961:22
    #8 0x5622c2cd8b00 in cranelift_codegen::machinst::compile::compile::h08fc74b4854279c6 third_party/rust/cranelift-codegen/src/machinst/compile.rs:76:8
    #9 0x5622c2cd8b00 in cranelift_codegen::isa::x64::X64Backend::compile_vcode::h0283d6425b674462 third_party/rust/cranelift-codegen/src/isa/x64/mod.rs:45:8
    #10 0x5622c2cd8b00 in _$LT$cranelift_codegen..isa..x64..X64Backend$u20$as$u20$cranelift_codegen..machinst..MachBackend$GT$::compile_function::h95050aac3ad52e30 third_party/rust/cranelift-codegen/src/isa/x64/mod.rs:56:20
    #11 0x5622c2c8acdc in cranelift_codegen::context::Context::compile::h1fa1e3479e90bffd third_party/rust/cranelift-codegen/src/context.rs:192:25
    #12 0x5622c2b4f116 in baldrdash::compile::BatchCompiler::compile::h6f3a4ea5f1ab0760 js/src/wasm/cranelift/src/compile.rs:147:19
    #13 0x5622c2b4d1e2 in cranelift_compile_function js/src/wasm/cranelift/src/lib.rs:220:20
    #14 0x5622c22f8379 in js::wasm::CraneliftCompileFunctions(js::wasm::ModuleEnvironment const&, js::LifoAlloc&, mozilla::Vector<js::wasm::FuncCompileInput, 8ul, js::SystemAllocPolicy> const&, js::wasm::CompiledCode*, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmCraneliftCompile.cpp:496:10
    #15 0x5622c23d85ce in ExecuteCompileTask(js::wasm::CompileTask*, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmGenerator.cpp:752:16
    #16 0x5622c23d8875 in js::wasm::ModuleGenerator::locallyCompileCurrentTask() js/src/wasm/WasmGenerator.cpp:815:8
    #17 0x5622c23d9ddd in js::wasm::ModuleGenerator::finishFuncDefs() js/src/wasm/WasmGenerator.cpp:953:24
    #18 0x5622c22f2e31 in bool DecodeCodeSection<js::wasm::Decoder>(js::wasm::ModuleEnvironment const&, js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/wasm/WasmCompile.cpp:579:13
    #19 0x5622c22f1b23 in js::wasm::CompileBuffer(js::wasm::CompileArgs const&, js::wasm::ShareableBytes const&, mozilla::UniquePtr<char [], JS::FreePolicy>*, mozilla::Vector<mozilla::UniquePtr<char [], JS::FreePolicy>, 0ul, js::SystemAllocPolicy>*, JS::OptimizedEncodingListener*, JSTelemetrySender) js/src/wasm/WasmCompile.cpp:603:8
    #20 0x5622c2488b84 in js::WasmModuleObject::construct(JSContext*, unsigned int, JS::Value*) js/src/wasm/WasmJS.cpp:1514:25
    #21 0x5622c0bb5ba8 in CallJSNative js/src/vm/Interpreter.cpp:507:13
    [...]

Not filing s-s as this seems to be specific to the x64 backend, which is disabled by default.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

It seems like this the verifier error happens just after the translation, so the translation is wrong here. This requires more investigation.

Comment 3

[Lars T Hansen [:lth]]

P3 because x64.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Actually all platforms: there's a bug in translation from wasm to clif. The verifier says a value is used while not dominated by a definition in a block, which means uses of uninitialized data; it could be pretty bad. Hiding, just to be safe, since cranelift has shipped on aarch64.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Reduced test case:

var data= wasmTextToBinary(`
(module
  (func $main (type 0) (param i32 i32 i32) (result i32)
    i32.const 0
    i32.const 0
    i32.const 0
    i32.const 0

    i32.const 0
    if (param i32 i32 i32) (result i32)  ;; label = @1
      br 0 (;@1;)
    else
      call $main
    end

    i32.const 0
    i32.const 0
    if (param i32 i32 i32) (result i32)  ;; label = @1
      unreachable
    else
      unreachable
    end
  )
  (export "main" (func $main)))
`);

var module = new WebAssembly.Module(data.buffer);

Investigating. In the first multi-value returning if, Cranelift doesn't seem to use the stacktop value (and pop the two others) that are currently available in the br 0.

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Fixed in https://github.com/bytecodealliance/wasmtime/pull/2194, will need a Cranelift bump.

Comment 7

[Chris Fallin [:cfallin]]

Fixed by patch on bug 1664453, which vendored in Ben's PR #2194 above as well as a later fix.

Comment 8

[Mihai Boldan, Desktop QA [:mboldan]]

Removing the qe-verify+ flag since we were not able to reproduce the issue.