1721006 - Assertion failure: aIndex < mLength, at include/mozilla/Vector.h:493

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P3)

Description

[lukas.bernhard]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:92.0) Gecko/20100101 Firefox/92.0

Steps to reproduce:

Assertion failure: aIndex < mLength, at include/mozilla/Vector.h:493

The following testcase crashes on git commit 740e77e6aec278385381ba9c22f9d88d91c2b858 (latest trunk) with commandline:

obj-x86_64-pc-linux-gnu/dist/bin/js --no-threads --cpu-count=1 --ion-offthread-compile=off --baseline-warmup-threshold=10 --ion-warmup-threshold=100 crash.js

function main() {
    let v3 = 0;
    while (v3 < 9) {
        v3++;
    }
    let v18 = 1;
    let v19 = 1;
    for (let v23 = 0; v23 !== 100; v23 = v23++) {
        v19 &= v18;
        for (let v28 = 0; v28 < 100; v28++) {}
        v18 = v23;
        for (let v33 = 0; v33 < 8; v33++) {}
    }   
}
main();

git blame identifies the first bad commit as 6e9c3aef5f40c309d6fb5541861c03e95b977f3f, which corresponds to "Bug 1702306: Enable branch pruning by default".
It is probably a regression of bug 1697696 though.
Flagged as sec since the other regressions of bug 1697696 were initially tagged as sec as well. ASAN doesn't detect a violation in release builds.

Comment 1

[Nicolas B. Pierron [:nbp] {away}]

So far, testing once, I was able to reproduce a similar crash at the following location:

(rr) bt
#0  0x0000556de7b5ac00 in mozilla::Vector<js::jit::MBasicBlock*, 1ul, js::jit::JitAllocPolicy>::operator[] (this=0x556deaac5238, aIndex=1)
    at /home/nicolas/mozilla/_build/js/bugzil.la/1718102/wip/x64/gcc/dbg/dist/include/mozilla/Vector.h:493
#1  0x0000556de7b451e9 in js::jit::MBasicBlock::getPredecessor (this=0x556deaac5200, i=1) at /home/nicolas/mozilla/wksp-4/js/src/jit/MIRGraph.h:362
#2  0x0000556de7e8cf33 in js::jit::MPhi::assertLoopPhi (this=0x556deaac58b8) at /home/nicolas/mozilla/wksp-4/js/src/jit/MIR.cpp:1764
#3  0x0000556de7b81244 in js::jit::MPhi::getLoopBackedgeOperand (this=0x556deaac58b8) at /home/nicolas/mozilla/wksp-4/js/src/jit/MIR.h:5729
#4  0x0000556de7b7acd9 in js::jit::LIRGeneratorShared::ShouldReorderCommutative (lhs=0x556deaac59a0, rhs=0x556deaac58b8, ins=0x556deaac6430) at /home/nicolas/mozilla/wksp-4/js/src/jit/shared/Lowering-shared.cpp:62
#5  0x0000556de7b7ad3f in js::jit::LIRGeneratorShared::ReorderCommutative (lhsp=0x7ffcc4bb0c58, rhsp=0x7ffcc4bb0c60, ins=0x556deaac6430) at /home/nicolas/mozilla/wksp-4/js/src/jit/shared/Lowering-shared.cpp:75
#6  0x0000556de7e2d1b2 in js::jit::LIRGenerator::lowerBitOp (this=0x7ffcc4bb0e30, op=JSOp::BitAnd, ins=0x556deaac6430) at /home/nicolas/mozilla/wksp-4/js/src/jit/Lowering.cpp:1105
#7  0x0000556de7e2da84 in js::jit::LIRGenerator::visitBitAnd (this=0x7ffcc4bb0e30, ins=0x556deaac6430) at /home/nicolas/mozilla/wksp-4/js/src/jit/Lowering.cpp:1201
#8  0x0000556de7e4f992 in js::jit::LIRGenerator::visitInstructionDispatch (this=0x7ffcc4bb0e30, ins=0x556deaac6430) at /home/nicolas/mozilla/wksp-4/js/src/jit/Lowering.cpp:5728
#9  0x0000556de7e52675 in js::jit::LIRGenerator::visitInstruction (this=0x7ffcc4bb0e30, ins=0x556deaac6430) at /home/nicolas/mozilla/wksp-4/js/src/jit/Lowering.cpp:5751
#10 0x0000556de7e52b35 in js::jit::LIRGenerator::visitBlock (this=0x7ffcc4bb0e30, block=0x556deaac5fe8) at /home/nicolas/mozilla/wksp-4/js/src/jit/Lowering.cpp:5818
#11 0x0000556de7e5304d in js::jit::LIRGenerator::generate (this=0x7ffcc4bb0e30) at /home/nicolas/mozilla/wksp-4/js/src/jit/Lowering.cpp:5892
#12 0x0000556de7d8d1d6 in js::jit::GenerateLIR (mir=0x556deaac2d18) at /home/nicolas/mozilla/wksp-4/js/src/jit/Ion.cpp:1496
#13 0x0000556de7d8d648 in js::jit::CompileBackEnd (mir=0x556deaac2d18, snapshot=0x556deaac3270) at /home/nicolas/mozilla/wksp-4/js/src/jit/Ion.cpp:1590
#14 0x0000556de7d8e057 in js::jit::IonCompile (cx=0x556deaa7b580, script=..., osrPc=0x556deab1d230 "\221\v") at /home/nicolas/mozilla/wksp-4/js/src/jit/Ion.cpp:1722
#15 0x0000556de7d8eaf5 in js::jit::Compile (cx=0x556deaa7b580, script=..., osrFrame=0x7ffcc4bb2570, osrPc=0x556deab1d230 "\221\v") at /home/nicolas/mozilla/wksp-4/js/src/jit/Ion.cpp:1907
#16 0x0000556de7d8f5b2 in BaselineCanEnterAtBranch (cx=0x556deaa7b580, script=..., osrFrame=0x7ffcc4bb2570, pc=0x556deab1d230 "\221\v") at /home/nicolas/mozilla/wksp-4/js/src/jit/Ion.cpp:2108

Based on the suggestion from comment 0, this would likely be a problem with the fixup block which does not last until the Lowering phase.

The assertion in assertLoopPhi checks that our Loop header have 2 predecessors. Optimizations such as Branch Pruning, GVN+UCE and Range Analysis+UCE have the potential for adding such modifications to the graph.

Comment 2

[Iain Ireland [:iain]]

Nice catch! Fortunately, this turns out to be innocuous.

LIRGeneratorShared::ShouldReorderCommutative calls getLoopBackedgeOperand, which returns the element at index 1 of the inputs_ vector. If this block is a loop header with a since-removed fake predecessor, then that vector will only have one element, so this will assert. In release builds, we will instead load the current value at inputs_[1]. We allocate two elements inline for inputs_, so this will never access unallocated memory. In practice, because we delete element 0 and copy element 1 to replace it, the stale value at inputs_[1] will always be the same as the value at inputs_[0] that we should actually be loading, and we do the right thing accidentally.

This is the only place that we call getLoopBackedgeOperand or getLoopPredecessorOperand after removing fake predecessor blocks. I searched through all the lowering/codegen files for "loop" and didn't find any other code that depends on anything more than "is this a loop header?".

Comment 3

[Iain Ireland [:iain]]

Attachment: Bug 1721006: Fix getLoopBackedgeOperand r=nbp!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6501a923ecf7
Fix getLoopBackedgeOperand r=nbp

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/6501a923ecf7