1501666 - Assertion failure: stepperCount == trappingScript->stepModeCount(), at js/src/vm/Debugger.cpp:2382

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

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

The following testcase crashes on mozilla-central revision c29f681979ee (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

// jsfunfuzz-generated
function f() {
    return (async function(y) {
        await /x/;
        await y;
    })
};
f()();
f()();
// Adapted from randomly chosen test: js/src/jit-test/tests/debug/bug1130756.js
g = newGlobal();
g.parent = this;
g.eval("(" + function() {
    dbg = Debugger(parent);
    dbg.onEnterFrame = function(frame) {
        frame.onStep = function() {}
    }
} + ")")();

Backtrace:

#0  0x0000563888d902bd in js::Debugger::onSingleStep (cx=0x7f2f29018000, vp=...) at js/src/vm/Debugger.cpp:2382
#1  0x00005638886b55d8 in Interpret (cx=0x7f2f29018000, state=...) at js/src/vm/Interpreter.cpp:2240
#2  0x00005638886b4d43 in js::RunScript (cx=0x7f2f29018000, state=...) at js/src/vm/Interpreter.cpp:447
#3  0x00005638886c74b1 in js::InternalCallOrConstruct (cx=0x7f2f29018000, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:587
#4  0x00005638886c7efd in js::Call (cx=0x7f2f2a13d680 <_IO_2_1_stderr_>, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:633
#5  0x0000563888f65e48 in js::CallSelfHostedFunction (cx=0x7f2f29018000, name=..., thisv=..., args=..., rval=...) at js/src/vm/SelfHosting.cpp:1874
#6  0x0000563888d578a4 in AsyncFunctionResume (cx=0x7f2f29018000, resultPromise=..., generatorVal=..., kind=<optimized out>, valueOrReason=...) at js/src/vm/AsyncFunction.cpp:200
#7  0x0000563888779d67 in AsyncFunctionPromiseReactionJob (cx=<optimized out>, reaction=..., rval=...) at js/src/builtin/Promise.cpp:1464
/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/6c8949f053e0
user:        Jason Orendorff
date:        Tue Oct 23 23:24:11 2018 +0000
summary:     Bug 1448880 - Part 6: Re-enable stepping when an async or generator frame with an .onStep hook is resumed. r=jimb

Jason, is bug 1448880 a likely regressor?

Comment 3

[Jason Orendorff [:jorendorff]]

It is! Thank you, fuzzers!

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Jason, is this something you can prioritize? We have about 2.5 weeks before soft freeze for 65.

Comment 6

[Jason Orendorff [:jorendorff]]

Well, the good news is it's the assertion that's wrong.

The assertion <https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/js/src/vm/Debugger.cpp#2362-2393> counts Debugger.Frames with active onStep hooks. But it doesn't count suspended generator frames.

Comment 7

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1501666 - Fix bug in elaborate assertion that counts step hooks. r?jimb

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12525933eb9e
Fix bug in elaborate assertion that counts step hooks. r=jimb

Comment 9

[Andreea Pavel [:apavel]]

Backed out for SM bustages

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&&selectedJob=214743169&revision=12525933eb9e4b77aed03ef536aac18a36f81278

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=214743169&repo=autoland&lineNumber=12253

Backout: https://hg.mozilla.org/integration/autoland/rev/2f0e92d76c4e96fe66f0aab96075c22b3692f116

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Hey Jason, given the lowered priority on this bug, should we still track this for 65?

Comment 11

[Jason Orendorff [:jorendorff]]

status-firefox65:fix-optional is the right setting.

Comment 12

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 3dc7d345da52).

Comment 13

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

// jsfunfuzz-generated
function f() {
    return (async function(y) {
        await /x/;
        await y;
    })
};
f()();
f()();
// Adapted from randomly chosen test: js/src/jit-test/tests/debug/bug1130756.js
g = newGlobal();
g.parent = this;
g.eval("(" + function() {
    dbg = Debugger(parent);
    dbg.onEnterFrame = function(frame) {
        frame.onStep = function() {}
    }
} + ")")();

asserts js shell compiled with --enable-debug on m-c rev 8ec327de0ba7 using --fuzzing-safe --no-threads --no-baseline --no-ion --more-compartments at Assertion failure: stepperCount == trappingScript->stepModeCount(), at js/src/vm/Debugger.cpp:2254

Comment 14

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 15

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

Jason, what's next here?

Comment 16

[Jim Blandy :jimb]

Slightly simplified test case, reproduces for me on 258af4e91151 (2019-4-16):

var g = newGlobal({ newCompartment: true });
g.eval(`
    async function f(y) {
        await true;
        await true;
    };
`);

g.f();
g.f();

var dbg = Debugger(g);
dbg.onEnterFrame = function(frame) {
    frame.onStep = function() {}
}

Comment 17

[Jim Blandy :jimb]

Attachment: Bug 1501666: Include suspended generators in count of Debugger.Frames with onStep handlers. r?jorendorff

Comment 18

[Jim Blandy :jimb]

Attachment: Bug 1501666: Make js::AbstractGeneratorObject state checks independent. r?jorendorff

An AbstractGeneratorObject's RESUME_INDEX_SLOT indicates the state of the
generator object: it may be undefined (before initial yield), null (closed),
or an integer (running, closing, or suspended).

AbstractGeneratorObject has a number of predicate methods to test for these
various states. Unfortunately, some of the predicates grab RESUME_INDEX_SLOT and
immediately call toInt32 on its value, which crashes if it is not an Int32.
This means the only safe way to ask if an AbstractGeneratorObject is suspended
is:

!isBeforeInitialYield() && !isClosed() && isSuspended()

If either of the first two conditions is true, isSuspended will assert. This is
verbose, and means the predicates cannot be used without studying the details of
the RESUME_INDEX_SLOT's representation.

This patch makes the predicates assertion-free. isSuspended acquires a new
branch, but the others should be just as efficient as they were before.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42558e3db76e
Make js::AbstractGeneratorObject state checks independent. r=jorendorff

Comment 20

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/42558e3db76e

Comment 21

[Pascal Chevrel:pascalc]

P2 and we shipped 65 and 66 with this bug, I think it can ride the trains given that RC week is upon us.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b7ce8f5ea168
Include suspended generators in count of Debugger.Frames with onStep handlers. r=jorendorff

Comment 23

[Daniel Varga [:dvarga]]

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