Closed Bug 1298354 Opened 8 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
https://hg.mozilla.org/mozilla-central/rev/6a2af8365238
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: