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

RESOLVED FIXED in Firefox 68

Status

()

defect
P2
critical
RESOLVED FIXED
7 months ago
13 days ago

People

(Reporter: gkw, Assigned: jorendorff)

Tracking

(Blocks 3 bugs, 4 keywords)

Trunk
mozilla68
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 unaffected, firefox65 wontfix, firefox66 wontfix, firefox67 wontfix, firefox68 fixed)

Details

(Whiteboard: [jsbugmon:testComment=13,origRev=8ec327de0ba7])

Attachments

(4 attachments)

Reporter

Description

7 months ago
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.
Reporter

Comment 2

7 months ago
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?
Blocks: 1448880
Flags: needinfo?(jorendorff)
Assignee

Comment 3

7 months ago
It is! Thank you, fuzzers!
Assignee: nobody → jorendorff
Duplicate of this bug: 1502015
Jason, is this something you can prioritize? We have about 2.5 weeks before soft freeze for 65.
Assignee

Comment 6

6 months ago
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.
Assignee

Updated

6 months ago
Flags: needinfo?(jorendorff)
Assignee

Updated

6 months ago
Priority: -- → P1

Comment 8

6 months ago
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/12525933eb9e
Fix bug in elaborate assertion that counts step hooks. r=jimb
Assignee

Updated

5 months ago
Flags: needinfo?(jorendorff)
Priority: P1 → P2
Hey Jason, given the lowered priority on this bug, should we still track this for 65?
Flags: needinfo?(jorendorff)
Assignee

Comment 11

5 months ago
status-firefox65:fix-optional is the right setting.
Flags: needinfo?(jorendorff)

Updated

4 months ago
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
Comment hidden (obsolete)
Reporter

Comment 13

4 months ago
// 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

Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update,testComment=13,origRev=8ec327de0ba7]

Updated

3 months ago
Whiteboard: [jsbugmon:update,testComment=13,origRev=8ec327de0ba7] → [jsbugmon:testComment=13,origRev=8ec327de0ba7]

Comment 14

3 months ago
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Reporter

Comment 15

2 months ago

Jason, what's next here?

Flags: needinfo?(jorendorff)

Comment 16

a month ago

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() {}
}

Updated

a month ago
See Also: → 1539654

Comment 18

a month ago

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.

Attachment #9058853 - Attachment description: Bug 1501666: Include suspended generators in count of Debugger.Frames with onStep handlers. → Bug 1501666: Include suspended generators in count of Debugger.Frames with onStep handlers. r?jorendorff

Comment 19

25 days ago
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/42558e3db76e
Make js::AbstractGeneratorObject state checks independent. r=jorendorff

Comment 20

25 days ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 25 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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

Updated

16 days ago

Updated

15 days ago
See Also: → 1549914

Comment 22

15 days ago
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b7ce8f5ea168
Include suspended generators in count of Debugger.Frames with onStep handlers. r=jorendorff
Assignee

Updated

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