1256480 - [wasm] Assertion failure: i.mirType() == MIRType_Double, at js/src/asmjs/WasmStubs.cpp:312

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The attached binary WebAssembly testcase crashes on mozilla-inbound revision f788142ec96f+ (build with --enable-gczeal --enable-optimize --enable-debug --enable-address-sanitizer --without-intl-api --enable-posix-nspr-emulation --disable-jemalloc --disable-tests --target=i686-pc-linux-gnu --enable-debug). To reproduce, you can run the following code in the JS shell:

var data = os.file.readFile(file, 'binary');
wasmEval(data.buffer);


Backtrace:

==2427==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000 (pc 0x084139c9 bp 0xffbb1c08 sp 0xffbb1a20 T0)
==2427==The signal is caused by a WRITE memory access.
==2427==Hint: address points to the zero page.
    #0 0x84139c8 in js::jit::ABIArg::offsetFromArgBase() const js/src/jit/RegisterSets.h:1287:42
    #1 0x84139c8 in FillArgumentArray(js::jit::MacroAssembler&, mozilla::Vector<js::wasm::ValType, 8u, js::SystemAllocPolicy> const&, unsigned int, unsigned int, js::jit::Register) js/src/asmjs/WasmStubs.cpp:304
    #2 0x84103ab in js::wasm::GenerateInterpExit(js::jit::MacroAssembler&, js::wasm::Import const&, unsigned int) js/src/asmjs/WasmStubs.cpp:354:5
    #3 0x8316ff6 in js::wasm::ModuleGenerator::finishCodegen(js::wasm::StaticLinkData*) js/src/asmjs/WasmGenerator.cpp:385:30
    #4 0x8328523 in js::wasm::ModuleGenerator::finish(mozilla::Vector<js::wasm::CacheableChars, 0u, js::SystemAllocPolicy>&&, mozilla::UniquePtr<js::wasm::ModuleData, JS::DeletePolicy<js::wasm::ModuleData> >*, mozilla::UniquePtr<js::wasm::StaticLinkData, JS::DeletePolicy<js::wasm::StaticLinkData> >*, mozilla::UniquePtr<js::wasm::ExportMap, JS::DeletePolicy<js::wasm::ExportMap> >*, mozilla::Vector<js::wasm::SlowFunction, 0u, js::TempAllocPolicy>*) js/src/asmjs/WasmGenerator.cpp:892:10
    #5 0x82be19a in DecodeModule(JSContext*, mozilla::UniquePtr<char [], JS::FreePolicy>, unsigned char const*, unsigned int, mozilla::Vector<ImportName, 0u, js::SystemAllocPolicy>*, mozilla::UniquePtr<js::wasm::ExportMap, JS::DeletePolicy<js::wasm::ExportMap> >*, JS::MutableHandle<js::ArrayBufferObject*>, JS::MutableHandle<js::WasmModuleObject*>) js/src/asmjs/Wasm.cpp:1456:10
    #6 0x82adaf4 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<JSObject*>) js/src/asmjs/Wasm.cpp:1597:10
    #7 0x821daaf in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5104:14
    #8 0x9e3a411 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:235:15
[...]
    #20 0x80aa1b8 in _start (/home/ubuntu/build/build/js+0x80aa1b8)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV js/src/jit/RegisterSets.h:1287:42 in js::jit::ABIArg::offsetFromArgBase() const
==2427==ABORTING

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Note that the suggested code in comment 0 doesn't work out of the box here:
- there's an import named 'a'
- the code should look like:

var data = os.file.readFile(file, 'binary');
Wasm.instantiateModule(new Uint8Array(data.buffer), { a: console.log.bind(console) });

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: 1256480.patch

- Forbid SIMD types in the import/export arg/return values.
- When a float32 arg is passed to a FFI:
  - if it's in a register, convert the value (on x64, the result would be wrong before this patch! added a test that would have caught that + the assertion failure)
  - if it's on the stack (thank you x86), load it as a float32 and convert it to a double.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: 1256480.patch

A better version that can even handle ARM and solve 1256481 by the way. ARM convertFloat32ToDouble requires the src register to be a float32 reg and the dest register to be a double reg; so we can't just use the same reg as src and dest for converting a float32 val to a double val :(

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 8730762 [details] [diff] [review]
1256480.patch

Nice!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/27a6eb894849

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/27a6eb894849

Comment 9

[Eric Faust [:efaust]]

WASM is off by default. Luke says we're not uplifting wasm bugs. WONTFIX 47.