Closed Bug 1883144 Opened 1 year ago Closed 1 year ago

Assertion failure: !cx->isExceptionPending(), at vm/Interpreter.cpp:483

Categories

(Core :: JavaScript: GC, defect, P3)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
126 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox123 --- wontfix
firefox124 --- wontfix
firefox125 --- wontfix
firefox126 --- fixed

People

(Reporter: gkw, Assigned: jonco)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, reporter-external, testcase)

Attachments

(2 files, 1 obsolete file)

Attached file debug stack
var x = newGlobal();
(function () {
  function f01() {
    let y = newGlobal({ newCompartment: true });
    y.parent = this;
    function g() {
      for (let i = 0; i < undefined; ) {}
      Debugger(parent).onEnterFrame = function (z) {
        z.eval("(" + z + ")()");
        return f01;
      };
    }
    y.eval("(" + g + ")()");
    class C {
      static set d(x) {
        function h() {(function () {
            (function(){})(); gczeal(10); m();
        })() }
        newGlobal().eval(
          "(function h() { (function () { \
            (function(){})(); gczeal(10); m(); \
          })() })()"
        );
      }
    }
    oomTest(function () {
      C.d = 0;
    });
  }
  f01();
})();
(gdb) bt
#0  CallJSNative (cx=0xf761a100, native=0x587fff20 <js::DebuggerFrame::CallData::ToNative<&js::DebuggerFrame::CallData::evalMethod>(JSContext*, unsigned int, JS::Value*)>, reason=js::CallReason::Call, args=...) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:483
#1  0x58089db2 in js::InternalCallOrConstruct (cx=0xf761a100, args=..., construct=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:574
#2  0x5808ab1a in InternalCall (cx=0xf761a100, args=..., reason=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:641
#3  0x5808aa92 in js::CallFromStack (cx=0xf761a100, args=..., reason=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:646
#4  0x58c3883a in js::jit::DoCallFallback (cx=0xf761a100, frame=0xf67ffa88, stub=0xf54eb80c, argc=1, vp=0xf67ffa48, res=...) at /home/ubumain/trees/mozilla-central/js/src/jit/BaselineIC.cpp:1659
#5  0x58dd7a35 in js::jit::Simulator::softwareInterrupt (this=0xf7623200, instr=0xf762af64) at /home/ubumain/trees/mozilla-central/js/src/jit/arm/Simulator-arm.cpp:2392
#6  0x58dd4a27 in js::jit::Simulator::decodeType7 (this=0xf7623200, instr=0xf762af64) at /home/ubumain/trees/mozilla-central/js/src/jit/arm/Simulator-arm.cpp:3401
#7  js::jit::Simulator::instructionDecode (this=0xf7623200, instr=0xf762af64) at /home/ubumain/trees/mozilla-central/js/src/jit/arm/Simulator-arm.cpp:4431
#8  0x58ddd37b in js::jit::Simulator::execute<false> (this=0xf7623200) at /home/ubumain/trees/mozilla-central/js/src/jit/arm/Simulator-arm.cpp:4487
#9  js::jit::Simulator::callInternal (this=0xf7623200, entry=0xe81fa9a0 "\377\377\377\352\360O-\351\377\377\377\352\004\320M\342\377\377\377\352\020\212-\355\377\377\377\352h\220\235\345\377\377\377\352\r\260\240\341\377\377\377\352t\240\235\345\377\377\377", <incomplete sequence \352>) at /home/ubumain/trees/mozilla-central/js/src/jit/arm/Simulator-arm.cpp:4565
#10 0x58ddd940 in js::jit::Simulator::call (this=0xf7623200, entry=0xe81fa9a0 "\377\377\377\352\360O-\351\377\377\377\352\004\320M\342\377\377\377\352\020\212-\355\377\377\377\352h\220\235\345\377\377\377\352\r\260\240\341\377\377\377\352t\240\235\345\377\377\377", <incomplete sequence \352>, argument_count=8) at /home/ubumain/trees/mozilla-central/js/src/jit/arm/Simulator-arm.cpp:4653
#11 0x5914d564 in EnterJit (code=<optimized out>, cx=<optimized out>, state=...) at /home/ubumain/trees/mozilla-central/js/src/jit/Jit.cpp:115
#12 js::jit::MaybeEnterJit (cx=0xf761a100, state=...) at /home/ubumain/trees/mozilla-central/js/src/jit/Jit.cpp:261
#13 0x5808916c in js::RunScript (cx=0xf761a100, state=...) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:442
#14 0x58089d10 in js::InternalCallOrConstruct (cx=0xf761a100, args=..., construct=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:606
#15 0x5808ab1a in InternalCall (cx=0xf761a100, args=..., reason=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:641
#16 0x5808acec in js::Call (cx=0xf761a100, fval=..., thisv=..., args=..., rval=..., reason=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:673
#17 0x5876bbee in js::Call (cx=0xf761a100, fval=..., thisObj=0xf507b040, arg0=..., rval=...) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.h:124
/snip
The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/6c8839af7efd
user:        André Bargull
date:        Wed Jan 10 15:14:22 2024 +0000
summary:     Bug 1873042 - Part 10: Copy multiple characters in Substr codegen. r=jandem

Run with --fuzzing-safe --ion-offthread-compile=off --ion-eager --arm-asm-nop-fill=1 --arm-hwcap=vfp, compile with 'CC="clang -msse2 -mfpmath=sse"' AR=ar 'CXX="clang++ -msse2 -mfpmath=sse"' PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig 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 76bfcd57b0cd.

Setting s-s to be safe. Andre, is bug 1873042 a likely regressor?

Flags: sec-bounty?
Flags: needinfo?(andrebargull)

Note that this testcase seems to involve gczeal(10) (hence cc'ing Steve since Jon is out on PTO for a few weeks), is a little slow (takes about 5s to crash with the assertion failure), but is reproducible, so you might want to try and reduce to a better testcase if possible.

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

Group: core-security → javascript-core-security

Not a regression from bug 1873042. Bug 1873042 just changed code-gen, which causes different allocations when --arm-asm-nop-fill=1 is used, which then can trigger the assertion.

The actual bug is in DependentAddPtr::refreshAddPtr when used in Debugger::wrapDebuggeeObject:

  1. Debugger::wrapDebuggeeObject creates DependentAddPtr.
  2. The DependentAddPtr constructor performs Debugger::ObjectWeakMap::lookupForAdd.
  3. Following the class hierarchy, we end up in mozilla::detail::HashTable::lookupForAdd.
  4. mozilla::detail::HashTable::lookupForAdd calls EnsureHash, which ends up calling StableCellHasher<T>::ensureHash.
  5. This call will eventually call NativeObject::setOrUpdateUniqueId.
  6. The oomTest in the test case leads to triggering an artificial OOM for the allocateSlots in NativeObject::setOrUpdateUniqueId.
  7. This sets a pending exception on the JSContext.
  8. DependentAddPtr::addPtr now contains a non-live add-pointer.
  9. This leads to entering the else block in Debugger::wrapDebuggeeObject.
  10. The else block creates some objects.
  11. The gczeal now triggers a GC.
  12. The GC number changed after the GC, so DependentAddPtr::refreshAddPtr refreshes the add-pointer.
  13. The new lookup in refreshAddPtr succeeded and DependentAddPtr::addPtr was overwritten with a valid, live add-pointer.
  14. There's still a pending OOM exception, but because DependentAddPtr::addPtr was overwritten, the add succeeds and we return true from Debugger::wrapDebuggeeObject.
  15. This triggers the assertion in CallJSNative, because the call succeeded, but there's still a pending exception on the JSContext.
Flags: needinfo?(andrebargull)
No longer regressed by: 1873042

Setting needinfo? from our GC folks then. Thank you Andre for the analysis!

Flags: needinfo?(sphink)
Flags: needinfo?(jcoppeard)

Setting needinfo? from our GC folks then.

(Assuming GC-related due to lines 11 and 12 of comment 3. If it's not GC, I've put Jan on cc, so please feel free to take over)

Is a pending exception a security issue? Or just like a correctness problem?

This is a correctness problem and is not security sensitive.

Assignee: nobody → jcoppeard
Group: javascript-core-security
Flags: needinfo?(sphink)
Flags: needinfo?(jcoppeard)
Blocks: GC
Severity: -- → S3
Component: JavaScript Engine: JIT → JavaScript: GC
Priority: -- → P3

The function CreateUniqueIdForNativeObject doesn't take a JSContext but can set
a pending exception by calling NativeObject::setUniqueId. The fix is to clear
this and return false on failure as usual.

Asserting that this was always called with no exception set turned up a couple
of places where we can do this during error handling with an exception
already pending which also required fixing.

The function CreateUniqueIdForNativeObject doesn't take a JSContext but can set
a pending exception by calling NativeObject::setUniqueId. The fix is to clear
this and return false on failure as usual.

Asserting that this was always called with no exception set turned up a couple
of places where we can do this during error handling with an exception
already pending which also required fixing.

Attachment #9392276 - Attachment is obsolete: true
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7278e5cc8ae4 Clear any pending exception when creating a unique ID for a native object r=jandem
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 126 Branch
Flags: sec-bounty? → sec-bounty-

Is this something we should backport to Beta? Please nominate if yes.

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

  • If yes, please nominate the patch for beta approval. Also, don't forget to request an uplift for the patches in the regression caused by this fix.
  • If no, please set status-firefox125 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(jcoppeard)
Flags: needinfo?(jcoppeard)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: