Assertion failure: jit::IsIonEnabled(cx), at jit/Ion.cpp

RESOLVED FIXED in mozilla28

Status

()

Core
JavaScript Engine: JIT
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gkw, Assigned: shu)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla28
x86_64
Mac OS X
assertion, regression, testcase
Points:
---

Firefox Tracking Flags

(firefox28 affected)

Details

(Whiteboard: [jsbugmon:])

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 8334182 [details]
lldb stack

ParallelArray(8, function() {
    return setJitCompilerOption('ion.enable', 0)
})

asserts js debug shell on m-c changeset f2adb62d07eb without any CLI arguments at Assertion failure: jit::IsIonEnabled(cx), at jit/Ion.cpp

My configure flags are:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --with-ccache --enable-threadsafe <other NSPR options>
(Reporter)

Comment 1

4 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/666d2f3254ab
user:        masaya iseki
date:        Tue Oct 22 17:47:00 2013 +0100
summary:     Bug 909997 - Add JS compiler options at runtime to expand differential testing; r=nbp"

Not sure if this is correct, this might be a pre-existing bug exposed by bug 909997. Since this involves ParallelArrays, setting needinfo? from Shu-yu.
status-firefox28: --- → affected
Flags: needinfo?(shu)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
(Reporter)

Comment 3

4 years ago
(In reply to Christian Holler (:decoder) from comment #2)
> JSBugMon: Cannot process bug: Unable to automatically reproduce, please
> track manually.

This needs a threadsafe shell to reproduce.
(In reply to Gary Kwong [:gkw] [:nth10sd] (yes, still catching up on bugmail) from comment #1)
> autoBisect shows this is probably related to the following changeset:
> 
> The first bad revision is:
> summary:     Bug 909997 - Add JS compiler options at runtime to expand
> differential testing; r=nbp"
> 
> Not sure if this is correct, this might be a pre-existing bug exposed by bug
> 909997. Since this involves ParallelArrays, setting needinfo? from Shu-yu.

Shu: Note that setJitCompilerOption is changing the flag at the time of the call instead of changing it at the end of the JS execution, as the about:config flag will do.  So the issue might be that the ForkSlice is reading the state of the flag when it is created and select a path which depends on Ion activation, while the function with the ForkSlice might change the state.

Still I wonder if this cannot be replicated as preferences might be changed during a call to an interruption callback.
(Assignee)

Comment 5

4 years ago
Created attachment 8341191 [details] [diff] [review]
Fix assumption that Ion stays enabled across warmup in ForkJoin.

Move the FastInvokeGuard inside the loop body so it recomputes useIon_.
Attachment #8341191 - Flags: review?(nmatsakis)
(Assignee)

Updated

4 years ago
Assignee: general → shu
Status: NEW → ASSIGNED
(Assignee)

Updated

4 years ago
Flags: needinfo?(shu)
Comment on attachment 8341191 [details] [diff] [review]
Fix assumption that Ion stays enabled across warmup in ForkJoin.

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

What is the point of using FastInvokeGuard in the first place, then?
(Assignee)

Comment 7

4 years ago
(In reply to Niko Matsakis [:nmatsakis] from comment #6)
> Comment on attachment 8341191 [details] [diff] [review]
> Fix assumption that Ion stays enabled across warmup in ForkJoin.
> 
> Review of attachment 8341191 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> What is the point of using FastInvokeGuard in the first place, then?

Well I assume it's still faster than Invoke even if it's reconstructed per loop iter. Could also add a new method to update the ionEnabled flag.
Comment on attachment 8341191 [details] [diff] [review]
Fix assumption that Ion stays enabled across warmup in ForkJoin.

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

What is the point of using FastInvokeGuard in the first place, then?
Attachment #8341191 - Flags: review?(nmatsakis) → review+
https://hg.mozilla.org/mozilla-central/rev/c2fa44046af3
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.