Assertion failure: hasIonScript(), at js/src/jsscript.h:1417

RESOLVED FIXED in Firefox 53

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: gkw, Assigned: jandem)

Tracking

(Blocks: 1 bug, {assertion, jsbugmon, testcase})

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

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments)

(Reporter)

Description

a year ago
The following testcase crashes on mozilla-central revision 2963cf6be7f8 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads):

var recursiveFunctions = [{
    text: "(function too_much_recursion(depth) { @; if (depth > 0) { @; too_much_recursion(depth - 1); @ } else { @ } @ })",
}, {
    text: "(function factorial(N) { @; if (N == 0) { @; return 1; } @; return N * factorial(N - 1); @ })",
}, {
    text: "(function factorial_tail(N, Acc) { @; if (N == 0) { @; return Acc; } @; return factorial_tail(N - 1, Acc * N); @ })",
}, {
    text: "(function a_indexing(array, start) { @; if (array.length == start) { @; return EXPR1; } var thisitem = array[start]; var recval = a_indexing(array, start + 1); STATEMENT1 })",
    testSub: function(text) {
        return text.replace(/EXPR1/, "0").replace(/STATEMENT1/, "return thisitem + recval;");
    }
}, {
    text: "(function sum_indexing(array, start) { @; return array.length == start ? 0 : array[start] + sum_indexing(array, start + 1); })",
}];
for (var i = 0; i < recursiveFunctions.length; ++i) {
    var a = recursiveFunctions[i];
    var text = a.text;
    if (a.testSub) {
        text = a.testSub(text);
    }
    var f = eval(text.replace(/@/g, ""));
}
function tryItOut(code) {
    deL = code.replace(/s/).replace(/s/);
    deL.match(/s/)
    deL.match(/s/)
    deL.match(/s/)
    code.match()
    code.replace(/s/, "");
    f = new Function(code);
    f()
}
tryItOut("''.split()");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("");
tryItOut("oomTest(()=>{'use asm';''.split()})")


Backtrace:

0   js-dbg-64-dm-clang-darwin-2963cf6be7f8	0x0000000107211e1c js::jit::IonIC::attachCacheIRStub(JSContext*, js::jit::CacheIRWriter const&, js::jit::CacheKind, JS::Handle<JSScript*>) + 1228 (jsscript.h:1417)
1   js-dbg-64-dm-clang-darwin-2963cf6be7f8	0x0000000107236774 js::jit::IonGetPropertyIC::update(JSContext*, JS::Handle<JSScript*>, js::jit::IonGetPropertyIC*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) + 468 (IonIC.cpp:143)
2   ???                           	0x00000001091f2331 0 + 4448002865
3   js-dbg-64-dm-clang-darwin-2963cf6be7f8	0x00000001071a3eea js::jit::IonCannon(JSContext*, js::RunState&) + 874 (Ion.cpp:2895)
4   js-dbg-64-dm-clang-darwin-2963cf6be7f8	0x0000000107032217 js::RunScript(JSContext*, js::RunState&) + 359 (Interpreter.cpp:383)
5   js-dbg-64-dm-clang-darwin-2963cf6be7f8	0x000000010704365d js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) + 621 (Interpreter.cpp:475)
/snip

For detailed crash information, see attachment.
(Reporter)

Comment 1

a year ago
Created attachment 8825700 [details]
Detailed Crash Information
(Reporter)

Comment 2

a year ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/547a8c1acff1
user:        Jan de Mooij
date:        Fri Dec 23 08:34:03 2016 +0100
summary:     Bug 1322093 part 19 - Enable the new IonGetPropertyIC that's based on CacheIR. r=h4writer

Jan, is bug 1322093 a likely regressor?
Blocks: 1322093
Flags: needinfo?(jdemooij)
(Assignee)

Comment 3

a year ago
Created attachment 8825780 [details] [diff] [review]
Patch

The IR generator can trigger GC (in ValueToNameOrSymbolId in this case) and GC can invalidate the IonScript, so we just need to check the script still has an IonScript.

The testcase is pretty large and will probably be fragile to test this bug, so I didn't add it.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8825780 - Flags: review?(hv1989)
Comment on attachment 8825780 [details] [diff] [review]
Patch

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

Good find!
Attachment #8825780 - Flags: review?(hv1989) → review+

Comment 5

a year ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8192f69e1a0
Make sure the script still has an IonScript before attaching a stub. r=h4writer

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b8192f69e1a0
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.