Closed Bug 1322932 Opened 5 years ago Closed 5 years ago

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


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- fixed


(Reporter: gkw, Assigned: h4writer)



(4 keywords, Whiteboard: [fuzzblocker][jsbugmon:update])


(2 files)

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 {


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)

For detailed crash information, see attachment.

This testcase happens quite often and also crashes opt builds [@ js::jit::BacktrackingAllocator::resolveControlFlow].
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
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]
Attached patch PatchSplinter Review
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)
Forgot to mention bug 733353 introduced this typo.
Comment on attachment 8818497 [details] [diff] [review]

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

Ouch, excellent catch.
Attachment #8818497 - Flags: review?(jdemooij) → review+
Pushed by
IonMonkey - Only iterate the backedge of the inner-loop when it has already be visited, r=jandem
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.