1591342 - Assertion failure: UncheckedUnwrap(wrappedDebugger) == debugger->object, at js/src/debugger/Debugger.cpp:356

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

The following testcase crashes on mozilla-central revision 11b08c1b0061 (build with --enable-debug --disable-optimize, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

// jsfunfuzz-generated
newGlobal();
oomTest(nukeAllCCWs);
// Adapted from randomly chosen testcase: js/src/jit-test/tests/debug/clear-old-analyses-02.js
var g = newGlobal({
    newCompartment: true
});
var dbg = Debugger();
gw = dbg.addDebuggee(g);
g.eval("" + function fib() {});
gw.makeDebuggeeValue(g.fib).script.setBreakpoint(0, {});

Backtrace:

#0  0x0000559bcaca9c48 in js::Breakpoint::Breakpoint (this=0x7f66f1be50c0, debugger=0x7f66f1d73000, wrappedDebugger=..., site=0x7f66f1bcf5e0, handler=...) at js/src/debugger/Debugger.cpp:356
#1  0x0000559bcadbfc22 in js::MallocProvider<JS::Zone>::new_<js::Breakpoint, js::Debugger*&, JS::Rooted<JSObject*>&, js::JSBreakpointSite*&, JS::Rooted<JSObject*>&> (this=0x7f66f1bd9000, args=..., args=..., args=..., args=...) at js/src/vm/MallocProvider.h:196
#2  0x0000559bcadbf992 in js::DebuggerScript::SetBreakpointMatcher::match (this=0x7ffd53117088, script=...) at js/src/debugger/Script.cpp:2095
#3  0x0000559bcadbfd2e in js::DebuggerScript::SetBreakpointMatcher::match (this=0x7ffd53117088, lazyScript=...) at js/src/debugger/Script.cpp:2108
#4  0x0000559bcadbfae0 in JS::detail::GCVariantImplementation<js::LazyScript*, js::WasmInstanceObject*>::match<js::DebuggerScript::SetBreakpointMatcher, mozilla::Variant<JSScript*, js::LazyScript*, js::WasmInstanceObject*> > (matcher=..., v=...) at /home/ubuntu/shell-cache/js-dbg-optDisabled-64-linux-x86_64-11b08c1b0061/objdir-js/dist/include/js/GCVariant.h:98
/snip

For detailed crash information, see attachment.

Comment 1

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

Attachment: Detailed Crash Information

Comment 2

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

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/843d64235cfa
user: Jim Blandy
date: Wed Oct 23 19:50:02 2019 +0000
summary: Bug 1586452: Let JSScripts and wasm::Instances own their BreakpointSites and Breakpoints. r=jonco

Jim, is bug 1586452 a likely regressor?

Comment 3

[Jim Blandy :jimb]

Yes! And this is probably a test case that reproduces bug 1591080! Superb!

I can reproduce; taking.

Comment 5

[Intermittent Failures Robot]

1 failures in 4182 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1

Platform breakdown:

• windows10-64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1591342&startday=2019-10-21&endday=2019-10-27&tree=all

Comment 6

[Jim Blandy :jimb]

The issue here is that the compartment containing the Debugger has nuked all incoming cross-compartment wrappers, meaning that the breakpoint cannot create cross-compartment wrappers for the Breakpoint's reference to the Debugger and breakpoint handler. It gets DeadObject proxies instead, which don't dereference well.

Comment 7

[Jim Blandy :jimb]

Attachment: Bug 1591342: When setting breakpoints, require usable cross-compartment wrappers. r?jonco

When the Debugger API sets a breakpoint in a JSScript or wasm::Instance, the
BreakpointSite and Breakpoint objects belong to the code's compartment
(logically, at least - they're C++ objects and don't actually have any
compartment). Since a Debugger and its debuggees must be in separate
compartments, the Breakpoint's references to its owning Debugger and its
handler object must go through cross-compartment wrappers.

If we have nuked the Debugger's compartment, it's not clear how we're still
trying to set breakpoints in its debuggees, but we should at least throw an
error, to capture a JavaScript stack when it occurs.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f42214158987
When setting breakpoints, require usable cross-compartment wrappers. r=jonco

Comment 9

[Narcis Beleuzu [:NarcisB]]

Backed out for SM bustages on bug1591342.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/2b7a0ce8428a15c0e767c552ca334e75e3e949c9
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274010628&repo=autoland&lineNumber=26480

Comment 10

[Jim Blandy :jimb]

When the shell is passed the --more-compartments flag, the test fails.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e3b05f8385fa
When setting breakpoints, require usable cross-compartment wrappers. r=jonco

Comment 12

[Andreea Pavel [:apavel]]

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