Closed
Bug 1001378
Opened 10 years ago
Closed 10 years ago
Assertion failure: isObject(), at js/Value.h:1157 or Crash [@ aliasedVarFromArguments]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
Tracking | Status | |
---|---|---|
firefox30 | --- | unaffected |
firefox31 | --- | affected |
People
(Reporter: decoder, Assigned: shu)
References
Details
(4 keywords, Whiteboard: [jsbugmon:update])
Crash Data
Attachments
(2 files)
1.16 KB,
text/plain
|
Details | |
2.09 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
The following testcase asserts on mozilla-central revision 5ecd532a167e (run with --fuzzing-safe --ion-eager --ion-eager): function boo() { return foo.arguments[0]; } function foo(a,b,c) { if (a == 0) { a ^= ""; return boo(); } } function inlined() { return foo.apply({}, arguments); } inlined(1,2,3); inlined(0,2,3);
Reporter | ||
Comment 1•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Crash Signature: [@ aliasedVarFromArguments]
status-firefox31:
--- → affected
Keywords: crash
Whiteboard: [jsbugmon:update,bisect]
Reporter | ||
Updated•10 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Reporter | ||
Comment 2•10 years ago
|
||
JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: http://hg.mozilla.org/mozilla-central/rev/d34458e80bcb user: Shu-yu Guo date: Thu Apr 24 01:59:36 2014 -0700 summary: Bug 716647 - Part 1: Introduce JS_OPTIMIZED_OUT magic for optimized out slots and teach Debugger about them. (r=jandem) This iteration took 208.183 seconds to run.
Comment 3•10 years ago
|
||
Shu-yu, is bug 716647 a possible regressor?
Assignee | ||
Comment 4•10 years ago
|
||
Yep, part 1 is most likely the culprit. Sorry bug 716647 is setting off the fuzzer left and right, should've asked for fuzzing beforehand. :/
Flags: needinfo?(shu)
Assignee | ||
Comment 5•10 years ago
|
||
Existing bug. What's going on is that Ion is optimizing out unused argument slots. If you run a shell from rev d34458e80bcb~1, you can see that the second call to |inline| returns |undefined| with --ion-eager, but |0| without --ion-eager. Differential testing didn't hit this, I suppose. Function.arguments basically means a function's argument slots can never be optimized out of resume points. I hate this patch but I think our hands are tied here. Jan, is |uses->index()| the right way to get the frame slot here?
Attachment #8415756 -
Flags: review?(jdemooij)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → shu
Comment 6•10 years ago
|
||
Comment on attachment 8415756 [details] [diff] [review] Don't optimize out argument slots from resume points. Nicolas may be a better reviewer for this patch.
Attachment #8415756 -
Flags: review?(jdemooij) → review?(nicolas.b.pierron)
Comment 7•10 years ago
|
||
Comment on attachment 8415756 [details] [diff] [review] Don't optimize out argument slots from resume points. Review of attachment 8415756 [details] [diff] [review]: ----------------------------------------------------------------- Whoa, I am surprised that such bugs hit us only now. We should be able to make a test case for such problem where we return the value of arguments[0]. This should highlight a differential behavior prior to Bug 716647 as we used to replace the value by undefined instead of a magic value. ::: js/src/jit/IonAnalysis.cpp @@ +139,5 @@ > + // non-strict scripts, so we can't optimize out any arguments. > + if (!block->info().script()->strict()) { > + uint32_t slot = uses->index(); > + uint32_t firstArgSlot = block->info().firstArgSlot(); > + if (firstArgSlot <= slot && slot - firstArgSlot < block->info().nargs()) { nit: CompileInfo &info = block()->info(); Note, that this is safe to use the block info instead of the mrp->block()->info() because of the previous check which bails at doing such optimization if this resume point is part of a different block. Thus there is no issue with arguments of the outermost scripts.
Attachment #8415756 -
Flags: review?(nicolas.b.pierron) → review+
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #7) > Comment on attachment 8415756 [details] [diff] [review] > Don't optimize out argument slots from resume points. > > Review of attachment 8415756 [details] [diff] [review]: > ----------------------------------------------------------------- > > Whoa, I am surprised that such bugs hit us only now. > > We should be able to make a test case for such problem where we return the > value of arguments[0]. This should highlight a differential behavior prior > to Bug 716647 as we used to replace the value by undefined instead of a > magic value. Sure, will do. > > ::: js/src/jit/IonAnalysis.cpp > @@ +139,5 @@ > > + // non-strict scripts, so we can't optimize out any arguments. > > + if (!block->info().script()->strict()) { > > + uint32_t slot = uses->index(); > > + uint32_t firstArgSlot = block->info().firstArgSlot(); > > + if (firstArgSlot <= slot && slot - firstArgSlot < block->info().nargs()) { > > nit: CompileInfo &info = block()->info(); > > Note, that this is safe to use the block info instead of the > mrp->block()->info() because of the previous check which bails at doing such > optimization if this resume point is part of a different block. Thus there > is no issue with arguments of the outermost scripts. Good to know.
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/08f0167547ce
https://hg.mozilla.org/mozilla-central/rev/08f0167547ce
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•