Closed
Bug 1263203
Opened 8 years ago
Closed 8 years ago
[wasm] Assertion failure: from->type() == to->type() (Def replacement has different type), at js/src/jit/ValueNumbering.cpp:145
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: decoder, Assigned: bbouvier)
Details
(Keywords: assertion, testcase)
Attachments
(2 files, 1 obsolete file)
232 bytes,
application/octet-stream
|
Details | |
5.47 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
The attached binary WebAssembly testcase crashes on mozilla-inbound revision 49784ed1b9e7+ (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). 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)); Backtrace: ==4264==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000127e78a bp 0x7ffd67543350 sp 0x7ffd67543350 T0) ==4264==The signal is caused by a WRITE memory access. ==4264==Hint: address points to the zero page. #0 0x127e789 in ReplaceAllUsesWith(js::jit::MDefinition*, js::jit::MDefinition*) js/src/jit/ValueNumbering.cpp:144:5 #1 0x127dd2d in js::jit::ValueNumberer::visitDefinition(js::jit::MDefinition*) js/src/jit/ValueNumbering.cpp:767:9 #2 0x12805aa in js::jit::ValueNumberer::visitBlock(js::jit::MBasicBlock*, js::jit::MBasicBlock const*) js/src/jit/ValueNumbering.cpp:961:14 #3 0x1280c1b in js::jit::ValueNumberer::visitDominatorTree(js::jit::MBasicBlock*) js/src/jit/ValueNumbering.cpp:1006:18 #4 0x128146b in js::jit::ValueNumberer::visitGraph() js/src/jit/ValueNumbering.cpp:1044:18 #5 0x1282cc5 in js::jit::ValueNumberer::run(js::jit::ValueNumberer::UpdateAliasAnalysisFlag) js/src/jit/ValueNumbering.cpp:1213:14 #6 0xd43b26 in js::jit::OptimizeMIR(js::jit::MIRGenerator*) js/src/jit/Ion.cpp:1628:14 #7 0x6cc9aa in js::wasm::IonCompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmIonCompile.cpp:3202:14 #8 0x693a18 in js::wasm::ModuleGenerator::finishFuncDef(unsigned int, unsigned int, js::wasm::FunctionGenerator*) js/src/asmjs/WasmGenerator.cpp:815:14 #9 0x6295d0 in DecodeFunctionBody(JSContext*, js::wasm::Decoder&, js::wasm::ModuleGenerator&, unsigned int) js/src/asmjs/Wasm.cpp:1349:12 #10 0x6295d0 in DecodeFunctionBodies(JSContext*, js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/asmjs/Wasm.cpp:1377 #11 0x6295d0 in DecodeModule(JSContext*, mozilla::UniquePtr<char [], JS::FreePolicy>, unsigned char const*, unsigned int, mozilla::Vector<ImportName, 0ul, 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:1476 #12 0x61cd00 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<JSObject*>) js/src/asmjs/Wasm.cpp:1633:10 #13 0x5943b5 in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5212:14 [...] #26 0x460f01 in _start (/home/ubuntu/build/build/js+0x460f01) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV js/src/jit/ValueNumbering.cpp:144:5 in ReplaceAllUsesWith(js::jit::MDefinition*, js::jit::MDefinition*) ==4264==ABORTING
Reporter | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
Hah, that's something I've wanted to investigate, in case it could cause bugs: we can end up with MDefinitions that have MIRType_Value type, because of the if/else rules of decoding: (if (cond) (f32.const 0) (i32.const 0)), for instance. We should make sure we never encounter them during wasm compilations. I need to think a bit more before asking for review; maybe we can avoid pushing if we know there was already a different pushed type, but I am not sure.
Assignee | ||
Comment 3•8 years ago
|
||
A better patch: instead of popping the definition and not returning it if it has MIRType_Value (meaning we've already done a bad thing), just prevent to push definitions with different types (so that no phi with MIRType_Value is created in the first place).
Assignee: nobody → bbouvier
Attachment #8739505 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8739970 -
Flags: review?(luke)
Comment 4•8 years ago
|
||
Comment on attachment 8739970 [details] [diff] [review] 1263203.patch Review of attachment 8739970 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this seems like the right fix. ::: js/src/asmjs/WasmIonCompile.cpp @@ +973,5 @@ > if (def && def->type() != MIRType_None) > curBlock_->push(def); > } > > + MDefinition* popDef() How about popDefIfPushed()?
Attachment #8739970 -
Flags: review?(luke) → review+
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6828e2e05e16
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•