Closed Bug 1289610 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- wontfix
firefox51 --- fixed
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: gkw, Assigned: jandem)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase, Whiteboard: [jsbugmon:])

Attachments

(4 files)

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.
Attached file Testcase
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Error: Failed to isolate test from comment
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)
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)
(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)
(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)
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)
Jan, what's next here?
Flags: needinfo?(jdemooij)
Note to self: this doesn't repro with --without-intl-api.
Attached patch PatchSplinter Review
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+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a6b99783596
Don't discard Baseline code in ~AutoClearTypeInferenceStateOnOOM. r=bhackett
Flags: needinfo?(jdemooij)
https://hg.mozilla.org/mozilla-central/rev/3a6b99783596
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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?
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+
Duplicate of this bug: 1316565
You need to log in before you can comment on or make changes to this bug.