1664612 - [warp] Crash [@ js::jit::AssertValidObjectPtr] or Crash [@ ??] with TypedObject

Status: VERIFIED FIXED

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

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200911-b133e2d673e8 (debug build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off --fast-warmup --warp):

var StructType = TypedObject.StructType;
var uint32 = TypedObject.uint32;
for (idx = 0; idx < 10; ++idx) {
  var grain = uint32;
  var type = grain.array(4);
  new type([0, 2, 4, 6])
  var grain = new StructType({f57: uint32});
  var type = grain.array(4);
  new type([{f57:0}, {f57:2}, {f57:4}, {f57:6}]);
}

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  js::jit::AssertValidObjectPtr (cx=0x7ffff6047000, obj=0x6800000000005) at js/src/jit/VMFunctions.cpp:1322
#1  0x00000af690255669 in ?? ()
#2  0xb948565122c98348 in ?? ()
#3  0x00007fffffffa508 in ?? ()
#4  0x3181ba62740e2900 in ?? ()
#5  0x0000000000000000 in ?? ()
rax	0x7ffff7f99e01	140737353719297
rbx	0x4	4
rcx	0x7ffff6047000	140737320873984
rdx	0x2fcc197d4c0	3284602967232
rsi	0x7ffff6047000	140737320873984
rdi	0x7ffff6049000	140737320882176
rbp	0x7fffffffa4e0	140737488332000
rsp	0x7fffffffa4a0	140737488331936
r8	0x2fcc19a1330	3284603114288
r9	0x0	0
r10	0xffffd555559c57f8	-46912491464712
r11	0x2fcc198ad28	3284603022632
r12	0x0	0
r13	0x7fffffffaf60	140737488334688
r14	0x7ffff6047000	140737320873984
r15	0x6800000000005	1829587348619269
rip	0x5555577d3b36 <js::jit::AssertValidObjectPtr(JSContext*, JSObject*)+70>
=> 0x5555577d3b36 <js::jit::AssertValidObjectPtr(JSContext*, JSObject*)+70>:	mov    (%r15),%rax
   0x5555577d3b39 <js::jit::AssertValidObjectPtr(JSContext*, JSObject*)+73>:	test   $0x7,%al

This requires warp which is still disabled, but filing s-s as recommended by :nbp, since we now have a pref to turn on warp in Nightly.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Jason Kratzer [:jkratzer]]

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

Start: ff2d89b0aab14c72ab8155e076fa7a80c948b701 (20200827163206)
End: 5bff314e558e70b402e31bc93faa35aa6bd405f6 (20200827163233)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ff2d89b0aab14c72ab8155e076fa7a80c948b701&tochange=5bff314e558e70b402e31bc93faa35aa6bd405f6

Comment 3

[Jan de Mooij [:jandem]]

Warp is inlining TypedObjectSet into TypedObjectSetArray and we then end up loop-hoisting slot loads for UnsafeGetObjectFromReservedSlot. But that's not valid because the get-from-reserved-slot calls in self-hosted code depend on control flow (it's inside a switch-case here).

I think we have to mark these loads non-movable... IonBuilder has the same issue, although it's possible it's harder to trigger there.

Comment 4

[André Bargull [:anba]]

(In reply to Jan de Mooij [:jandem] from comment #3)

Warp is inlining TypedObjectSet into TypedObjectSetArray and we then end up loop-hoisting slot loads for UnsafeGetObjectFromReservedSlot. But that's not valid because the get-from-reserved-slot calls in self-hosted code depend on control flow (it's inside a switch-case here).

Hmm, that sounds just too similar to bug 1437842.

I think we have to mark these loads non-movable...

These specific UnsafeGetObjectFromReservedSlot loads or all UnsafeGetReservedSlot?

Comment 5

[Jan de Mooij [:jandem]]

(In reply to André Bargull [:anba] from comment #4)

Hmm, that sounds just too similar to bug 1437842.

Yeah..

I think we have to mark these loads non-movable...

These specific UnsafeGetObjectFromReservedSlot loads or all UnsafeGetReservedSlot?

I was thinking all of them, I don't see a good (and robust) alternative.

Comment 6

[André Bargull [:anba]]

We could change the TypedObject code to use the "guard"-style approach.

function TypedObjectSet(descr, typedObj, offset, name, fromValue) {
  var guardedDesc;
  if ((guardedDesc = guardToScalarDesc(descr)) !== null) {
    TypedObjectSetScalar(descr, typedObj, offset, fromValue);
    return;
  }
  if ((guardedDesc = guardToReferenceDesc(descr)) !== null) {
    TypedObjectSetReference(descr, typedObj, offset, name, fromValue);
    return;
  }
  // etc.
}

(Or just move the code into C++, because the various Store_X functions call into C++ anyway.)

Comment 7

[Jan de Mooij [:jandem]]

The plan is to remove the typed object code that's not used by Wasm. I expect that will include the self-hosted code here, but we'll have to see.

Comment 8

[Steven DeTar [:sdetar]]

Jan, now that 1666494 has landed, can we close this bug?

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Steven DeTar [:sdetar] from comment #8)

Jan, now that 1666494 has landed, can we close this bug?

Yes, bug 1666494 and bug 1669784 removed the self-hosted code for typed objects.

Comment 10

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Bug appears to be fixed on mozilla-central 20201018211631-cb4114308a0e but BugMon was unable to reproduce using mozilla-central 20200911151042-b133e2d673e8.

Comment 11

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Bug appears to be fixed on mozilla-central 20201018211631-cb4114308a0e but BugMon was unable to reproduce using mozilla-central 20200911151042-b133e2d673e8.

Comment 12

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Bug appears to be fixed on mozilla-central 20201019213835-e90233e0fe71 but BugMon was unable to reproduce using mozilla-central 20200911151042-b133e2d673e8.

Comment 13

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Bug appears to be fixed on mozilla-central 20201019213835-e90233e0fe71 but BugMon was unable to reproduce using mozilla-central 20200911151042-b133e2d673e8.

Comment 14

[André Bargull [:anba]]

I've manually removed the bugmon keyword because the bugmon analysis seems to repeating every eight hours without getting any actionable results.

Comment 15

[Jason Kratzer [:jkratzer]]

There were a few issues in Bugmon causing this to spam report. They have since been fixed. Re-enabling bugmon. Please NI me if you have any other issues.

Comment 16

[Jason Kratzer [:jkratzer]]

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

Comment 17

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

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.