Closed Bug 1565278 Opened 5 years ago Closed 5 years ago

Crash [@ MOZ_Crash] with compartment mismatch or Assertion failure: false (cx->getPendingException(&exception)), at vm/Debugger.cpp:1888

Categories

(Core :: JavaScript Engine, defect, P1)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- fixed

People

(Reporter: decoder, Assigned: jimb)

References

(Regression)

Details

(5 keywords, Whiteboard: [jsbugmon:update] [debugger-mvp] )

Crash Data

Attachments

(1 file)

The following testcase crashes on mozilla-central revision ad05396bfeed (build with --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --disable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

evalInFrame = function(global) {
    dbgGlobal = newGlobal({
        newCompartment: true
    })
    dbg = new dbgGlobal.Debugger
    return function(upCount, code) {
        dbg.addDebuggee(global)
        var frame = dbg.getNewestFrame().older
        if (new f >> 0) frame = older
    }
}(this);
function h() {
    evalInFrame(true, "")
}
function g() {
    return h();
}
function f() {
    return g();
}
f()

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  MOZ_Crash (aReason=0x55555766b0a0 <sPrintfCrashReason> "*** Compartment mismatch 0x7ffff4ce5350 vs. 0x7ffff4ce5580 at argument 0", aLine=45, aFilename=<synthetic pointer>) at dist/include/mozilla/Assertions.h:313
#1  js::ContextChecks::fail (argIndex=<optimized out>, c2=<optimized out>, c1=<optimized out>) at js/src/vm/JSContext-inl.h:44
#2  js::ContextChecks::check (this=<optimized out>, c=<optimized out>, argIndex=<optimized out>) at js/src/vm/JSContext-inl.h:60
#3  0x00005555559181d2 in js::ContextChecks::check (this=<optimized out>, c=<optimized out>, argIndex=<optimized out>) at js/src/vm/JSContext-inl.h:58
#4  0x00005555559b743a in JSContext::checkImpl<JS::Handle<JS::Value>>(int, JS::Handle<JS::Value> const&) (head=<synthetic pointer>, argIndex=0, this=0x7ffff5f18000) at js/src/vm/JSContext-inl.h:185
#5  JSContext::check<JS::Handle<JS::Value> > (this=0x7ffff5f18000) at js/src/vm/JSContext-inl.h:193
#6  JSContext::setPendingException (this=0x7ffff5f18000, v=v@entry=..., stack=stack@entry=...) at js/src/vm/JSContext-inl.h:341
#7  0x00005555559aa35e in js::Debugger::slowPathOnLeaveFrame (cx=<optimized out>, cx@entry=0x7ffff5f18000, frame=..., pc=pc@entry=0x7ffff5f045f9 "R", frameOk=frameOk@entry=false) at js/src/vm/Debugger.cpp:1087
#8  0x0000555555fb2881 in js::Debugger::onLeaveFrame (ok=false, pc=0x7ffff5f045f9 "R", frame=..., cx=0x7ffff5f18000) at js/src/vm/Debugger-inl.h:30
#9  js::jit::HandleExceptionBaseline (pc=<optimized out>, rfe=0x7fffffefe288, frame=..., cx=0x7ffff5f18000) at js/src/jit/JitFrames.cpp:522
#10 js::jit::HandleException (rfe=<optimized out>) at js/src/jit/JitFrames.cpp:703
#11 0x000011d44ee81fab in ?? ()
[...]
#79 0x0000000000000000 in ?? ()
rax	0x55555766b0a0	93825026928800
rbx	0x7ffff5f18000	140737319632896
rcx	0x0	0
rdx	0x55555766b080	93825026928768
rsi	0x0	0
rdi	0x7fffffefd750	140737487296336
rbp	0x7fffffefdaf0	140737487297264
rsp	0x7fffffefd9b0	140737487296944
r8	0x0	0
r9	0x48	72
r10	0x0	0
r11	0x0	0
r12	0x7fffffefdb10	140737487297296
r13	0x1	1
r14	0xfff9800000000000	-1829587348619264
r15	0x0	0
rip	0x5555558e5e06 <js::ContextChecks::check(JS::Compartment*, int)+54>
=> 0x5555558e5e06 <js::ContextChecks::check(JS::Compartment*, int)+54>:	movl   $0x0,0x0
   0x5555558e5e11 <js::ContextChecks::check(JS::Compartment*, int)+65>:	ud2
Type: task → defect

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/f0aa53a43409
user: Jim Blandy
date: Sun Jul 07 17:03:57 2019 +0000
summary: Bug 1470558: Distinguish yields and awaits in completion values. r=jorendorff

Jim, is bug 1470558 a likely regressor?

Flags: needinfo?(jimb)
Regressed by: 1470558
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Priority: -- → P1

I can reproduce this. Yes, I think 1470558 is probably the regressor.

Assignee: nobody → jimb
Flags: needinfo?(jimb)

Here's a reduced case that fails more quickly:

// |jit-test| --no-baseline; --no-ion

var dbgGlobal = newGlobal({ newCompartment: true });
var dbg = new dbgGlobal.Debugger();
dbg.addDebuggee(this);

function recur(n=0) {
  dbg.getNewestFrame();
  if (n < 70)
    recur(n+1);
}

stackTest(recur, { keepFailing: true, expectExceptionOnFailure: false });

The test cases from the fuzzer take a long time to crash, and an even longer time to pass (> 60s), so I don't want to include them in the patch. The following test case crashes or passes in a few seconds, so it's what I'm going to include in the patch:

// Failure to rewrap an exception in Completion::fromJSResult should be propagated.

var dbgGlobal = newGlobal({ newCompartment: true });
var dbg = new dbgGlobal.Debugger();
dbg.addDebuggee(this);

function test() {
  // Make this call's stack frame a debuggee, to ensure that
  // slowPathOnLeaveFrame runs when this frame exits. That calls
  // Completion::fromJSResult to capture this frame's completion value.
  dbg.getNewestFrame();

  // Throw from the non-debuggee compartment, to force Completion::fromJSResult
  // to rewrap the exception.
  dbgGlobal.assertEq(1,2);
}

stackTest(test, {
  // When the bug is fixed, the failure to rewrap the exception turns the throw
  // into a termination, so we won't get an exception.
  expectExceptionOnFailure: false
});

The fuzzer's test case from bug 1576862 also takes a long time to crash and a long time to pass, so I'm not going to include it.

cx->getPendingException can fail if re-wrapping the exception for the current
compartment fails, which it can for OOM or stack overflow. Rather than applying
MOZ_ALWAYS_TRUE to the success code, Completion::fromJSResult should preserve
prior behavior by turning failure at that point into a Terminate completion.

Original fuzzer test cases take a long time to fail and an even longer time to
pass, so they're not included in this patch. The provided test detects the
presence and absence of the fix in a few seconds.

That try push doesn't handle builds that don't include testing functions.
New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7d06d963fa673192006d6573fc1852aed4c3308

Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e14e78de48d
Completion::fromJSResult should convert getPendingException failure into termination. r=jorendorff
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Flags: in-testsuite+

Is this something we should consider for Beta backport or can it ride Fx71 to release?

Flags: needinfo?(jimb)
Blocks: dbg-71
Whiteboard: [jsbugmon:update] → [jsbugmon:update] [debugger-mvp]

I think it can ride the trains.

Flags: needinfo?(jimb)
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: