858749 - OdinMonkey: Assertion failure: predecessors_[j]->getSlot(i) == mine

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: crash report

1. http://n3emscripten.appspot.com/dragons_asmjs.html
2. wait a minute
3. Assertion failure: predecessors_[j]->getSlot(i) == mine, at c:/work/mozilla/builds/nightly/mozilla/js/src/ion/MIRGraph.cpp:677

js::ion::MBasicBlock::addPredecessorPopN js::ion::MBasicBlock::addPredecessor FunctionCompiler::bindBreaksOrContinues FunctionCompiler::bindContinues CheckWhile

Aurora/22, Nightly/23 - Windows, Mac, Linux

Comment 1

[Luke Wagner [:luke]]

Attachment: fix and test

Great find!

I was incorrectly using "is the MBasicBlock" empty to detect whether a join block had already been created when binding breaks/continues; this misses the case where no instruction is added by the SSA state has been updated.  EIBTI once again.

Comment 2

[Luke Wagner [:luke]]

s/by the SSA state/but the SSA state/

Comment 3

[Sean Stangl [:sstangl]]

Comment on attachment 738738 [details] [diff] [review]
fix and test

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

Just as a thought, it might be nice to take an MBasicBlock** instead of bool*, then, if the join block is provided, explicitly refer to it instead of using curBlock_. When refactoring IonBuilder, clarity was often improved by just naming blocks instead of using |current|.

::: js/src/ion/AsmJS.cpp
@@ +2232,1 @@
>      {

Although the code is obviously correct as-is, it may be nice to leave |JS_ASSERT(*createdJoinBlock == false)| as a signal to the reader that the initial value is expected to have been set by the caller.

Comment 4

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d061ec6e8451

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d061ec6e8451

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 738738 [details] [diff] [review]
fix and test

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 840282
User impact if declined: incorrect asm.js evaluation
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): very low

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1b749c57ba70