Closed
Bug 1259476
Opened 8 years ago
Closed 8 years ago
Assertion failure: data.s.payload.why == why, at dist/include/js/Value.h:1218
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: gkw, Unassigned)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])
Attachments
(1 file)
The following testcase crashes on mozilla-central revision 3381aa98edf7 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager --ion-pgo=on): try { x = evalcx(''); x.__proto__ = 0; } catch (e) {} (function() { for (var i = 0; i < 1; ++i) { if (i % 5 == 0) { for (let z of[0, 0, new Boolean(false), new Boolean(false), new Boolean(false), new Boolean(false)]) { this.x; } } } })() Backtrace: 0 js-dbg-64-dm-clang-darwin-3381aa98edf7 0x000000010047603e js::jit::ComputeGetPropResult(JSContext*, js::jit::BaselineFrame*, JSOp, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) + 990 (Value.h:1218) 1 js-dbg-64-dm-clang-darwin-3381aa98edf7 0x000000010045e188 js::jit::DoGetPropFallback(JSContext*, void*, js::jit::ICGetProp_Fallback*, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) + 3064 (SharedIC.cpp:2935) 2 ??? 0x0000000102ba5b77 0 + 4340734839
Reporter | ||
Comment 1•8 years ago
|
||
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/70094106841c user: Jan de Mooij date: Thu Jan 07 17:02:05 2016 +0100 summary: Bug 1237566 - Bake in global this-value in IonBuilder::jsop_functionthis in some cases. r=h4writer Jan, is bug 1237566 a likely regressor?
Blocks: 1237566
Flags: needinfo?(jdemooij)
Comment 2•8 years ago
|
||
I think --ion-pgo=on may be the real culprit. What's happening is that in the function's prologue, JSOP_FUNCTIONTHIS pushes MConstant(global). Then inside the loop, this.x is compiled as a constant (in getPropTryInferredConstant). Phi elimination gets rid of the phi for the |this| local slot and when we bail to Baseline, the this.x GETPROP sees a JS_OPTIMIZED_OUT MagicValue. nbp, any idea what's going on?
Flags: needinfo?(jdemooij) → needinfo?(nicolas.b.pierron)
Comment 3•8 years ago
|
||
This is a pgo only issue. IonBuilder do mark the Phi instruction has being used implicitly, but while looking if the operands of the phi nodes should have removed uses, we forgot to check the current node, and only looked for the uses of the current node. This patch makes sure that we check the current node, which implies that when we remove the MPhi which had the isImplicitlyUsed flag, we would have tagged its operand, which is still alive after the branch pruning optim. As the operand is flagged, EliminatePhis is not allowed to do any optimizations on the MPhi, and thus it is no longer replaced by an OptimizedOut magic constant. Thanks for the analysis from comment 2, this proved to be really valuable.
Attachment #8734844 -
Flags: review?(jdemooij)
Comment 4•8 years ago
|
||
Comment on attachment 8734844 [details] [diff] [review] Branch Pruning: Check if the Phi nodes have removed uses after popping them out of the worklist. Review of attachment 8734844 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Attachment #8734844 -
Flags: review?(jdemooij) → review+
Updated•8 years ago
|
Flags: needinfo?(nicolas.b.pierron)
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b6f0f273f2f7
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Reporter | ||
Updated•8 years ago
|
Summary: Assertion failure: data.s.payload.why == why, at /Users/skywalker/shell-cache/js-dbg-64-dm-clang-darwin-3381aa98edf7/objdir-js/dist/include/js/Value.h:1218 → Assertion failure: data.s.payload.why == why, at dist/include/js/Value.h:1218
Reporter | ||
Comment 7•8 years ago
|
||
Can we get this on to mozilla-aurora as well?
Flags: needinfo?(jdemooij)
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(jdemooij) → needinfo?(nicolas.b.pierron)
Comment 8•8 years ago
|
||
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #7) > Can we get this on to mozilla-aurora as well? Based on comment 3, I think this is not necessary, knowing that branch pruning is not enabled by default yet, and we won't enable it retroactively.
Flags: needinfo?(nicolas.b.pierron)
You need to log in
before you can comment on or make changes to this bug.
Description
•