Closed Bug 1117882 Opened 7 years ago Closed 7 years ago

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


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox37 --- affected
firefox38 --- fixed


(Reporter: azakai, Assigned: sunfish)



(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update,testComment=1,origRev=912036eeb024])


(2 files, 1 obsolete file)

Attached file fc.out.js (obsolete) —
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.
Component: JavaScript Engine → JavaScript Engine: JIT
(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:
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?
Blocks: 1029830
Severity: normal → critical
Flags: needinfo?(sunfish)
OS: Linux → All
Whiteboard: [jsbugmon:update,testComment=1,origRev=912036eeb024]
Version: unspecified → Trunk
Attached file 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 = '', 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

We should also check that fc.out.js no longer asserts after this bug is fixed.
Attachment #8544098 - Attachment is obsolete: true
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.
Opening up as per comment 3.
Group: core-security
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.
Assignee: nobody → sunfish
Flags: needinfo?(sunfish)
Attachment #8562158 - Flags: review?(nicolas.b.pierron)
Attachment #8562158 - Flags: review?(nicolas.b.pierron) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.