Closed
Bug 1293313
Opened 8 years ago
Closed 8 years ago
[wasm] Assertion failure: JSVAL_IS_DOUBLE_IMPL(l), at dist/include/js/Value.h:406
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: decoder, Assigned: sunfish)
References
Details
(Keywords: assertion, testcase)
Attachments
(2 files)
65 bytes,
application/octet-stream
|
Details | |
11.88 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
The attached binary WebAssembly testcase crashes on mozilla-inbound revision aa4472559aa9+ (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 --enable-simulator=arm). To reproduce, you can run the following code in the JS shell (running with --wasm-always-baseline might be necessary): var data = os.file.readFile(file, 'binary'); Wasm.instantiateModule(new Uint8Array(data.buffer)); Backtrace: ==8780==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000 (pc 0x0916223c bp 0xffa1efc8 sp 0xffa1ef00 T0) #0 0x916223b in DOUBLE_TO_JSVAL_IMPL(double) dist/include/js/Value.h:406:5 #1 0x916223b in JS::Value::setDouble(double) dist/include/js/Value.h:1071 #2 0x916223b in JS::DoubleValue(double) dist/include/js/Value.h:1460 #3 0x916223b in js::jit::MToDouble::foldsTo(js::jit::TempAllocator&) js/src/jit/MIR.cpp:4138 #4 0x950f22d in js::jit::ValueNumberer::simplified(js::jit::MDefinition*) const js/src/jit/ValueNumbering.cpp:618:12 #5 0x950f22d in js::jit::ValueNumberer::visitDefinition(js::jit::MDefinition*) js/src/jit/ValueNumbering.cpp:777 #6 0x951362e in js::jit::ValueNumberer::visitBlock(js::jit::MBasicBlock*, js::jit::MBasicBlock const*) js/src/jit/ValueNumbering.cpp:987:14 #7 0x9513d29 in js::jit::ValueNumberer::visitDominatorTree(js::jit::MBasicBlock*) js/src/jit/ValueNumbering.cpp:1032:18 #8 0x95147ae in js::jit::ValueNumberer::visitGraph() js/src/jit/ValueNumbering.cpp:1070:18 #9 0x9516ae0 in js::jit::ValueNumberer::run(js::jit::ValueNumberer::UpdateAliasAnalysisFlag) js/src/jit/ValueNumbering.cpp:1241:14 #10 0x8c90a59 in js::jit::OptimizeMIR(js::jit::MIRGenerator*) js/src/jit/Ion.cpp:1688:14 #11 0xb5b8465 in js::wasm::IonCompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmIonCompile.cpp:3592:14 #12 0xb5e5a58 in js::wasm::CompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmIonCompile.cpp:3617:16 #13 0xb557d48 in js::wasm::ModuleGenerator::finishFuncDef(unsigned int, js::wasm::FunctionGenerator*) js/src/asmjs/WasmGenerator.cpp:866:14 #14 0xb4de7fb in DecodeFunctionBody(js::wasm::Decoder&, js::wasm::ModuleGenerator&, unsigned int) js/src/asmjs/WasmCompile.cpp:1299:12 #15 0xb4de7fb in DecodeCodeSection(js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/asmjs/WasmCompile.cpp:1359 #16 0xb4de7fb in js::wasm::Compile(js::wasm::ShareableBytes const&, js::wasm::CompileArgs&&, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/asmjs/WasmCompile.cpp:1640 #17 0x82f2918 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) js/src/asmjs/WasmJS.cpp:238:27 #18 0x8222f8c in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5271:14 #19 0xa46d89d in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:235:15 [...] #33 0x80aea21 in _start (/home/ubuntu/build/build/js+0x80aea21) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV dist/include/js/Value.h:406:5 in DOUBLE_TO_JSVAL_IMPL(double) ==8780==ABORTING
Reporter | ||
Comment 1•8 years ago
|
||
Comment 2•8 years ago
|
||
This repros reliably on a vanilla x64 debug build if you pass --no-threads. I can confirm this is introduced by https://hg.mozilla.org/mozilla-central/rev/e5f310c9b166. I assume the fix is pretty easy if you're familiar with the invariants of GVN -- we're sticking a non-canonical double value into a js::Value which is a no no -- but I don't really know the invariants here. Dan, do you suppose you could look at this?
Blocks: 1248555
Flags: needinfo?(sunfish)
Assignee | ||
Comment 3•8 years ago
|
||
This fixes the constant-folding code to create new MConstants with NewRawDouble and NewRawFloat32 when the result of constant folding can be a non-canonical NaN.
Assignee: nobody → sunfish
Status: NEW → ASSIGNED
Flags: needinfo?(sunfish)
Attachment #8779422 -
Flags: review?(luke)
Comment 4•8 years ago
|
||
Comment on attachment 8779422 [details] [diff] [review] nan-bits.patch Review of attachment 8779422 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: js/src/jit/MIR.cpp @@ +172,5 @@ > + // constant-folding observable. > + if (retVal.isDouble()) > + return MConstant::NewRawDouble(alloc, ret); > + > + return MConstant::New(alloc, retVal); I know this is largely pre-existing, but I feel like the logic could be simplified: if (ins->type() == MIRType::Double) return MConstant::NewRawDouble(alloc, ret); Value retVal = NumberValue(JS::CanonicalizeNaN(ret)); if (ins->type() != MIRTypeFromValue(retVal)) ... return MConstant::New(alloc, retVal);
Attachment #8779422 -
Flags: review?(luke) → review+
Pushed by dgohman@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/aab58242977a IonMonkey: Handle non-canonical NaNs in constant folding. r=luke
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #4) > ::: js/src/jit/MIR.cpp > @@ +172,5 @@ > > + // constant-folding observable. > > + if (retVal.isDouble()) > > + return MConstant::NewRawDouble(alloc, ret); > > + > > + return MConstant::New(alloc, retVal); > > I know this is largely pre-existing, but I feel like the logic could be > simplified: > > if (ins->type() == MIRType::Double) > return MConstant::NewRawDouble(alloc, ret); > > Value retVal = NumberValue(JS::CanonicalizeNaN(ret)); > if (ins->type() != MIRTypeFromValue(retVal)) > ... > > return MConstant::New(alloc, retVal); Tidied up accordingly.
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/aab58242977a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•