1652732 - [warp] Assertion failure: !val.isMagic(), at vm/JSObject.cpp:3195 or Crash [@ js::BigIntObject::create(JSContext*, JS::Handle<JS::BigInt*>)]

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200712-22f5f7e91444 (debug build, run with --fuzzing-safe --ion-offthread-compile=off --warp --baseline-eager --ion-warmup-threshold=0):

(function(global) {
    global.makeIterator = function makeIterator(overrides) {};
})(this);
function test() {
    var iterable = {};
    iterable[{}] = makeIterator({});
    assertThrowsValue(function() {
        for (var x of iterable) {}
    }, 42);
}
test();

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0000555555c82134 in js::ToObjectSlowForPropertyAccess(JSContext*, JS::Handle<JS::Value>, int, JS::Handle<JS::Value>) ()
#1  0x0000555556396524 in js::jit::DoSetElemFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICSetElem_Fallback*, JS::Value*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) ()
#2  0x000025098f740b3d in ?? ()
[...]
#22 0x0000000000000000 in ?? ()
rax	0x555557061e41	93825020599873
rbx	0xfffe000000000000	-562949953421312
rcx	0x5555583ca840	93825040951360
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffae90	140737488334480
rsp	0x7fffffffae40	140737488334400
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f9bd40	140737353727296
r10	0x58	88
r11	0x7ffff6dac7a0	140737334921120
r12	0x7ffff6027000	140737320742912
r13	0x7ffff60731a8	140737321054632
r14	0xfffffffd	4294967293
r15	0x27	39
rip	0x555555c82134 <js::ToObjectSlowForPropertyAccess(JSContext*, JS::Handle<JS::Value>, int, JS::Handle<JS::Value>)+372>
=> 0x555555c82134 <_ZN2js29ToObjectSlowForPropertyAccessEP9JSContextN2JS6HandleINS2_5ValueEEEiS5_+372>:	movl   $0xc7b,0x0
   0x555555c8213f <_ZN2js29ToObjectSlowForPropertyAccessEP9JSContextN2JS6HandleINS2_5ValueEEEiS5_+383>:	callq  0x5555558492fe <abort>

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200714153520-bca48c382991.
The bug appears to have been introduced in the following build range:
> Start: 9dac3cf64ae60633df1b5668dd59872982baaf75 (20200630100415)
> End: 5efefa92861dbc5764f28d7a0ab6ee2dc3933c89 (20200630102017)
> Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9dac3cf64ae60633df1b5668dd59872982baaf75&tochange=5efefa92861dbc5764f28d7a0ab6ee2dc3933c89

Comment 3

[Jan de Mooij [:jandem]]

Simpler test:

function test() {
  var obj = {};
  obj[{}] = 1;
  f = () => { for (var x of obj) {} };
}
test();

This is a pre-existing scalar-replacement issue. WarpBuilder inserts a bailout for the SetElem and marks the operands as ImplicitlyUsed. For obj that's set on the MLoadFixedSlot from the call object. Then scalar replacement optimizes the call object allocation and replaces the MLoadFixedSlot with the MNewObject for obj, but this way we lose the ImplicitlyUsed flag that was set on MLoadFixedSlot.

Nicolas, should SR check for or propagate the ImplicitlyUsed flag (in this case it's set on LoadFixedSlot)?

Comment 4

[Nicolas B. Pierron [:nbp]]

ImplicitlyUsed flag is set to protect the value form being optimized out, such that it can be recovered as-is from the snapshot in case another code-path which is not present in the MIR would use this value.

Scalar Replacement uses an ObjectMemoryView to emulate the storage of an object.
Scalar Replacement creates MObjectState when the object is mutated and replaces MLoadFixedSlot instructions by the value extracted from the MObjectState [1].

The ImplicitlyUsed flag should probably be propagated from the MLoadFixedSlot to the value extracted from the MObjectState. The object allocation made with MNewObject does not need to have the ImplicitlyUsed flag set due to usage of one of its slots.

Making this logic part of replaceAllUsesWith would make sense, as this is not specific to Scalar Replacement.

[1] https://searchfox.org/mozilla-central/source/js/src/jit/ScalarReplacement.cpp#535-536

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1652732 - Propagate ImplicitlyUsed flag in justReplaceAllUsesWith. r?nbp!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35fd26e8cafc
Propagate ImplicitlyUsed flag in justReplaceAllUsesWith. r=nbp

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/35fd26e8cafc

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 11

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20200829091226-fdf95334aded.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.