Closed Bug 1275224 Opened 3 years ago Closed 3 years ago

[wasm] Hit MOZ_CRASH(NYI: AsmReinterpretToI64) at js/src/jit/LIR.h:879


(Core :: JavaScript Engine, defect, critical)

Not set



Tracking Status
firefox49 --- disabled
firefox50 --- fixed


(Reporter: decoder, Assigned: bbouvier)


(Blocks 1 open bug)


(Keywords: assertion, testcase)


(2 files)

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

var data = os.file.readFile(file, 'binary');
Wasm.instantiateModule(new Uint8Array(data.buffer));


==25159==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000 (pc 0x08d4a079 bp 0xffbdba58 sp 0xffbdba30 T0)
    #0 0x8d4a078 in js::jit::LElementVisitor::visitAsmReinterpretToI64(js::jit::LAsmReinterpretToI64*) js/src/jit/LIR.h:879:5
    #1 0x93c9c38 in js::jit::LAsmReinterpretToI64::accept(js::jit::LElementVisitor*) js/src/jit/shared/LIR-shared.h:1277:5
    #2 0x8c07447 in js::jit::CodeGenerator::generateBody() js/src/jit/CodeGenerator.cpp:5070:13
    #3 0x8c9ce37 in js::jit::CodeGenerator::generateAsmJS(js::wasm::FuncOffsets*) js/src/jit/CodeGenerator.cpp:8808:10
    #4 0x83c9462 in js::wasm::IonCompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmIonCompile.cpp:3460:14
    #5 0x83737e4 in js::wasm::ModuleGenerator::finishFuncDef(unsigned int, unsigned int, js::wasm::FunctionGenerator*) js/src/asmjs/WasmGenerator.cpp:823:14
    #6 0x82e2765 in DecodeFunctionBody(JSContext*, js::wasm::Decoder&, js::wasm::ModuleGenerator&, unsigned int) js/src/asmjs/Wasm.cpp:966:12
    #7 0x82e2765 in DecodeCodeSection(JSContext*, js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/asmjs/Wasm.cpp:994
    #8 0x82e2765 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:1093
    #9 0x82d467b in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<JSObject*>) js/src/asmjs/Wasm.cpp:1250:10
    #10 0x8214fdc in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5220:14
    #11 0xa5b06a8 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:235:15
    #25 0x80aaabc in _start (/home/ubuntu/build/build/js+0x80aaabc)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV js/src/jit/LIR.h:879:5 in js::jit::LElementVisitor::visitAsmReinterpretToI64(js::jit::LAsmReinterpretToI64*)
Attached file Testcase
Duplicate of this bug: 1275225
Duh. For CopySign on x86/ARM, we can't use AsmReinterpret64, because we don't have 64 bits support there.
What if we just leave copysign as NYI on x86/arm until they have i64 support?

Alternatively, a dedicated MIR node would enable us to emit better copysign code (using andpd, orpd, etc. on x86/x64), so that might be useful anyway.
Flags: needinfo?(sunfish)
Dan, the other alternative I can think of is to have our own MCopySign MIR node. Then on x86/arm, we could put the upper 32 bits of the double input into a GPR, then apply the bit twiddling, then put back the upper 32 bits into the highest bits of a copy of the input. This is how fdlibm does it. What do you think? Is there a better alternative that would not need its own MIR node?
Flags: needinfo?(sunfish)
Flags: needinfo?(bbouvier)
Attached patch copysign.patchSplinter Review
Assignee: nobody → bbouvier
Flags: needinfo?(bbouvier)
Attachment #8759147 - Flags: review?(sunfish)
Comment on attachment 8759147 [details] [diff] [review]

Review of attachment 8759147 [details] [diff] [review]:

Looks good. I haven't looked at whether there's a more efficient sequence on ARM, but this is good for now either way.
Attachment #8759147 - Flags: review?(sunfish) → review+
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Duplicate of this bug: 1277210
Worth uplifting to 49? Or do you want to leave this to ride to release on 50?
Flags: needinfo?(bbouvier)
WebAssembly is only enabled on Nightly, so it does not need to be uplifted to 49 yet. Thanks for the heads-up though.
Flags: needinfo?(bbouvier)
You need to log in before you can comment on or make changes to this bug.