Assertion failure: zone->isGCScheduled(), at js/src/jsgc.cpp:6785

RESOLVED FIXED in Firefox 56

Status

()

defect
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gkw, Assigned: jonco)

Tracking

(Blocks 2 bugs, {assertion, jsbugmon, testcase})

Trunk
mozilla57
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 wontfix, firefox56 fixed, firefox57 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
The following testcase crashes on mozilla-central revision 3bfcbdf5c6c3 (build with --enable-debug, run with --fuzzing-safe --ion-offthread-compile=off --no-baseline --no-ion):

// jsfunfuzz-generated
setGCCallback({
    action: "majorGC"
});
// Adapted from randomly chosen test: js/src/jit-test/tests/gc/bug-1374797.js
gcparam("allocationThreshold", 1);
offThreadCompileScript("");
for (let i = 0; i < 99999; i++) {
    Symbol.for(i);
}
eval(0);


Backtrace:

#0  js::gc::GCRuntime::budgetIncrementalGC (this=0x7f8ca4b4d670, nonincrementalByAPI=<optimized out>, reason=<optimized out>, budget=..., lock=...) at js/src/jsgc.cpp:6791
#1  0x00000000009f626e in js::gc::GCRuntime::gcCycle (this=this@entry=0x7f8ca4b4d670, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::ALLOC_TRIGGER) at js/src/jsgc.cpp:6901
#2  0x00000000009f6a85 in js::gc::GCRuntime::collect (this=this@entry=0x7f8ca4b4d670, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::ALLOC_TRIGGER) at js/src/jsgc.cpp:7060
#3  0x00000000009f6ef2 in js::gc::GCRuntime::gcSlice (this=0x7f8ca4b4d670, reason=JS::gcreason::ALLOC_TRIGGER, millis=0) at js/src/jsgc.cpp:7146
#4  0x00000000009f70f1 in js::gc::GCRuntime::gcIfRequested (this=0x7f8ca4b4d670) at js/src/jsgc.cpp:7348
/snip

For detailed crash information, see attachment.

Setting s-s as a start because this seems to involve GC.
(Reporter)

Comment 2

2 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/e33e38d61829
user:        Jon Coppeard
date:        Tue Jul 25 11:28:41 2017 +0100
summary:     Bug 1374797 - Fix logic around triggering atoms GCs r=sfink

Jon, is bug 1374797 a likely regressor?
Blocks: 1374797
Flags: needinfo?(jcoppeard)
(Assignee)

Comment 3

2 years ago
The problem here is that when we're scheduling zones for GC and checking the GC budget we look at all zones, even those used by helper threads that we can't collect anyway.  So let's ignore these.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Attachment #8896961 - Flags: review?(sphink)
(Assignee)

Comment 4

2 years ago
This is not a security issue.
Group: javascript-core-security
Attachment #8896961 - Flags: review?(sphink) → review+

Comment 5

2 years ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e5d28a1a277
Ignore zones that we can't collect when scheduling zones for GC r=sfink

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2e5d28a1a277
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Please request Beta approval on this when you get a chance.
Flags: needinfo?(jcoppeard)
(Assignee)

Comment 8

2 years ago
Comment on attachment 8896961 [details] [diff] [review]
bug1390087-zone-scheduling

Approval Request Comment
[Feature/Bug causing the regression]: This has been like this since forever basically.
[User impact if declined]: We could occasionally end up doing GCs non-incrementally for no good reason.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It's a very simple change that ignores zones that we aren't going to collect anyway when budgeting GC.
[String changes made/needed]: None.
Flags: needinfo?(jcoppeard)
Attachment #8896961 - Flags: approval-mozilla-beta?
Comment on attachment 8896961 [details] [diff] [review]
bug1390087-zone-scheduling

Fix an assertion. Looks low risk. Beta56+.
Attachment #8896961 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Jon Coppeard (:jonco) from comment #8)
> [Is this code covered by automated tests?]: Yes.
> [Has the fix been verified in Nightly?]: Yes.
> [Needs manual test from QE? If yes, steps to reproduce]: No.

Setting qe-verify- based on Jon's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
You need to log in before you can comment on or make changes to this bug.