Assertion failure: script_->hasBaselineScript(), at jit/IonFrames.cpp

VERIFIED FIXED in Firefox 33

Status

()

--
critical
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: gkw, Assigned: shu)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla33
x86_64
Linux
assertion, regression, sec-high, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox32 unaffected, firefox33 verified, firefox-esr24 unaffected, firefox-esr31 unaffected)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8445047 [details]
stack

Array.buildPar(3, function() {})
gczeal(10, 3)
Array.buildPar(9, function() {
    Array.prototype.unshift.call([], undefined)
})

asserts js debug shell on m-c changeset e86b84998b18 with --ion-eager --ion-offthread-compile=off at Assertion failure: script_->hasBaselineScript(), at jit/IonFrames.cpp

My configure flags are:

AR=ar sh /home/fuzz2lin/trees/mozilla-central/js/src/configure --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

Setting s-s and sec-high to be safe because this seems to involve gc.
(Reporter)

Comment 1

4 years ago
I'm bisecting overnight.
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 3

4 years ago
Probable that this came in with (or was exposed by) the stack-capture-on-PJS-bailout work.
(Reporter)

Comment 4

4 years ago
Is bug 1008332 likely related?
Whiteboard: [jsbugmon:] → [jsbugmon:update,bisect]

Comment 5

4 years ago
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #4)
> Is bug 1008332 likely related?

It could be.  There are two occurrences of hasBaselineScript in that file, both in that same function. (Though it's exceedingly unlikely that that OpenStreetMap is using PJS, much as we might like them to :)
(Reporter)

Comment 6

4 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/cd7125c33385
user:        Shu-yu Guo
date:        Fri Jun 20 18:39:14 2014 -0700
summary:     Bug 1019304 - Part 2: Overhaul PJS bailout mechanism to be like the normal bailout mechanism. (r=nmatsakis)

Shu-yu, is bug 1019304 also likely to be related?

(PJS-related, likely nightly-only)
Blocks: 1019304
status-firefox32: --- → unaffected
status-firefox33: --- → affected
Flags: needinfo?(shu)
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
(Assignee)

Comment 7

4 years ago
Created attachment 8445616 [details] [diff] [review]
Preserve all JIT code when there's recent parallel activity.

We try to preserve JIT code for recently active PJS scripts, but there's no way
to do so in the right order for inlined scripts, so we may end up collecting
the BaselineScripts of inlined scripts, which causes problems here when walking
the stack to dump debug info.

Currently we trying to be exact about which scripts to preserve. This patch
relaxes the preservation heuristic: if there's any recent parallel activity at
all, preserve all JIT code in the zone.
Attachment #8445616 - Flags: review?(terrence)
(Assignee)

Updated

4 years ago
Assignee: nobody → shu
Status: NEW → ASSIGNED
Flags: needinfo?(shu)
(Assignee)

Comment 8

4 years ago
Comment on attachment 8445616 [details] [diff] [review]
Preserve all JIT code when there's recent parallel activity.

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

::: js/src/jsgc.cpp
@@ +2975,5 @@
>      if (comp->lastAnimationTime + PRMJ_USEC_PER_SEC >= currentTime)
>          return true;
>  
> +#ifdef JS_ION
> +    if (!comp->jitCompartment() && comp->jitCompartment()->hasRecentParallelActivity())

Oops, that's a if (comp->jitCompartment() && ...)
(Assignee)

Comment 9

4 years ago
Created attachment 8445631 [details] [diff] [review]
Preserve all JIT code when there's recent parallel activity.

v2, fixed some bugs
Attachment #8445616 - Attachment is obsolete: true
Attachment #8445616 - Flags: review?(terrence)
Attachment #8445631 - Flags: review?(terrence)
Comment on attachment 8445631 [details] [diff] [review]
Preserve all JIT code when there's recent parallel activity.

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

Yes please: simpler is better until PJS code retention is an actual problem in the wild.

::: js/src/jit/IonCode.h
@@ +607,5 @@
>      }
>      uint32_t parallelAge() const {
>          return parallelAge_;
>      }
> +    uint32_t shouldPreserveParallelCode(bool increaseAge = false) {

Please make this bool parameter into a bool valued enum. Something like:

enum ShouldIncreaseAge {
    IncreaseAge = true,
    KeepAge = false
};
uint32_t shouldPreserveParallelCode(ShouldIncreaseAge increaseAge = KeepAge) {...}
Attachment #8445631 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/5240d1d1c1a0
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox33: affected → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Status: RESOLVED → VERIFIED
status-firefox33: fixed → verified
JSBugMon: This bug has been automatically verified fixed.
status-firefox-esr24: --- → unaffected
Group: javascript-core-security
status-firefox-esr31: --- → unaffected
Group: core-security
You need to log in before you can comment on or make changes to this bug.