1965618 - Assertion failure: !iter_.moreFrames() && (catchingException() || propagatingIonExceptionForDebugMode()), at jit/BaselineBailouts.cpp:856

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

var x = newGlobal({ newCompartment: true });
x.parent = this;
x.eval("Debugger(parent).onExceptionUnwind=function(){}");
var z = [, , , 0, 0, , , , , , , , , -2, , 0];
for (var j = 0; j < 16; j++) {
  for (var k = 0; k < 99; k++) {
    function f() {
      a;
    }
    try {
      (function () {
        if (Math.ceil(z[k])) {
        } else {
          if (z[j] + 1) {
            f(z[j] >>> 0, Math.ceil(~(z[j] >>> 0)));
          }
        }
      })();
    } catch (e) {}
  }
}

(gdb) bt
#0  0x58d36d8f in MOZ_CrashSequence (aAddress=0x0, aLine=856)
    at /home/ifive8400/shell-cache/js-dbg-32-armsim32-linux-x86_64-0d55bf3d56e4/objdir-js/dist/include/mozilla/Assertions.h:248
#1  BaselineStackBuilder::buildExpressionStack (this=0xffffbc78) at /home/ifive8400/trees/mozilla-central/js/src/jit/BaselineBailouts.cpp:854
#2  0x58d3a67d in BaselineStackBuilder::buildOneFrame (this=0xffffbc78) at /home/ifive8400/trees/mozilla-central/js/src/jit/BaselineBailouts.cpp:1511
#3  0x58d3286b in js::jit::BailoutIonToBaseline (cx=0xf6b1d100, activation=0xffffc3d8, iter=..., bailoutInfo=0xffffbe00, excInfo=0xffffc120,
    reason=js::jit::BailoutReason::ExceptionHandler) at /home/ifive8400/trees/mozilla-central/js/src/jit/BaselineBailouts.cpp:1684
#4  0x58d336c2 in js::jit::ExceptionHandlerBailout (cx=0xf6b1d100, frame=..., rfe=0xf69ffe40, excInfo=...)
    at /home/ifive8400/trees/mozilla-central/js/src/jit/Bailouts.cpp:296
#5  0x59261320 in js::jit::HandleExceptionIon (cx=0xf6b1d100, frame=..., rfe=<optimized out>, hitBailoutException=<optimized out>)
    at /home/ifive8400/trees/mozilla-central/js/src/jit/JitFrames.cpp:278
/snip

The testcase seems to still occur as far back as m-c rev c1acdcb0c63c (Feb 2024).

Run with --fuzzing-safe --no-threads --baseline-eager, compile with AR=ar PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig 'CC="clang -msse2 -mfpmath=sse"' 'CXX="clang++ -msse2 -mfpmath=sse"' sh ../configure --host=x86_64-pc-linux-gnu --target=i686-pc-linux --enable-simulator=arm --enable-debug --enable-debug-symbols --with-ccache --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev 0d55bf3d56e4.

Arai-san, setting needinfo? from you as a start since the debugger seems to be involved, but feel free to bounce this to others if it involves the JIT, etc.

Comment 1

[Jan de Mooij [:jandem]]

This only reproduces with the arm32 simulator build for me and arai doesn't have a 32-bit build environment set up atm. Mind taking a quick look at this, Iain?

Comment 2

[Iain Ireland [:iain]]

I think this is a problem with ReduceSetForPush.

We have a safepoint that captures d0, a double register. In JSJitFrameIter::machineState, we initialize the set of floating point registers by reading all the float spills (which return only d0) then calling reduceSetForPush. This changes the set to instead contain s0 and s1, as float registers. This is semantically equivalent to d0, because they're overlaid. However, when we try to read the value of d0, MachineState::has returns false, because it's only checking for d0 specifically.

Our other supported platforms instead implement reduceSetForPush by filtering out any float registers that would also be pushed as doubles. At a glance, it looks like mips, loong, and risc-v always push single registers as doubles. It's not immediately clear to me why our arm32 code chooses to do things this way.

If I rewrite this code to prefer double registers, the test passes. I'm running tests to make sure it doesn't break anything else.

I think this is probably not security-sensitive. If the assertion fails, then we'll return the magic OptimizedOut value. But the value is unused, so the only code that can observe it is the debugger, and the debugger already knows how to handle OptimizedOut.

Comment 3

[Nicolas B. Pierron [:nbp]]

We have 2 register set, the AllocatableRegisterSet and the LiveRegisterSet, the AllocatableRegisterSet will coalesce s0 and s1 into d0, while the LiveRegisterSet will not, and adding s0 and s1 does not corresponds to adding d0.

The MachineState is suppose to match a LiveRegisterSet of allocated registers. Thus this could be normal if s0 and s1 are part of the MachineState but not d0.

Comment 4

[Iain Ireland [:iain]]

The problem here is that d0 is supposed to be part of the MachineState (we have a live double value), but when we create the MachineState we convert that to s0 + s1. My patch changes the arm implementation of ReduceSetForPush to more closely match what other architectures are doing. It fixes this case and doesn't appear to break anything else.

Comment 5

[Iain Ireland [:iain]]

Attachment: Bug 1965618: Refactor ReduceSetForPush on arm r=nbp

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2319aab57bd
Refactor ReduceSetForPush on arm r=nbp

Comment 7

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/d2319aab57bd

Comment 8

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

The patch landed in nightly and beta is affected.
:iain, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox140 to wontfix.

For more information, please visit BugBot documentation.