Closed Bug 1080991 Opened 5 years ago Closed 5 years ago

Assertion failure: op->block()->dominates(resume->block()) (Resume point is not dominated by its operands), at jit/IonAnalysis.cpp

Categories

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

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox35 --- affected

People

(Reporter: gkw, Assigned: sunfish)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [fuzzblocker][jsbugmon:update])

Attachments

(3 files)

(function(){
    for (var i = 0; i < 8; i++) {
        var c = i
    }
})()

asserts js debug shell on m-c changeset 95d1486223f7 with --ion-eager --ion-loop-unrolling=on --no-threads at Assertion failure: op->block()->dominates(resume->block()) (Resume point is not dominated by its operands), at jit/IonAnalysis.cpp.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/628fabf04fda
user:        Dan Gohman
date:        Wed Oct 08 18:21:46 2014 -0700
summary:     Bug 1070258 - IonMonkey: Assert that resume points are dominated by their operands r=nbp

Dan, is bug 1070258 a possible regressor?
Flags: needinfo?(sunfish)
Whiteboard: [jsbugmon:update] → [fuzzblocker][jsbugmon:update]
Attached file stack
(lldb) bt 5
* thread #1: tid = 0xcaf2e, 0x00000001002b34af js-dbg-opt-64-dm-nsprBuild-darwin-95d1486223f7`js::jit::AssertExtendedGraphCoherency(js::jit::MIRGraph&) [inlined] AssertDominatorTree(graph=<unavailable>) + 13 at IonAnalysis.cpp:1948, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001002b34af js-dbg-opt-64-dm-nsprBuild-darwin-95d1486223f7`js::jit::AssertExtendedGraphCoherency(js::jit::MIRGraph&) [inlined] AssertDominatorTree(graph=<unavailable>) + 13 at IonAnalysis.cpp:1948
    frame #1: 0x00000001002b34a2 js-dbg-opt-64-dm-nsprBuild-darwin-95d1486223f7`js::jit::AssertExtendedGraphCoherency(graph=<unavailable>) + 3250 at IonAnalysis.cpp:2008
    frame #2: 0x0000000100277278 js-dbg-opt-64-dm-nsprBuild-darwin-95d1486223f7`js::jit::OptimizeMIR(mir=0x00000001028b5e40) + 2280 at Ion.cpp:1642
    frame #3: 0x0000000100279a47 js-dbg-opt-64-dm-nsprBuild-darwin-95d1486223f7`js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*, bool, js::ExecutionMode) [inlined] js::jit::CompileBackEnd(mir=0x00000001028b5fa8, aRhs=<unavailable>) + 42 at Ion.cpp:1836
    frame #4: 0x0000000100279a1d js-dbg-opt-64-dm-nsprBuild-darwin-95d1486223f7`js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*, bool, js::ExecutionMode) [inlined] js::jit::IonCompile(script=<unavailable>, baselineFrame=<unavailable>, executionMode=SequentialExecution) + 758 at Ion.cpp:2126
(lldb)
Another instance of bug 1055690. Discarded operands are getting left in resume points. I left a note in bug 1055690 mentioning this bug, and included a comment in this bug, so when the underlying bug gets fixed, we can re-enable the assert.
Assignee: nobody → sunfish
Attachment #8503317 - Flags: review?(nicolas.b.pierron)
Flags: needinfo?(sunfish)
This patch should fix the current issue without removing the failing
assertion.
Attachment #8504191 - Flags: review?(sunfish)
Comment on attachment 8503317 [details] [diff] [review]
ignore-discarded-resumepoint-operands.patch

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

I think this is better if we can remove such corner cases and start having stringer invariants,
Attachment #8503317 - Flags: review?(nicolas.b.pierron)
(In reply to Nicolas B. Pierron [:nbp] from comment #4)
> I think this is better if we can remove such corner cases and start having
> stringer invariants,

s/stringer/stronger/
Comment on attachment 8504191 [details] [diff] [review]
Replace unused Phis by an optimized-out constant.

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

Cool! Yes, I agree this is a better way. Can this make the isDiscarded() check in releaseResumePointOperands unnecessary too?
Attachment #8504191 - Flags: review?(sunfish) → review+
(In reply to Dan Gohman [:sunfish] from comment #6)
> Cool! Yes, I agree this is a better way. Can this make the isDiscarded()
> check in releaseResumePointOperands unnecessary too?

So far this seems to be the case, so I turned the condition into an assertion.
(In reply to Nicolas B. Pierron [:nbp] from comment #7)
> (In reply to Dan Gohman [:sunfish] from comment #6)
> > Cool! Yes, I agree this is a better way. Can this make the isDiscarded()
> > check in releaseResumePointOperands unnecessary too?
> 
> So far this seems to be the case, so I turned the condition into an
> assertion.

Correction, this does not seems to work with parallel test cases yet. So I will investigate that later and land the current patch.
https://hg.mozilla.org/mozilla-central/rev/7d5bc73600d2
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.