Closed Bug 1034400 Opened 10 years ago Closed 10 years ago

Assertion failure: !v.isObject(), at jit/Recover.cpp

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: gkw, Assigned: bbouvier)

References

Details

(Keywords: assertion, regression, testcase)

Attachments

(2 files)

Attached file stack
function f(x, y) {
    !y != (y != Math.abs())
}
function g(x) {
    f(f(), (function() {}))
}
g(function(){})
g()

asserts js debug shell on m-c changeset 06e9a27a6fcc with --ion-eager --ion-offthread-compile=off at Assertion failure: !v.isObject(), at jit/Recover.cpp

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140630233053" and the hash "db4f6e215872".
The "bad" changeset has the timestamp "20140701004453" and the hash "881e7b3d101f".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=db4f6e215872&tochange=881e7b3d101f

Benjamin, is bug 1013821 a likely regressor?
Flags: needinfo?(benj)
Nice catch! So after discussion with nbp, the assertion is silly and we can just make canRecoverOnBailout return true in all cases, as the instruction isn't effectful (see MNot::getAliasSet).

Guillaume, this error is a fallout from the RNot bug. It's my fault as I didn't think of this particular case (look at the output of the iongraph if you want to see what goes wrong). As it's a really small patch to do, would you be interested in doing it? If so, please modify the code and add the testcase provided in this bug in your patch as well. Otherwise, I'll take care of it :)
Flags: needinfo?(benj) → needinfo?(layus.on)
(In reply to Benjamin Bouvier [:bbouvier] from comment #1)
> [...] the assertion is silly and we can just make canRecoverOnBailout 
> return true in all cases, as the instruction isn't effectful

Ahah ! So you finally surrender to my arguments :-)

> (see MNot::getAliasSet).

Huh ?

> Guillaume, this error is a fallout from the RNot bug. It's my fault as I
> didn't think of this particular case (look at the output of the iongraph if
> you want to see what goes wrong). As it's a really small patch to do, would
> you be interested in doing it? If so, please modify the code and add the
> testcase provided in this bug in your patch as well. Otherwise, I'll take
> care of it :)

I would be glad to work on this.
As I will be on holiday starting from Sunday (2014-07-06), feel free to fix it yourself if I was not able to do it before.
Flags: needinfo?(layus.on)
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 1dc6b294800d).
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:bisectfix]
Just ran bisection by hand:

The first good revision is:
changeset:   192441:0f85fbed35dc
user:        Hannes Verschore <hv1989@gmail.com>
date:        Fri Jul 04 19:43:24 2014 +0200
summary:     Bug 1019983 - Don't optimize compare based on baseline caches when more types are seen than present in the cache, r=jandem

So, this patch made the Compare a generic one (inputs are Values) while the Compare was specialized as a double compare before (the parameter y was converted into a double). As the compare is now generic, there is no bailout anymore. However, as the issue can still happen in other situations where a Compare isn't involved, I'll work on a patch.
Whiteboard: [jsbugmon:bisectfix]
Attached patch bug1034400.patchSplinter Review
With another baked test case that shows the issue.
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attachment #8451477 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8451477 [details] [diff] [review]
bug1034400.patch

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

::: js/src/jit-test/tests/ion/bug1034400.js
@@ +7,5 @@
> +    }
> +}
> +
> +var countH = 0;
> +function h() {

s/h/uceFault/
Attachment #8451477 - Flags: review?(nicolas.b.pierron) → review+
https://hg.mozilla.org/mozilla-central/rev/3708aa158f68
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: