1802100 - Crash [@ js::FrameIter::frameSlotValue] or Assertion failure: slot < debugNumValueSlots(), at jit/BaselineFrame.h:131

Status: VERIFIED FIXED

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

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20221123-eac66a6b1959 (opt build, run with --fuzzing-safe --ion-offthread-compile=off):

Object.prototype[Symbol.toPrimitive] = Object;
setInterruptCallback(function() {
    throw () => a
});
function b() {
    for (var c; 0;);
    interruptIf(true);
    b();
}
b();

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00005555560f45b2 in js::FrameIter::frameSlotValue(unsigned long) const ()
#1  0x00005555560e3a54 in js::DecompileValueGenerator(JSContext*, int, JS::Handle<JS::Value>, JS::Handle<JSString*>, int) ()
#2  0x0000555555801267 in js::ReportValueError(JSContext*, unsigned int, int, JS::Handle<JS::Value>, JS::Handle<JSString*>, char const*, char const*) ()
#3  0x0000555555808d04 in ReportCantConvert(JSContext*, unsigned int, JS::Handle<JSObject*>, JSType) ()
#4  0x0000555555d9896d in js::ToPrimitiveSlow(JSContext*, JSType, JS::MutableHandle<JS::Value>) ()
#5  0x0000555555dfe2f0 in JSString* js::ToStringSlow<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType) ()
#6  0x0000555556161fbe in JS::ErrorReportBuilder::init(JSContext*, JS::ExceptionStack const&, JS::ErrorReportBuilder::SniffingBehavior) ()
#7  0x0000555556053287 in js::shell::AutoReportException::~AutoReportException()::$_0::operator()<JS::ErrorReportBuilder, JS::ExceptionStack> ()
#8  0x0000555556052fab in js::shell::AutoReportException::~AutoReportException() ()
#9  0x000055555605edfb in ShellInterruptCallback(JSContext*) ()
#10 0x000055555613c144 in JSContext::handleInterrupt() ()
#11 0x0000555556279fc0 in js::jit::CheckOverRecursedBaseline(JSContext*, js::jit::BaselineFrame*) ()
#12 0x00000ce8656013db in ?? ()
#13 0x0000000000000000 in ?? ()
rax	0x5df	1503
rbx	0x7fffffffb518	140737488336152
rcx	0x7fffffffc150	140737488339280
rdx	0xfffffff6	4294967286
rsi	0xfffffffffffffa1f	-1505
rdi	0x7fffffffb570	140737488336240
rbp	0x7fffffffb3d0	140737488335824
rsp	0x7fffffffb330	140737488335664
r8	0x55555656b4b0	93825009104048
r9	0x31	49
r10	0x7ffff602a4a1	140737320756385
r11	0x7ffff4e470c0	140737301999808
r12	0x7ffff6015120	140737320669472
r13	0x0	0
r14	0xfffffffffffffa1f	-1505
r15	0x7ffff602a4a1	140737320756385
rip	0x5555560f45b2 <js::FrameIter::frameSlotValue(unsigned long) const+338>
=> 0x5555560f45b2 <_ZNK2js9FrameIter14frameSlotValueEm+338>:	mov    -0x48(%rcx,%rax,8),%rax
   0x5555560f45b7 <_ZNK2js9FrameIter14frameSlotValueEm+343>:	jmpq   0x5555560f44a7 <_ZNK2js9FrameIter14frameSlotValueEm+71>

Marking s-s because the crash address keeps changing and the assert in the debug build looks like something might be out-of-bounds.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20221123094429-f150bc1f71d0.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: d2c1f4743cfa4557cd2e1383ea2745aaf1813aaf (20211124155224)
End: eac66a6b195990ad2a4fddcf18f49eefb79a8b36 (20221123044553)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=False, fuzzing=False, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False)

Comment 4

[Daniel Veditz [:dveditz]]

This looks like it's a Shell-only function? Or, we do interrupts ourselves but I don't think we allow user-defined ones.

Comment 5

[Iain Ireland [:iain]]

This is pretty convoluted. The sequence of events is:

1. We schedule an interrupt with interruptIf.
2. In the prologue of a function with one local variable (script->nfixed() == 1), we do a VM call to CheckOverRecursedBaseline. Because this is the prologue, the CallVMPhase is BeforePushingLocals.
3. In the interrupt handler, we throw an object as an exception.
4. The AutoReportException in ShellInterruptCallback tries to report the exception. It calls toPrimitive to try to print the exception, which has been monkey-patched to return an object. We call ReportCantConvert.
5. ReportCantConvert tries to decompile the thrown object. We create a FrameIter to walk the stack frame.
6. The top stack frame is the function from step 2. In numFrameSlots we subtract the number of value slots (which was 0, because this was BeforePushingLocals) from the number of fixed slots, which is 1. We get a bogus number of frame slots (0xffffffffffffffff), and everything explodes.

We don't have user-defined interrupts, so I don't immediately see a way to replicate this outside the shell.

Not sure what the best fix is. We already have code to give up if we're in the prologue, but unfortunately this function has no prologue bytecode, so the check does nothing.

Comment 6

[Christian Holler (:decoder)]

The slow-script dialog is one way to perform an interrupt and I believe that was the main reason why we added this to the shell. I wouldn't rule this out entirely in the browser without careful investigation.

Comment 7

[Nicolas B. Pierron [:nbp]]

Iain, are you working on this bug or should we ask someone else?

Comment 8

[Iain Ireland [:iain]]

Jan and I discussed it this morning and agreed on a potential solution.

I believe it's not an issue in the browser because it requires the interrupt handler to do certain weird things that don't appear to happen in any of our existing interrupt handlers, and we don't allow user-defined interrupt handlers in the browser, but I'm happy to leave it hidden for now.

Comment 9

[Iain Ireland [:iain]]

Attachment: Bug 1802100: Add FrameIter::inPrologue r=jandem

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/03fce75ab7a1

Comment 11

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20221201161829-4ec5232d1c53.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.