Closed Bug 1298354 Opened 9 years ago Closed 8 years ago

Assertion failure: *block->begin() == block->lastIns() || !mir()->optimizationInfo().gvnEnabled(), at js/src/jit/Lowering.cpp:4734

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox51 --- wontfix
firefox52 --- fixed

People

(Reporter: decoder, Unassigned)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 01748a2b1a46 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --ion-eager --ion-check-range-analysis --ion-offthread-compile=off): new Function(` while (true) { try { var buf = new Uint8ClampedArray(a); } catch (e) { break; } } var caughtInvalidArguments = false; while (true) { var a = inIon() ? -true.get : 0; while (x > 7 & 0) {} } `)(); Backtrace: received signal SIGSEGV, Segmentation fault. 0x0000000000714dbf in js::jit::LIRGenerator::visitBlock (this=0x7fffffffb010, block=0x7ffff69dbe40) at js/src/jit/Lowering.cpp:4733 #0 0x0000000000714dbf in js::jit::LIRGenerator::visitBlock (this=0x7fffffffb010, block=0x7ffff69dbe40) at js/src/jit/Lowering.cpp:4733 #1 0x000000000071502b in js::jit::LIRGenerator::generate (this=this@entry=0x7fffffffb010) at js/src/jit/Lowering.cpp:4790 #2 0x0000000000677f74 in js::jit::GenerateLIR (mir=mir@entry=0x7ffff69c8280) at js/src/jit/Ion.cpp:1933 #3 0x000000000069d832 in js::jit::CompileBackEnd (mir=mir@entry=0x7ffff69c8280) at js/src/jit/Ion.cpp:2028 #4 0x000000000069e651 in js::jit::IonCompile (cx=cx@entry=0x7ffff695f000, script=<optimized out>, baselineFrame=baselineFrame@entry=0x7fffffffc458, osrPc=<optimized out>, constructing=<optimized out>, recompile=<optimized out>, optimizationLevel=js::jit::OptimizationLevel::Normal) at js/src/jit/Ion.cpp:2303 #5 0x000000000069eda9 in js::jit::Compile (cx=cx@entry=0x7ffff695f000, script=script@entry=..., osrFrame=osrFrame@entry=0x7fffffffc458, osrPc=osrPc@entry=0x7ffff025c094 "゚", constructing=<optimized out>, forceRecompile=<optimized out>) at js/src/jit/Ion.cpp:2470 #6 0x000000000069f6b3 in BaselineCanEnterAtBranch (pc=0x7ffff025c094 "゚", osrFrame=0x7fffffffc458, script=..., cx=0x7ffff695f000) at js/src/jit/Ion.cpp:2657 #7 js::jit::IonCompileScriptForBaseline (cx=cx@entry=0x7ffff695f000, frame=frame@entry=0x7fffffffc458, pc=pc@entry=0x7ffff025c094 "゚") at js/src/jit/Ion.cpp:2715 #8 0x0000000000e2069f in js::jit::DoWarmUpCounterFallbackOSR (cx=0x7ffff695f000, frame=0x7fffffffc458, stub=0x7ffff69d1318, infoPtr=0x7fffffffc410) at js/src/jit/BaselineIC.cpp:143 #9 0x00007ffff7e3db24 in ?? () [...] #20 0x0000000000000000 in ?? () rax 0x0 0 rbx 0x7ffff69dce80 140737330925184 rcx 0x7ffff6c28a2d 140737333332525 rdx 0x0 0 rsi 0x7ffff6ef7770 140737336276848 rdi 0x7ffff6ef6540 140737336272192 rbp 0x7fffffffaf60 140737488334688 rsp 0x7fffffffaf00 140737488334592 r8 0x7ffff6ef7770 140737336276848 r9 0x7ffff7fe4740 140737354024768 r10 0x58 88 r11 0x7ffff6b9f750 140737332770640 r12 0x7ffff69dbe40 140737330921024 r13 0x7ffff69dbe40 140737330921024 r14 0x7fffffffb010 140737488334864 r15 0x7ffff69c8280 140737330840192 rip 0x714dbf <js::jit::LIRGenerator::visitBlock(js::jit::MBasicBlock*)+527> => 0x714dbf <js::jit::LIRGenerator::visitBlock(js::jit::MBasicBlock*)+527>: movl $0x0,0x0 0x714dca <js::jit::LIRGenerator::visitBlock(js::jit::MBasicBlock*)+538>: ud2 Test involves TypedArray and the assertion looks like a lower level jit assert, marking s-s.
Flags: needinfo?(nicolas.b.pierron)
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result: === Treeherder Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20160315095526" and the hash "1b13d13b8cc582431eb30bb19fee8a20c0047d86". The "bad" changeset has the timestamp "20160315095634" and the hash "130026ae6a1e5b748dc8a583ec612ccdb4b31c61". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1b13d13b8cc582431eb30bb19fee8a20c0047d86&tochange=130026ae6a1e5b748dc8a583ec612ccdb4b31c61
Nice finding decoder, this test case highlights one way that I did not thought of for adding fixup blocks in the graph. This is not a security issue. The problem comes from the fact that Range Analysis adds a MAssertRange instruction in a fixup block added by GVN. Which is a debug feature enabled by --ion-check-range-analysis flag. Fixup blocks are only allowed to have Phi instructions with no operands, and a goto. They are used to keep the reverse post-order the same across the compilation, by adding extra predecessors to loop edges. Adding any instructions in these blocks is an error, as these blocks are not supposed to have any associated code.
Group: javascript-core-security
Before this patch, we were adding an MAssertRange instruction checking the range of a Phi with no operand, from a fixup block added by GVN. This triggered the Lowering assertion which ensures that we have no other instructions than a Goto in unreachable blocks (fixup blocks). This patch prevents us from adding the range analysis assertion if the block is unreachable.
Attachment #8788224 - Flags: review?(sunfish)
Flags: needinfo?(nicolas.b.pierron)
Attachment #8788224 - Flags: review?(sunfish) → review+
Pushed by npierron@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6a2af8365238 Do not add AssertRange instructions in unreachable blocks. r=sunfish
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Hi :nbp, Since this bug is a regression and also affects 51, do you consider to uplift this for 51 if this patch is not too risky?
Flags: needinfo?(nicolas.b.pierron)
(In reply to Gerry Chang [:gchang] from comment #6) > Since this bug is a regression and also affects 51, do you consider to > uplift this for 51 if this patch is not too risky? Short answer: No. This affects many versions before 51, and this instrumentation is only used in the jit-test test suite. So, unless we have similar intermittent in the jittest-* on our infra, I do not see any reasons to backport it.
Flags: needinfo?(nicolas.b.pierron)
Per comment #7, mark 51 as won't fix.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: