Closed Bug 1261404 Opened 8 years ago Closed 8 years ago

Hit MOZ_CRASH(predecessor was not found) at js/src/jit/MIRGraph.cpp:1472

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: decoder, Assigned: bbouvier)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

The attached binary WebAssembly testcase crashes on mozilla-inbound revision f53dbc1c638a+ (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:

==7895==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000001067fe8 bp 0x7ffd521a8690 sp 0x7ffd521a8670 T0)
==7895==The signal is caused by a WRITE memory access.
==7895==Hint: address points to the zero page.
    #0 0x1067fe7 in mozilla::Vector<js::jit::MBasicBlock*, 1ul, js::jit::JitAllocPolicy>::length() const dist/include/mozilla/Vector.h:419:34
    #1 0x1067fe7 in js::jit::MBasicBlock::numPredecessors() const js/src/jit/MIRGraph.h:349
    #2 0x1067fe7 in js::jit::MBasicBlock::replacePredecessor(js::jit::MBasicBlock*, js::jit::MBasicBlock*) js/src/jit/MIRGraph.cpp:1458
    #3 0x1065463 in js::jit::MBasicBlock::NewSplitEdge(js::jit::MIRGraph&, js::jit::CompileInfo const&, js::jit::MBasicBlock*, unsigned long, js::jit::MBasicBlock*) js/src/jit/MIRGraph.cpp:405:5
    #4 0xd87698 in SplitCriticalEdgesForBlock(js::jit::MIRGraph&, js::jit::MBasicBlock*) js/src/jit/IonAnalysis.cpp:426:30
    #5 0xd87698 in js::jit::SplitCriticalEdges(js::jit::MIRGraph&) js/src/jit/IonAnalysis.cpp:441
    #6 0xd434db in js::jit::OptimizeMIR(js::jit::MIRGenerator*) js/src/jit/Ion.cpp:1494:14
    #7 0x6ccd13 in js::wasm::IonCompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmIonCompile.cpp:3181:14
    #8 0x693e08 in js::wasm::ModuleGenerator::finishFuncDef(unsigned int, unsigned int, js::wasm::FunctionGenerator*) js/src/asmjs/WasmGenerator.cpp:815:14
    #9 0x629b2d in DecodeFunctionBody(JSContext*, js::wasm::Decoder&, js::wasm::ModuleGenerator&, unsigned int) js/src/asmjs/Wasm.cpp:1349:12
    #10 0x629b2d in DecodeFunctionBodies(JSContext*, js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/asmjs/Wasm.cpp:1377
    #11 0x629b2d 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 0x61d1d6 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<JSObject*>) js/src/asmjs/Wasm.cpp:1633:10
    #13 0x594855 in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5212:14
    #14 0x1b12818 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:235:15
[...]
    #26 0x4613a1 in _start (/home/ubuntu/build/build/js+0x4613a1)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV dist/include/mozilla/Vector.h:419:34 in mozilla::Vector<js::jit::MBasicBlock*, 1ul, js::jit::JitAllocPolicy>::length() const
==7895==ABORTING
Attached file Testcase
We could add several times the same successor to a MTableSwitch, which shouldn't be done: here, SplitEdge would try to split the same edge two times (succeed the first time, and assert the second time with this assertion).

It's sad that we have to use a hashmap here, but if we'd use an array, it could be very sparse (e.g. (br_table 500 0 (cond))).
Assignee: nobody → bbouvier
Status: NEW → ASSIGNED
Attachment #8737786 - Flags: review?(sunfish)
Comment on attachment 8737786 [details] [diff] [review]
brtable.successors.patch

Review of attachment 8737786 [details] [diff] [review]:
-----------------------------------------------------------------

Would it be possible to use the mark flags, similar to what bindBranches does to avoid adding predecessors multiple times? If not, then this patch looks fine; br_table isn't common enough for this to be a significant performance concern.
Attachment #8737786 - Flags: review?(sunfish) → review+
Thank you for the review! I don't think we can reuse such mechanism: in bindBranches, we just wanted to know whether a block already had been visited (semantics of a set); here we want to reuse a case index if it's already set (semantics of a map). So I'll push it as is.
https://hg.mozilla.org/mozilla-central/rev/07d56e431cbf
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.

Attachment

General

Created:
Updated:
Size: