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

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

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

Comment 1

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

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.

Comment 2

[Christian Holler (:decoder)]

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

Comment 3

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

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

Comment 4

[Nicolas B. Pierron [:nbp]]

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

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Fix assumption that Ion stays enabled across warmup in ForkJoin.

Move the FastInvokeGuard inside the loop body so it recomputes useIon_.

Comment 6

[Niko Matsakis [:nmatsakis]]

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?

Comment 7

[Shu-yu Guo [:shu]]

(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 8

[Niko Matsakis [:nmatsakis]]

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?

Comment 9

[Shu-yu Guo [:shu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2fa44046af3

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c2fa44046af3