Assertion failure: hasBaselineScript(), at js/src/jsscript.h:1590

RESOLVED FIXED in Firefox 51

Status

()

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

People

(Reporter: gkw, Assigned: jandem)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

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

Firefox Tracking Flags

(firefox50 wontfix, firefox51 fixed, firefox52 fixed, firefox53 fixed)

Details

(Whiteboard: [jsbugmon:])

Attachments

(4 attachments)

(Reporter)

Description

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

See attachment.

Backtrace:

0   js-dbg-32-dm-clang-darwin-e0bc88708ffe	0x00a29734 JSScript::baselineScript() const + 100 (jsscript.h:1590)
1   js-dbg-32-dm-clang-darwin-e0bc88708ffe	0x00888672 JSScript::setIonScript(JSRuntime*, js::jit::IonScript*) + 34 (BaselineJIT.h:481)
2   js-dbg-32-dm-clang-darwin-e0bc88708ffe	0x003e9dcd js::jit::CodeGenerator::link(JSContext*, js::CompilerConstraintList*) + 1789 (CodeGenerator.cpp:9357)
3   js-dbg-32-dm-clang-darwin-e0bc88708ffe	0x004ac910 LinkCodeGen(JSContext*, js::jit::IonBuilder*, js::jit::CodeGenerator*) + 256 (Ion.cpp:538)
4   js-dbg-32-dm-clang-darwin-e0bc88708ffe	0x004419f8 js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*, bool, bool) + 4664 (Ion.cpp:2306)
5   js-dbg-32-dm-clang-darwin-e0bc88708ffe	0x004403c8 js::jit::CanEnter(JSContext*, js::RunState&) + 392 (Ion.cpp:2557)
6   js-dbg-32-dm-clang-darwin-e0bc88708ffe	0x009ed64a js::RunScript(JSContext*, js::RunState&) + 378 (Interpreter.cpp:376)
/snip

For detailed crash information, see attachment.
(Reporter)

Comment 1

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

Comment 2

2 years ago
Created attachment 8774914 [details]
Testcase
(Reporter)

Comment 3

2 years ago
Created attachment 8774915 [details]
OOM_VERBOSE=1 stack from m-c rev e0bc88708ffe

Updated

2 years ago
Whiteboard: [jsbugmon:update] → [jsbugmon:]

Comment 4

2 years ago
JSBugMon: Cannot process bug: Error: Failed to isolate test from comment
(Reporter)

Comment 5

2 years ago
The testcase is largely unreduced, my usual tricks fail after trying awhile, so since this is an oomTest bug (the last generated line when run involves oomTest), setting needinfo? from Jon as a start.

I might also be able to get a regression window.
Flags: needinfo?(jcoppeard)
(Reporter)

Comment 6

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/d4cf63e47ae9
user:        Till Schneidereit
date:        Thu Jul 21 00:44:16 2016 +0200
summary:     Bug 911216 - Part 30: Enable SpiderMonkey Promise implementation. r=bz,efaust,bholley,Paolo,tromey,shu

Till, perhaps this might be related to Promises?
Flags: needinfo?(till)
Running the testcase locally I get an error about a missing file:

Error: can't open /Users/skywalker/trees/mozilla-central/js/src/tests/js1_5/extensions/regress-385393-09.js: No such file or directory

The stack suggests it's an OOM handling bug in type inference or Ion compilation though.
Flags: needinfo?(jcoppeard)
(Reporter)

Comment 8

2 years ago
(In reply to Jon Coppeard (:jonco) from comment #7)
> Running the testcase locally I get an error about a missing file:

Sorry, I forgot to mention to change the path to point to your m-c tree (ideally at the same rev) at the top of the testcase (first line).
Flags: needinfo?(jcoppeard)
(Reporter)

Comment 9

2 years ago
(In reply to Jon Coppeard (:jonco) from comment #7)
> The stack suggests it's an OOM handling bug in type inference or Ion
> compilation though.

Please feel free to bounce the needinfo? to :jandem as an alternative.
I can't reproduce this. Even if I could, I agree with Jon: the stack indicates something in TI or Ion. Enabling SpiderMonkey Promise might trigger this, but is somewhat unlikely to be responsible itself.
Flags: needinfo?(till)
We're hitting OOM while lazily sweeping type information inside ion builder.  Then later on we crash while trying to link.  Is the script null or is something else happening?  I'm not sure what's going on here.

jandem will have a better idea from the JIT point of view.
Flags: needinfo?(jcoppeard) → needinfo?(jdemooij)
(Assignee)

Comment 12

2 years ago
The problem is that ~AutoEnterAnalysis discards JIT code on OOM when sweeping failed, so in this case we no longer have a Baseline script when Ion compilation finishes.

I'm not sure how best to handle this - all places that use AutoEnterAnalysis could check if we hit an OOM, but that feels hackish :(
Flags: needinfo?(jdemooij) → needinfo?(bhackett1024)
Maybe this is kicking the can down the road but ~AutoEnterAnalysis could just discard Ion code instead of all JIT code when sweeping fails.  Baseline scripts do not depend on the type constraints that might have been invalidated when the OOM occurred.
Flags: needinfo?(bhackett1024)
(Reporter)

Comment 14

2 years ago
Jan, what's next here?
Flags: needinfo?(jdemooij)
(Assignee)

Comment 15

2 years ago
Note to self: this doesn't repro with --without-intl-api.
(Assignee)

Comment 16

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

Here's a patch to make AutoClearTypeInferenceStateOnOOM only discard Ion code.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8810343 - Flags: review?(bhackett1024)
Attachment #8810343 - Flags: review?(bhackett1024) → review+

Comment 17

2 years ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a6b99783596
Don't discard Baseline code in ~AutoClearTypeInferenceStateOnOOM. r=bhackett
(Assignee)

Updated

2 years ago
Flags: needinfo?(jdemooij)

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3a6b99783596
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Assignee)

Comment 20

2 years ago
Comment on attachment 8810343 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Older bug.
[User impact if declined]: Cashes or assertion failures on OOM.
[Describe test coverage new/current, TreeHerder]: Fixes the test the fuzzer found.
[Risks and why]: Low risk. This has been in a few Nightlies.
[String/UUID change made/needed]: None.
Flags: needinfo?(jdemooij)
Attachment #8810343 - Flags: approval-mozilla-beta?
Attachment #8810343 - Flags: approval-mozilla-aurora?

Updated

2 years ago
status-firefox51: --- → affected
status-firefox52: --- → affected
Comment on attachment 8810343 [details] [diff] [review]
Patch

Fix an assertion failure. Beta51+ and Aurora52+. Should be in 51 beta 3.
Attachment #8810343 - Flags: approval-mozilla-beta?
Attachment #8810343 - Flags: approval-mozilla-beta+
Attachment #8810343 - Flags: approval-mozilla-aurora?
Attachment #8810343 - Flags: approval-mozilla-aurora+

Comment 22

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/10a8d1be0861
status-firefox52: affected → fixed

Comment 23

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/2df930d5262a
status-firefox51: affected → fixed
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1316565
status-firefox50: affected → wontfix
You need to log in before you can comment on or make changes to this bug.