1647309 - Assertion failure: genObj.isSuspended(), at debugger/Debugger.cpp:2576

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200622-24787602a9f6 (debug build, run with --fuzzing-safe --ion-offthread-compile=off --blinterp-eager):

const g = newGlobal({ newCompartment: true });
const dbg = new Debugger(g);
g.eval(`
  async function* f() {
    await null;
  }
`);
dbg.onEnterFrame = f => {};
const it = g.f();
let promise = it.next();
interruptIf(true);
drainJobQueue();
g.eval("function h() { debugger; }");
var hits = 0;
dbg.onDebuggerStatement = function (frame) {
  hits++;
    var rv = frame.eval("h();");
    frame.older.onStep = function () { return null; };
  }
g.h();

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0000555555fc5a64 in js::DebugAPI::onSingleStep(JSContext*) ()
#1  0x0000555555929b42 in Interpret(JSContext*, js::RunState&) ()
#2  0x0000555555928c42 in js::RunScript(JSContext*, js::RunState&) ()
#3  0x00005555559408a5 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) ()
#4  0x0000555555ffbb9a in js::DebuggerGenericEval(JSContext*, mozilla::Range<char16_t const>, JS::Handle<JSObject*>, js::EvalOptions const&, js::Debugger*, JS::Handle<JSObject*>, js::FrameIter*) ()
#5  0x0000555555ffc5b7 in js::DebuggerFrame::eval(JSContext*, JS::Handle<js::DebuggerFrame*>, mozilla::Range<char16_t const>, JS::Handle<JSObject*>, js::EvalOptions const&) ()
#6  0x0000555556000832 in js::DebuggerFrame::CallData::evalMethod() ()
#7  0x00005555560043e7 in bool js::DebuggerFrame::CallData::ToNative<&js::DebuggerFrame::CallData::evalMethod>(JSContext*, unsigned int, JS::Value*) ()
#8  0x00003558d25052ff in ?? ()
[...]
#11 0x0000000000000000 in ?? ()
rax	0x5555570465ed	93825020487149
rbx	0x37386a900960	60715445520736
rcx	0x555558358840	93825040484416
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffe02c10	140737486269456
rsp	0x7fffffe02630	140737486267952
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f9bd40	140737353727296
r10	0x0	0
r11	0x0	0
r12	0xfffe000000000000	-562949953421312
r13	0xffffffffffffff	72057594037927935
r14	0xfffe000000000000	-562949953421312
r15	0x2b3ac7987100	47531456753920
rip	0x555555fc5a64 <js::DebugAPI::onSingleStep(JSContext*)+5332>
=> 0x555555fc5a64 <_ZN2js8DebugAPI12onSingleStepEP9JSContext+5332>:	movl   $0xa10,0x0
   0x555555fc5a6f <_ZN2js8DebugAPI12onSingleStepEP9JSContext+5343>:	callq  0x55555584464e <abort>

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200622154848-fa015682f653.
The bug appears to have been introduced in the following build range:
> Start: 16da75efcffe8c6cb038fbd4651c30e6dd79e7f5 (20190713101518)
> End: 467644cd89e8174f92825611d0888ee81403117d (20190713112317)
> Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=16da75efcffe8c6cb038fbd4651c30e6dd79e7f5&tochange=467644cd89e8174f92825611d0888ee81403117d

Comment 3

[Ted Campbell [:tcampbell]]

Taking a look at this

Comment 4

[Ted Campbell [:tcampbell]]

const g = newGlobal({ newCompartment: true });
const dbg = new Debugger(g);

// Define async generator in debuggee compartment
g.eval("async function* f() { await null; }");

// ???
dbg.onEnterFrame = () => {};

// Trigger an interrupt durring AsyncGeneratorNext
interruptTest(function() {
    g.f().next();
    drainJobQueue();
}, {expectExceptionOnFailure: false})

// Trigger DebugAPI::onSingleStep to check generatorFrames.
dbg.onDebuggerStatement = frame => { frame.onStep = () => {}; }
g.eval("debugger");

Here is a no-JIT testcase. This involves debugger, async-generators, and interrupts still and is related to unbalanced enter/leave frame calls for debugger.

Comment 5

[Ted Campbell [:tcampbell]]

This actually seems to go back to Bug 1551176 which added the assert and removed the workaround. I think I need to understand the thinking behind that bug to understand what the appropriate fix is.

As well, the interruptTest in Comment 4 can be replaced by a simpler oomTest:

// Trigger failure in AsyncGeneratorNext
ignoreUnhandledRejections();
oomTest(function() { g.f().next(); });

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1647309 - Account for closed generators in debugger stepperCount assert. r?loganfsmyth

Errors during async generator operations can close the generator but leave
entries in the Debugger::generatorFrames map. This trips up asserts in the
single-step code. Since a closed generator will not match the targettedScript
we simply ignore such entries while checking the assert.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fdf48de3a69e
Account for closed generators in debugger stepperCount assert. r=loganfsmyth

Comment 8

[Natalia Csoregi [:nataliaCs]]

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

Comment 9

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20200630020930-79d69f36a220.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.