Crash [@ js::jit::BacktrackingAllocator::resolveControlFlow] or Assertion failure: pred->isLoopBackedge(), at js/src/jit/IonAnalysis.cpp:2789

RESOLVED FIXED in Firefox 53

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gkw, Assigned: h4writer)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla53
x86_64
Mac OS X
assertion, crash, jsbugmon, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 unaffected, firefox52 unaffected, firefox53 fixed)

Details

(Whiteboard: [fuzzblocker][jsbugmon:update])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
The following testcase crashes on mozilla-central revision c51e7406d7b2 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager):

(function() {
    for (var i = 0; i < 4; ++i) {
        if (i % 3 == 0) {
            for (var x in y) {}
        } else {
            continue;
        }
    }
})()


Backtrace:

0   js-dbg-64-dm-clang-darwin-c51e7406d7b2	0x000000010f97522b js::jit::AssertGraphCoherency(js::jit::MIRGraph&) + 411 (IonAnalysis.cpp:2789)
1   js-dbg-64-dm-clang-darwin-c51e7406d7b2	0x000000010f976253 js::jit::AssertExtendedGraphCoherency(js::jit::MIRGraph&, bool) + 51 (InlineList.h:284)
2   js-dbg-64-dm-clang-darwin-c51e7406d7b2	0x000000010f9709a9 js::jit::OptimizeMIR(js::jit::MIRGenerator*) + 7769 (MIRGenerator.h:115)
3   js-dbg-64-dm-clang-darwin-c51e7406d7b2	0x000000010f97c2da js::jit::CompileBackEnd(js::jit::MIRGenerator*) + 74 (Ion.cpp:2023)
4   js-dbg-64-dm-clang-darwin-c51e7406d7b2	0x000000010f97e3d9 js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*, bool) + 3961 (Ion.cpp:2304)
/snip

For detailed crash information, see attachment.

This testcase happens quite often and also crashes opt builds [@ js::jit::BacktrackingAllocator::resolveControlFlow].
(Reporter)

Comment 1

2 years ago
Created attachment 8817917 [details]
Detailed Crash Information
(Reporter)

Comment 2

2 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b877875abb3a
user:        Hannes Verschore
date:        Fri Dec 09 14:14:37 2016 -1000
summary:     Bug 1322724: IonMonkey - Add the hit count information on the extra false branch blocks, r=jandem

Hannes, is bug 1322724 a likely regressor?
Blocks: 1322724
Flags: needinfo?(hv1989)
Keywords: crash
Summary: Assertion failure: pred->isLoopBackedge(), at js/src/jit/IonAnalysis.cpp:2789 → Crash [@ js::jit::BacktrackingAllocator::resolveControlFlow] or Assertion failure: pred->isLoopBackedge(), at js/src/jit/IonAnalysis.cpp:2789
Whiteboard: [jsbugmon:update] → [fuzzblocker][jsbugmon:update]
(Assignee)

Comment 3

2 years ago
Created attachment 8818497 [details] [diff] [review]
Patch

A typo that existed for 3 years in MarkLoopBlocks that happened to work before due to circumstances.

Due to this typo the condition was always true and we would always back up and iterate the innerBackedge again.

In the case this was not needed this wasn't an issue because the successor (that exits the loop) would always be the "backedge->id() + 1" (if it exists). As a result this was a NOP.

Fixed the typo to actually test if we visited the innerBackedge already and only backtrack if needed. Instead of forwarding to innerBackedge and skipping iterating some blocks.

(The IonBuilder.cpp changes does the same as before, only makes sure the id is already correct. Which isn't an issue, because we renumber the ids before using the ids. But it is just nice the have the ids in RPO in IonBuilder.)
Assignee: nobody → hv1989
Flags: needinfo?(hv1989)
Attachment #8818497 - Flags: review?(jdemooij)
(Assignee)

Comment 4

2 years ago
Forgot to mention bug 733353 introduced this typo.
Comment on attachment 8818497 [details] [diff] [review]
Patch

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

Ouch, excellent catch.
Attachment #8818497 - Flags: review?(jdemooij) → review+

Comment 6

2 years ago
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9fafc43d7c7
IonMonkey - Only iterate the backedge of the inner-loop when it has already be visited, r=jandem

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f9fafc43d7c7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
status-firefox51: --- → unaffected
status-firefox52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.