1709328 - Assertion failure: cx_->hadNondeterministicException(), at jit/WarpOracle.cpp:190

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20210426-6f8320a4798f (debug build, run with --fuzzing-safe --ion-offthread-compile=off --fast-warmup --ion-warmup-threshold=0 --blinterp-eager):

b = [
  Float64Array,
  Float32Array,
  Int32Array,
  Int16Array,
  Int8Array,
  Uint32Array,
  Uint16Array,
  Uint8Array,
  Uint8ClampedArray
]
function c(d) {
    constructors = b
    for (e = 0;; ++e) {
        constructor = constructors[e]
        try {
            d(constructor)
        } catch {}
    }
}
f = [
  "", 
  "1000000000000000000000", 
  "0.0000001"
]
c(function(TA) {
    f.forEach(function(g) {
        h = new TA;
        Reflect.get(h, g, "es3000");
    })
})

Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000555557945a13 in js::jit::WarpOracle::createSnapshot() ()
#0  0x0000555557945a13 in js::jit::WarpOracle::createSnapshot() ()
#1  0x00005555578e1716 in js::jit::CreateWarpSnapshot(JSContext*, js::jit::MIRGenerator*, JS::Handle<JSScript*>) ()
#2  0x00005555578c104f in js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*) ()
#3  0x00005555578c1dd7 in IonCompileScriptForBaseline(JSContext*, js::jit::BaselineFrame*, unsigned char*) ()
[...]
#13 0x0000000000000000 in ?? ()
rax	0x55555572bcf5	93824994163957
rbx	0x7ffff6026000	140737320738816
rcx	0x555558023238	93825037120056
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffb240	140737488335424
rsp	0x7fffffffb1a0	140737488335264
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99840	140737353717824
r10	0x0	0
r11	0x0	0
r12	0x7ffff4a614a0	140737297912992
r13	0x7fffffffb260	140737488335456
r14	0x7ffff604e510	140737320903952
r15	0xbdb6f2b3	3182883507
rip	0x555557945a13 <js::jit::WarpOracle::createSnapshot()+1635>
=> 0x555557945a13 <_ZN2js3jit10WarpOracle14createSnapshotEv+1635>:	movl   $0xbe,0x0
   0x555557945a1e <_ZN2js3jit10WarpOracle14createSnapshotEv+1646>:	callq  0x555556a8553e <abort>

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Nicolas B. Pierron [:nbp] {away}]

Jon, could this be related with the template object removal?

Comment 3

[Jon Coppeard (:jonco)]

Bisection points at:

changeset: 562477:6b0fac1de385
user: Iain Ireland <[e-mail]>
date: Wed Jan 06 16:38:48 2021 +0000
summary: Bug 1673497: Part 22: Don't bail on -0 for GuardToInt32Index r=jandem

Comment 4

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210504092024-3aa883d4b622.
The bug appears to have been introduced in the following build range:

Start: f4af0087a1b49c221f54143a10b7bebca35db49c (20210111195436)
End: febd0fad07331284c49334bab4d9c653f2c80275 (20210111195806)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f4af0087a1b49c221f54143a10b7bebca35db49c&tochange=febd0fad07331284c49334bab4d9c653f2c80275

Comment 5

[Iain Ireland [:iain]]

This was a journey.

The underlying problem here isn't a bailout loop. It's that our GetElemSuper ICs in baseline have been silently broken for three years.

In bug 1472211, we reordered the operands of GetElemSuper. However, it looks like we missed one change: we fixed the order of the arguments to DoGetElemSuperFallback, but we did not fully update the register assignments. In BaselineCodeGen::emit_GetElemSuper, we pass obj on the stack, and the receiver and index in R0 and R1 respectively. GetPropIRGenerator::tryAttachStub uses operand 0 for obj, operand 1 for the index, and operand 2 for the receiver. But in BaselineCacheIRCompiler::init, input location 0 (obj) is on the stack, input location 1 (index) is R0, and input location 2 is R1 (receiver). This swaps the index and the receiver.

Most of the time, this just causes the IC to fail, either because we are guarding that the receiver is an object, or because we are calling into GetNativeDataPropertyByValuePure with an object as the id and failing. However, using Reflect.get, we can have a GetElemSuper with a string as a receiver. (This wasn't exposed until bug 1661211 part 1.) In that case, GetNativeDataPropertyByValuePure can accidentally succeed by looking up the receiver instead of the index.

Here's a simplified testcase that more directly targets the bug:

class A1 { constructor() { this.dummy = true; } }
class A2 { constructor() { this.dummy = true; } }
class A3 { constructor() { this.dummy = true; } }
class A4 { constructor() { this.dummy = true; } }
class A5 { constructor() { this.dummy = true; } }
class A6 { constructor() { this.dummy = true; } }
class A7 { constructor() { this.dummy = true; } }
class A8 { constructor() { this.dummy = true; } }
class A9 { constructor() { this.dummy = true; } }

var constructors = [A1, A2, A3, A4, A5, A6, A7, A8, A9];
for (var i = 0; i < 1000; i++) {
    for (var construct of constructors) {
	var h = new construct();
        // This should access h.nonexistent, not h.dummy.
	assertEq(Reflect.get(h, "nonexistent", "dummy"), undefined);
    }
}

I think the fix is to simply update BaselineCacheIRCompiler::init.

Comment 6

[Iain Ireland [:iain]]

Attachment: Bug 1709328: Fix baseline IC register assignment for GetElemSuper r=jandem

When bug 1472211 reordered the operands of GetElemSuper, we got our register assignments out of sync. Our GetElemSuper baseline ICs have been silently failing ever since.

This patch re-establishes a consistent assignment between BaselineCodeGen::emit_GetElemSuper, GetPropIRGenerator, and BaselineCacheIRCompiler::init:
obj = input operand 0 = baseline frame slot 0
id/key = input operand 1 = R1
receiver = input operand 2 = R0

We have a testcase for megamorphic GetElemSuper, but it didn't catch this bug because our ICs just failed silently (either because a property name is not a valid receiver, or because the superbase object is not a valid property name) and let the fallback handle it. The testcase I've attached to this bug doesn't fail prior to bug 1661211 part 1, where we added support for non-object receivers for GetElemSuper, but I checked out old revisions immediately before and after bug 1472211 and verified in rr that GetNativeDataPropertyByValuePure was passed the correct id before, and the receiver object after.

Drive-by: while investigating this bug, I realized that attachMegamorphicNativeSlot had an unused argument left over from TI removal.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9ada5c21801
Fix baseline IC register assignment for GetElemSuper r=jandem

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/d9ada5c21801

Comment 9

[Bugmon [:jkratzer for issues]]

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