1520998 - Crash [@ js::jit::UnrollLoops]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

The following testcase crashes on mozilla-central revision 081c6ac45c5d (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion --ion-loop-unrolling=on):

// Adapted from randomly chosen test: js/src/jit-test/tests/ion/lazyLink-bug1150783.js
(function() {
    'use asm';
    function f() {
        var $x = 0;
        while (($x | 0) < 0) {
            $x = $x + 1 | 0;
        }
    }
    return f;
})()

Backtrace:

#0 js::jit::UnrollLoops (graph=..., bounds=...) at js/src/jit/LoopUnroller.cpp:451
#1 0x000055e4ff623e01 in js::jit::OptimizeMIR (mir=0x7ffc54704388) at js/src/jit/Ion.cpp:1478
#2 0x000055e4ff8acb5f in js::wasm::IonCompileFunctions (env=..., lifo=..., inputs=..., code=<optimized out>, dvs=..., error=0x0) at js/src/wasm/WasmIonCompile.cpp:4078
#3 0x000055e4ff89e1fc in ExecuteCompileTask (task=0x7fa92b288800, error=0x0) at js/src/wasm/WasmGenerator.cpp:718
#4 0x000055e4ff89eca7 in js::wasm::ModuleGenerator::locallyCompileCurrentTask (this=<optimized out>) at js/src/wasm/WasmGenerator.cpp:757
#5 js::wasm::ModuleGenerator::finishFuncDefs (this=0x7ffc547046f8) at js/src/wasm/WasmGenerator.cpp:884
#6 0x000055e4ff86d496 in ModuleValidator<char16_t>::finish (this=0x7ffc547055b0, linkData=0x7ffc54705de0) at js/src/wasm/AsmJS.cpp:2176
/snip

For detailed crash information, see attachment.

Not sure if this is s-s. --ion-loop-unrolling=on is required, but then again, MIR is on the stack, so locking s-s to be safe.

Comment 1

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

Attachment: Detailed Crash Information

Comment 2

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

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/e68c233270e3
user: Jan de Mooij
date: Tue Jan 15 15:35:46 2019 +0000
summary: Bug 1520183 - Add a script flag for JSScript::trackRecordReplayProgress(). r=bhackett

Jan, is bug 1520183 a likely regressor?

Comment 3

[Jan de Mooij [:jandem]]

This is a pre-existing issue, it would have crashed when recording/replaying with --ion-loop-unrolling=on before.

But doesn't affect anything because unrolling is disabled.

Comment 4

[Nicolas B. Pierron [:nbp]]

Should we remove the loop unrolling code?

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Nicolas B. Pierron [:nbp] from comment #4)

Should we remove the loop unrolling code?

Good idea. I think it's time to remove this now. It has been unused for years and if we ever need it we can look at the current code in hg/git.

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1520998 - Remove Ion LoopUnroller code. r?nbp!

It was added more than 4 years ago but never got enabled due to benchmark
regressions. Websites can now use WebAssembly for hot code (and the plan is to
use Cranelift instead of Ion there) so it's unlikely we will need this soon.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0cbcad902d49
Remove Ion LoopUnroller code. r=nbp

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/0cbcad902d49