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)
Tracking
()
RESOLVED
FIXED
mozilla33
People
(Reporter: gkw, Assigned: bbouvier)
References
Details
(Keywords: assertion, regression, testcase)
Attachments
(2 files)
2.51 KB,
text/plain
|
Details | |
2.36 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
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)
Assignee | ||
Comment 1•10 years ago
|
||
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)
Comment 2•10 years ago
|
||
(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)
Updated•10 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
Comment 3•10 years ago
|
||
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 1dc6b294800d).
Updated•10 years ago
|
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:bisectfix]
Assignee | ||
Comment 4•10 years ago
|
||
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]
Assignee | ||
Comment 5•10 years ago
|
||
With another baked test case that shows the issue.
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attachment #8451477 -
Flags: review?(nicolas.b.pierron)
Comment 6•10 years ago
|
||
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+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3708aa158f68
Flags: in-testsuite+
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.
Description
•