1117882 - "Assertion failure: phi->hasUses() (Missed an unused phi)" on asm.js testcase

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Alon Zakai (:azakai)]

Attachment: fc.out.js

Attached testcase asserts in a debug build of SpiderMonkey on

Assertion failure: phi->hasUses() (Missed an unused phi), at /home/alon/Dev/mozilla-inbound/js/src/jit/ValueNumbering.cpp:676
Segmentation fault (core dumped)

Gary, this is the only bug I found after a weekend of csmith fuzzing on debug builds.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(function() {
    'use asm'
    function f(i1) {
        i1 = i1 | 0
        var i2 = 0
        while (1) {
            while (0 < 1);
            i1 = d(i1, i2) | 0
            i2 = 0
        }
    }
})()

asserts js debug builds on m-c rev 912036eeb024 with --fuzzing-safe --no-threads --ion-eager at Assertion failure: phi->hasUses() (Missed an unused phi), at jit/ValueNumbering.cpp

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/8f27a48a25d5
user:        Dan Gohman
date:        Wed Sep 17 10:27:25 2014 -0700
summary:     Bug 1029830 - IonMonkey: GVN: Replace UCE with GVN r=nbp

Dan, is bug 1029830 a likely regressor?

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x63ced, 0x0000000100440293 js-dbg-opt-64-dm-nsprBuild-darwin-912036eeb024`js::jit::ValueNumberer::loopHasOptimizablePhi(this=<unavailable>, header=<unavailable>) const + 291 at ValueNumbering.cpp:676, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100440293 js-dbg-opt-64-dm-nsprBuild-darwin-912036eeb024`js::jit::ValueNumberer::loopHasOptimizablePhi(this=<unavailable>, header=<unavailable>) const + 291 at ValueNumbering.cpp:676
    frame #1: 0x0000000100441285 js-dbg-opt-64-dm-nsprBuild-darwin-912036eeb024`js::jit::ValueNumberer::visitDominatorTree(this=0x00007fff5fbfb048, dominatorRoot=0x0000000102116188) + 357 at ValueNumbering.cpp:985
    frame #2: 0x00000001004414cb js-dbg-opt-64-dm-nsprBuild-darwin-912036eeb024`js::jit::ValueNumberer::visitGraph(this=0x00007fff5fbfb048) + 75 at ValueNumbering.cpp:1016
    frame #3: 0x0000000100441779 js-dbg-opt-64-dm-nsprBuild-darwin-912036eeb024`js::jit::ValueNumberer::run(this=0x00007fff5fbfb048, updateAliasAnalysis=<unavailable>) + 73 at ValueNumbering.cpp:1087
    frame #4: 0x00000001002c1437 js-dbg-opt-64-dm-nsprBuild-darwin-912036eeb024`js::jit::OptimizeMIR(mir=0x00000001021160d8) + 2327 at Ion.cpp:1431
(lldb)


We should also check that fc.out.js no longer asserts after this bug is fixed.

Comment 3

[Dan Gohman [:sunfish]]

Fortunately, this bug is not security sensitive. The assert is just asserting that we didn't miss an opportunity to optimize -- to delete an unused phi in this case.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Opening up as per comment 3.

Comment 5

[Dan Gohman [:sunfish]]

Attachment: gvn-dce-pinned-phi.patch

In a loop, walking the use-def chain from a phi operand in the loop header can lead us back around to other phis in the same loop header. The nextDef_ mechanism protects us from invalidating our iterator, but it was also causing us to leave unused phis sitting around, in cases such as the testcase above. The attached patch adds code to check for such cases and cleans them up.

It also fixes a harmless-in-practice dereference of an end iterator.

Comment 6

[Dan Gohman [:sunfish]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/59512894ddd4

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/59512894ddd4