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

Status: RESOLVED FIXED

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

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20210530-1514fcbf80a0 (debug build, run with --fuzzing-safe --no-threads --ion-warmup-threshold=1 --baseline-warmup-threshold=1 --ion-gvn=off):

function testMathyFunction (f, inputs) {
  var results = [];
    for (var j = 0; j < inputs.length; ++j)
      for (var k = 0; k < inputs.length; ++k)
          results.push(f(inputs[j], inputs[k]));
}
mathy3=(function(){
  heap = new ArrayBuffer(40);
  var Uint16ArrayView = new Uint16Array(heap);
  Uint32ArrayView = new Uint32Array(heap);
  function f(i0,i1) {
    Uint16ArrayView[2+i1 >>1] = !0+i0 -(8>>0xb<i0 >>> (Uint32ArrayView[0]))
  }
  return f
})();
testMathyFunction(mathy3,[Math.PI,0xfffff,0x80000000,1,-Number.M,(5),(3)])

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x000055555796e1f4 in js::jit::WarpOracle::createSnapshot() ()
#1  0x000055555790b006 in js::jit::CreateWarpSnapshot(JSContext*, js::jit::MIRGenerator*, JS::Handle<JSScript*>) ()
#2  0x00005555578ec214 in js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*) ()
#3  0x00005555578ecf19 in IonCompileScriptForBaseline(JSContext*, js::jit::BaselineFrame*, unsigned char*) ()
#4  0x000030c9ae4b08c5 in ?? ()
[...]
#37 0x0000000000000000 in ?? ()
rax	0x55555572c062	93824994164834
rbx	0x7ffff6019000	140737320685568
rcx	0x555558047558	93825037268312
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffb670	140737488336496
rsp	0x7fffffffb5d0	140737488336336
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99840	140737353717824
r10	0x0	0
r11	0x0	0
r12	0x7ffff5a5a5f0	140737314661872
r13	0x7fffffffb690	140737488336528
r14	0x7ffff5e42498	140737318757528
r15	0x7b399092	2067370130
rip	0x55555796e1f4 <js::jit::WarpOracle::createSnapshot()+1492>
=> 0x55555796e1f4 <_ZN2js3jit10WarpOracle14createSnapshotEv+1492>:	movl   $0xc2,0x0
   0x55555796e1ff <_ZN2js3jit10WarpOracle14createSnapshotEv+1503>:	callq  0x555556a86d7a <abort>

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

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

Start: 1add9a1bbfacd4ff1bee961fa8886b8cd0566d23 (20210227094458)
End: 80f5cb43d64e45bad9247216faa6553d836e53fe (20210227004609)
Pushlog: https://hg.mozilla.org/mozilla-unified/pushloghtml?fromchange=1add9a1bbfacd4ff1bee961fa8886b8cd0566d23&tochange=80f5cb43d64e45bad9247216faa6553d836e53fe

Comment 3

[Iain Ireland [:iain]]

This is very similar to bug 1686207, except with LoadFromTypedArray as the source of the maybe-double instead of Ursh.

Here's a somewhat reduced testcase that works with --no-threads --fast-warmup --ion-gvn=off:

var heap = new ArrayBuffer(4);
var view32 = new Uint32Array(heap);

function foo(i0) {
    var t1 = i0 + 1;
    var t2 = i0 >>> view32[0];
    var t3 = t1 - (t2 > 0);
    return t3;
}

with ({}) {}
view32[0] = 0x80000000;
for (var i = 0; i < 100; i++) {
    foo(0);
}
view32[0] = 0;
for (var i = 0; i < 100; i++) {
    foo(0x80000000);
}

First we call foo with a non-Int32 Uint32 in view32[0], which causes us to attach a GetElem IC where the LoadTypedArrayElementResult has allowDoubleForUint32 set to true. Then we call it again with a smaller value in view32[0], but a double value for i0. This causes us to attach a new IC for the Ursh that expects a number for the lhs, and an Int32 for the rhs. When we transpile, we get something like this:

23 loadUnboxedScalar (as Double)
...
26 unbox loadUnboxedScalar to Int32

Apply types adds a box:

23 loadUnboxedScalar ... Double
...
33 box loadUnboxedScalar23
26 unbox loadUnboxedScalar to Int32

If GVN is enabled, MUnbox::foldsTo converts this to MToNumberInt32. With GVN disabled, we fail to unbox (even though the 'double' can be losslessly converted to int32) and bail. In baseline, LoadTypedElementResult converts the double to an Int32, so the Ursh IC succeeds and we recompile with the same CacheIR.

In bug 1686207, we fixed this by forcing Ursh to always return a double in baseline when allowDouble is true. We should probably do the same thing here.

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 1713567: Clean up uint32 boxing r=jandem!

We've fixed similar issues a couple of times before, so I'm doing a general cleanup to nail this all down. Baseline ICs should return the same type as the resulting transpiled MIR.

In theory visitLoadTypedArrayElementHole could safely return either Int32 or Double, because the possibility of returning undefined already forces the result to be a Value, but I'm not convinced that's valuable. (My initial draft had a third AllowDouble option for Uint32Mode; we could reintroduce it if we thought it was valuable.)

Comment 5

[Iain Ireland [:iain]]

Attachment: Bug 1713567: Rename allowDouble to forceDouble r=jandem!

We started out calling this allowDouble, then we incrementally rewrote all the uses to force the use of doubles instead. Renaming for clarity.

Depends on D116881

Comment 6

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

:iain, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 7

[Iain Ireland [:iain]]

Marking the bailout loop bug as the regressor, because we didn't have a clear policy against this until then.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/71db66b73a47
Clean up uint32 boxing r=jandem
https://hg.mozilla.org/integration/autoland/rev/a60835242de7
Rename allowDouble to forceDouble r=jandem

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/71db66b73a47
https://hg.mozilla.org/mozilla-central/rev/a60835242de7

Comment 10

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Bug appears to be fixed on mozilla-central 20210608040833-abe5cbf9daec but BugMon was unable to reproduce using mozilla-central 20210530214555-1514fcbf80a0.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 11

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

The patch landed in nightly and beta is affected.
:iain, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 12

[Iain Ireland [:iain]]

This is not important enough to uplift. I don't think it's possible to trigger without using the shell-only --ion-gvn=off option to disable global value numbering. Even if it was, it's just a perf issue.