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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

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].

Comment 1

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

Attachment: Detailed Crash Information

Comment 2

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

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?

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: 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.)

Comment 4

[Hannes Verschore [:h4writer]]

Forgot to mention bug 733353 introduced this typo.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8818497 [details] [diff] [review]
Patch

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

Ouch, excellent catch.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f9fafc43d7c7