Closed Bug 1259476 Opened 4 years ago Closed 4 years ago

Assertion failure: data.s.payload.why == why, at dist/include/js/Value.h:1218

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: gkw, Unassigned)

References

(Blocks 1 open bug)

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
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)
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)
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 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+
Flags: needinfo?(nicolas.b.pierron)
https://hg.mozilla.org/mozilla-central/rev/b6f0f273f2f7
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
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
Can we get this on to mozilla-aurora as well?
Flags: needinfo?(jdemooij)
Flags: needinfo?(jdemooij) → needinfo?(nicolas.b.pierron)
(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.