Closed Bug 1029440 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla33
Tracking Status
firefox32 --- unaffected
firefox33 --- verified
firefox-esr24 --- unaffected
firefox-esr31 --- unaffected

People

(Reporter: gkw, Assigned: shu)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(2 files, 1 obsolete file)

Attached file 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.
I'm bisecting overnight.
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Probable that this came in with (or was exposed by) the stack-capture-on-PJS-bailout work.
Whiteboard: [jsbugmon:] → [jsbugmon:update,bisect]
(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 :)
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
Flags: needinfo?(shu)
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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: nobody → shu
Status: NEW → ASSIGNED
Flags: needinfo?(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() && ...)
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
Closed: 10 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Group: javascript-core-security
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: