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

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: 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.

Comment 1

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

I'm bisecting overnight.

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 3

[Lars T Hansen [:lth]]

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

Comment 4

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

Is bug 1008332 likely related?

Comment 5

[Lars T Hansen [:lth]]

(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 :)

Comment 6

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

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)

Comment 7

[Shu-yu Guo [:shu]]

Attachment: 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.

Comment 8

[Shu-yu Guo [:shu]]

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() && ...)

Comment 9

[Shu-yu Guo [:shu]]

Attachment: Preserve all JIT code when there's recent parallel activity.

v2, fixed some bugs

Comment 10

[Terrence Cole [: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) {...}

Comment 11

[Shu-yu Guo [:shu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5240d1d1c1a0

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5240d1d1c1a0

Comment 13

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.