1934425 - Assertion failure: propagatingIonExceptionForDebugMode() && !iter_.moreFrames(), at jit/BaselineBailouts.cpp:845

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20241129-ed73389dc144 (debug build, run with --fuzzing-safe --ion-offthread-compile=off --enable-explicit-resource-management --ion-warmup-threshold=0 --baseline-warmup-threshold=0):

a = [];
async function b() {
  c = [{ [Symbol.asyncDispose]: () => a}, 0];
  for (await using d of c);
}
b();

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0000555557aefda2 in BaselineStackBuilder::buildExpressionStack() ()
#1  0x0000555557af4015 in BaselineStackBuilder::buildOneFrame() ()
#2  0x0000555557aeaae3 in js::jit::BailoutIonToBaseline(JSContext*, js::jit::JitActivation*, js::jit::JSJitFrameIter const&, js::jit::BaselineBailoutInfo**, js::jit::ExceptionBailoutInfo const*, js::jit::BailoutReason) ()
#3  0x0000555557aebb20 in js::jit::ExceptionHandlerBailout(JSContext*, js::jit::InlineFrameIterator const&, js::jit::ResumeFromException*, js::jit::ExceptionBailoutInfo const&) ()
#4  0x000055555801b677 in js::jit::HandleException(js::jit::ResumeFromException*) ()
#5  0x00003b4933c5dfe6 in ?? ()
[...]
#15 0x0000000000000000 in ?? ()
rax	0x5555557c65f1	93824994797041
rbx	0x7fffffffc440	140737488340032
rcx	0x5555588ab1e0	93825046065632
rdx	0x1	1
rsi	0x0	0
rdi	0x7ffff7bef7d0	140737349875664
rbp	0x7fffffffc280	140737488339584
rsp	0x7fffffffc1e0	140737488339424
r8	0x0	0
r9	0x3	3
r10	0x0	0
r11	0x0	0
r12	0x5555558cd11e	93824995873054
r13	0xfffa800000000009	-1548112371908599
r14	0x7fffffffc230	140737488339504
r15	0x2	2
rip	0x555557aefda2 <BaselineStackBuilder::buildExpressionStack()+1570>
=> 0x555557aefda2 <_ZN20BaselineStackBuilder20buildExpressionStackEv+1570>:	movl   $0x34d,0x0
   0x555557aefdad <_ZN20BaselineStackBuilder20buildExpressionStackEv+1581>:	callq  0x555556ec97b0 <abort>

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 20241201095257-4df19decbcec.
The bug appears to have been introduced in the following build range:

Start: c1acf137ed794e8b553c1f40512d21090d1a9b7c (20241114072145)
End: e299ddd844812c1cd97440fd74eb94e0736fbbe9 (20241114100954)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c1acf137ed794e8b553c1f40512d21090d1a9b7c&tochange=e299ddd844812c1cd97440fd74eb94e0736fbbe9

Comment 4

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

Set release status flags based on info from the regressing bug 1927195

:debadree333, since you are the author of the regressor, bug 1927195, could you take a look?

For more information, please visit BugBot documentation.

Comment 5

[Debadree Chatterjee]

Some of my investigations on this bug as well:

So if you look at the bytecode disassembly of the b fn() we have some code like this:

# from JumpIfFalse @ 00568
00586:   4  JumpTarget (ic: 50)         # c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value
00591:   4  CheckIsObj 5                # c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value
00593:   4  Dup                         # c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value
00594:   4  Dup                         # c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value
00595:   4  Symbol 14                   # c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value Symbol.asyncDispose
00597:   4  GetElem                     # c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value[Symbol.asyncDispose]
00598:   4  IsNullOrUndefined           # c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value[Symbol.asyncDispose] IS_NULL_OR_UNDEF
00599:   4  JumpIfFalse 620 (+21)       # c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value c[Symbol.iterator]().next().value[Symbol.asyncDispose]

Now its the optimised CheckIsObj code that throws and thats why try to bailout and it seems for some reason its not able to reconstruct the data over here https://searchfox.org/mozilla-central/source/js/src/jit/JitFrames.cpp#1920 as to why its still under investigation!

Comment 6

[Iain Ireland [:iain]]

I took a look at this. I think that this actually just exposed a subtle existing issue. The fix may be as simple as relaxing the assertion, although it's possible we can find a better fix.

tryRead fails because the resume point it can't find a value for rcx. The snapshot iterator was initialized from a safepoint that only preserved rax. Notably, looking at the iongraph for this compilation, rcx is the register where we expected to store the result of the CheckIsObj. It makes sense that we wouldn't have marked rcx as live across the call, because it wasn't: the op is overwriting rcx. But the CheckIsObj threw, so rcx isn't available yet.

Normally when we catch an exception, the expression stack is empty. The main exception is iterators: if we have a catch inside a for-of, then the state for the iterator will be on the stack when we do the catch. Here, that's c[Symbol.iterator]().next c[Symbol.iterator]() c[Symbol.iterator]().next().value.

When we resume in the catch block, our snapshot thinks that the current value of c[Symbol.iterator]().next().value is the result of evaluating CheckIsObj. But as previously mentioned, that result isn't actually available, because we threw.

My assumption is that this wasn't a problem before the explicit resource management patch because, until that patch landed, we didn't generate a CheckIsObj inside a try-catch inside a for-of loop (where the CheckIsObj has to be checking the iterator state for that particular for-of loop).

I tried disabling the assertion locally and verified that the result value is never actually used, at least in this case.

Comment 7

[Iain Ireland [:iain]]

Attachment: Bug 1934425: Relax assertion in buildExpressionStack r=jandem

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43a636194479
Relax assertion in buildExpressionStack r=jandem

Comment 9

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/43a636194479

Comment 10

[Bugmon [:jkratzer for issues]]

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