1335146 - Crash [@ ??] or Assertion failure: inputStores.length() > 0, at jit/FlowAliasAnalysis.cpp:627

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 71224049c0b5 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu, run with --fuzzing-safe --ion-aa=flow-sensitive):

while (true) {
    try {
        var buf = new Uint8ClampedArray(a);
    } catch (e) {
        break;
    }
}
while (true) {
    var e = inIon() ? -true.get : 1;
    while ([]) {
        inIon = 'x1';
    }
}



Backtrace:

 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xf7767b40 (LWP 12102)]
0x00000000 in ?? ()
#0  0x00000000 in ?? ()
#1  0x0829d590 in js::jit::MDefinition::is<js::jit::MStoreSlot> (this=0xf1350444) at js/src/jit/MIR.h:889
#2  js::jit::MDefinition::isStoreSlot (this=0xf1350444) at js/src/jit/MIR.h:909
#3  js::jit::MLoadSlot::mightAlias (this=0xf134e890, def=0xf1350444) at js/src/jit/MIR.cpp:5206
#4  0x0829ff13 in js::jit::MInstruction::foldsToStore (this=0xf134e890, alloc=...) at js/src/jit/MIR.cpp:295
#5  0x082a0183 in js::jit::MLoadSlot::foldsTo (this=0xf134e890, alloc=...) at js/src/jit/MIR.cpp:5230
#6  0x08335dbd in js::jit::ValueNumberer::simplified (def=0xf134e890, this=0xf77671ac) at js/src/jit/ValueNumbering.cpp:618
#7  js::jit::ValueNumberer::visitDefinition (this=0xf77671ac, def=0xf134e890) at js/src/jit/ValueNumbering.cpp:778
#8  0x0833691a in js::jit::ValueNumberer::visitBlock (this=0xf77671ac, block=0xf134e1e0, dominatorRoot=0xf79808b8) at js/src/jit/ValueNumbering.cpp:997
#9  0x08336d65 in js::jit::ValueNumberer::visitDominatorTree (this=0xf77671ac, dominatorRoot=0xf79808b8) at js/src/jit/ValueNumbering.cpp:1042
#10 0x08336fae in js::jit::ValueNumberer::visitGraph (this=<optimized out>) at js/src/jit/ValueNumbering.cpp:1080
#11 js::jit::ValueNumberer::run (this=0xf77671ac, updateAliasAnalysis=js::jit::ValueNumberer::UpdateAliasAnalysis) at js/src/jit/ValueNumbering.cpp:1251
#12 0x08209dc2 in js::jit::OptimizeMIR (mir=0xf1349130) at js/src/jit/Ion.cpp:1724
#13 0x0820acf0 in js::jit::CompileBackEnd (mir=0xf1349130) at js/src/jit/Ion.cpp:2068
#14 0x0854c5a0 in js::HelperThread::handleIonWorkload (this=0xf7941000, locked=...) at js/src/vm/HelperThreads.cpp:1508
#15 0x0854d3ec in js::HelperThread::threadLoop (this=0xf7941000) at js/src/vm/HelperThreads.cpp:1871
#16 0x08553cbb in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::callMain<0u> (this=0xf790b080) at js/src/threading/Thread.h:234
#17 js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start (aPack=0xf790b080) at js/src/threading/Thread.h:227
#18 0xf7fb228a in start_thread (arg=0xf7767b40) at pthread_create.c:333
#19 0xf7cd44ce in clone () from /lib32/libc.so.6
eax	0xf1350444	-248183740
ebx	0xf1350444	-248183740
ecx	0x8	8
edx	0xf1350444	-248183740
esi	0x8959ff4	144023540
edi	0xf134e890	-248190832
ebp	0xf7766f18	4151734040
esp	0xf7766efc	4151734012
eip	0x0	0
=> 0x0:	


This looks like a jump to NULL but I'm not sure if this is guaranteed, so marking s-s until it has been investigated. This also only crashes on 32 bit while it also asserts on 64 bit.

Comment 1

[Andrew McCreight [:mccr8]]

It looks like you changed FlowAliasAnalysis.cpp recently, nbp, can you take a look at this? Thanks.

Comment 2

[Nicolas B. Pierron [:nbp]]

I will look at it next week, if I have time during a firefox compilation.
Otherwise, Hannes feel free to take it.

Comment 3

[Andrew McCreight [:mccr8]]

This sounds potentially bad so I'm going to mark it sec-high. Hopefully somebody can take a look soon.

Comment 4

[Hannes Verschore [:h4writer]]

The correct people are needinfo'ed. This crash is flow alias analysis which is not enabled by default, currently.

Comment 5

[Andrew McCreight [:mccr8]]

(In reply to Hannes Verschore [:h4writer] from comment #4)
> The correct people are needinfo'ed. This crash is flow alias analysis which
> is not enabled by default, currently.

If there's a bug for enabling it by default, can you make this blocking it? Thanks.

Comment 6

[Hannes Verschore [:h4writer]]

Attachment: Patch

Due to GVN we sometimes get a disconnected graph. In that state (before fixup) the first block of the graph is not the entry of the graph. It only get deleted lateron till the start of the OSR. (Since that is the entry in that case). As a result in FlowAA we don't have any dependencies in the fake-entry and we are asserting. We can just run no FlowAA on those. They will get deleted after GVN ends and we re-run AA again.

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 8839858 [details] [diff] [review]
Patch

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

Hm annoying.

Comment 8

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8839858 [details] [diff] [review]
Patch

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

::: js/src/jit/FlowAliasAnalysis.cpp
@@ +469,5 @@
>          BlockStoreInfo& blockInfo = stores_->current();
> +
> +        // When the store dependencies is empty it means we have a disconnected
> +        // graph. Those blocks will never get reached but it is only fixed up
> +        // after GVN. Don't run AA on those blocks.

Are these OSR-dominated blocks, or dead blocks?

During GVN all loop header should have an extra predecessor to keep them alive, if the blocks are OSR-dominated, then they got to keep these fix-up blocks as unreachable fake parent block.

Comment 9

[Frederik Braun [:freddy]]

Hannes, this bug is assigned to you and the patch has an r+. What's the next step? :)

Comment 10

[Hannes Verschore [:h4writer]]

Opening since this is not enabled by default path.

(In reply to Frederik Braun [:freddyb] from comment #9)
> Hannes, this bug is assigned to you and the patch has an r+. What's the next
> step? :)

Landing would be the next step.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cba3e613627
IonMonkey - Don't do flow-aa on blocks without entry predecessor, r=jandem

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8cba3e613627