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)
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: decoder, Unassigned)
References
Details
(4 keywords, Whiteboard: [jsbugmon:update])
Attachments
(1 file)
1.76 KB,
patch
|
sunfish
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•9 years ago
|
Flags: needinfo?(nicolas.b.pierron)
Updated•8 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 1•8 years ago
|
||
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
Comment 2•8 years ago
|
||
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
Comment 3•8 years ago
|
||
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)
Updated•8 years ago
|
Flags: needinfo?(nicolas.b.pierron)
Updated•8 years ago
|
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
Comment 5•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 6•8 years ago
|
||
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)
Comment 7•8 years ago
|
||
(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)
Comment 8•8 years ago
|
||
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.
Description
•