1450503 - Assertion failure: INITHOMEOBJECT expects undefined slot, at js/src/jit/MacroAssembler.cpp:2013

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 8063b0c54b88 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --baseline-eager):

(function(index) {
    var o = {
        [index]: class {
            static constructor() {
                super["prop"]();
            }
        }
    }
    if (index === 0) {
        (function self() {
            self.caller(1);
        })();
    }
})(0);


Backtrace:

received signal SIGTRAP, Trace/breakpoint trap.
0x00003a0983763db0 in ?? ()
#0  0x00003a0983763db0 in ?? ()
[...]
#27 0x00007fffffffa680 in ?? ()
#28 0x0000000000794ca0 in EnterJit (cx=0x7fffffff9d40, state=..., code=0x0) at js/src/jit/Jit.cpp:101
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
rax	0x7ffff4caf600	140737300329984
rbx	0xfffe7ffff4c90160	-422212653219488
rcx	0xfffe7ffff4caf5b0	-422212653091408
rdx	0x7ffff5f3f000	140737319792640
rsi	0x7ffff4c8d060	140737300189280
rdi	0x7fffffff9f90	140737488330640
rbp	0x7fffffffa0d8	140737488330968
rsp	0x7fffffffa070	140737488330864
r8	0x0	0
r9	0x0	0
r10	0x0	0
r11	0x7ffff6b9e7a0	140737332766624
r12	0x0	0
r13	0x7fffffffae50	140737488334416
r14	0x2043	8259
r15	0x7fffffffa200	140737488331264
rip	0x3a0983763db0	63812534681008
=> 0x3a0983763db0:	mov    %rcx,0x40(%rax)
   0x3a0983763db4:	mov    %rax,%r11


Filing s-s because this hits a trap in MacroAssembler with no stack.

Comment 1

[Jan de Mooij [:jandem]]

Maybe you can take a look at this because bug 1169743 added this check. That said, it's probably unrelated to that.

Comment 2

[Ted Campbell [:tcampbell]]

This is a VM bug, not a JIT bug. I can reproduce by adding the same assert to JSOP_INITHOMEOBJECT. We seem to be be sharing the constructor lambda across multiple class expressions (which each have different [[HomeObject]] bindings). The logic for singleton lambdas probably needs to be tweaked.

Comment 3

[Ted Campbell [:tcampbell]]

My assessment was incorrect. The issue is due to [1] which uses an existing lambda and the slot is initialized with existing value before we call setExtendedSlot. In the interpreter this is fine, but my implementation in Baseline was assuming we could |init| and avoid barriers. The simple fix is to add the barriers and remove assert from baseline.

I think it is worth investigating how various FunctionExtended interact with barriers and cloning.

[1] https://searchfox.org/mozilla-central/rev/a0665934fa05158a5a943d4c8b277465910c029c/js/src/vm/JSFunction.cpp#2175

Comment 4

[Daniel Veditz [:dveditz]]

Not sure how serious it is; guessing sec-high

Comment 5

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/24191914f43b
user:        Ted Campbell
date:        Wed Jun 07 14:04:13 2017 -0400
summary:     Bug 1169743 - Implement class decls with extends in Baseline (cont.) r=jandem

This iteration took 261.595 seconds to run.

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug-1450503-Remove-INITHOMEOBJECT-assert.patch

Under the Function.caller loophole with run-once lambdas, a method may be cloned after being assigned a HomeObject. This patch changes the initSlot with debug assert to a typical setSlot pattern to match the interpreter code.

Comment 7

[Ted Campbell [:tcampbell]]

This bug is equivalent to a missing pre-barrier in release. The previous value is always reachable from JSScript::objects, so if I understand what :sfink explained this isn't violating snapshot-at-the-beginning and so this may not be exploitable at all.

Either way, the change itself is low risk.

Comment 8

[Ted Campbell [:tcampbell]]

It looks like this is a footgun rather than a sec issue so dropping severity. We'll fix the barriers anyways but only for FF61+.

Comment 9

[Ted Campbell [:tcampbell]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d3b07135ca57112f1a053f491a338a618def73c

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7d3b07135ca5

Comment 11

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.