1766648 - Assertion failure: !hasBlackEntries(), at js/src/gc/Marking.cpp:2469

Status: VERIFIED FIXED

(Core :: JavaScript: GC, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20220427-139c89a60b72 (debug build, run with --fuzzing-safe --ion-offthread-compile=off):

function a() {
  enqueueMark('set-color-gray');
  enqueueMark('set-color-black');
  enqueueMark(newGlobal());
  startgc();
}
a();
a();
a();

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0000555557593ed1 in js::GCMarker::setMarkColor(js::gc::MarkColor) ()
#1  0x000055555759284c in js::GCMarker::processMarkQueue() ()
#2  0x0000555557595a98 in js::GCMarker::enterWeakMarkingMode() ()
#3  0x00005555575e106a in js::gc::IncrementalProgress js::gc::GCRuntime::markWeakReferences<js::gc::SweepGroupZonesIter>(js::SliceBudget&) ()
#4  0x00005555575e6af5 in js::gc::GCRuntime::endMarkingSweepGroup(JS::GCContext*, js::SliceBudget&) ()
#5  0x00005555576099f1 in sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&) ()
#6  0x00005555575ff113 in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&) ()
#7  0x00005555575ee6da in js::gc::GCRuntime::performSweepActions(js::SliceBudget&) ()
#8  0x00005555575556d9 in js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, JS::GCReason, bool) ()
#9  0x0000555557559461 in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget const&, JS::GCReason) ()
#10 0x000055555755a6a6 in js::gc::GCRuntime::collect(bool, js::SliceBudget const&, JS::GCReason) ()
#11 0x0000555557011cf5 in StartGC(JSContext*, unsigned int, JS::Value*) ()
[...]
#23 0x0000555556b43362 in main ()
rax	0x555555852dda	93824995372506
rbx	0x7ffff6019508	140737320686856
rcx	0x5555581d5ae8	93825038899944
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffb990	140737488337296
rsp	0x7fffffffb970	140737488337264
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99840	140737353717824
r10	0x0	0
r11	0x0	0
r12	0xf6018701	4127295233
r13	0x7ffff603e100	140737320837376
r14	0x1	1
r15	0x1	1
rip	0x555557593ed1 <js::GCMarker::setMarkColor(js::gc::MarkColor)+353>
=> 0x555557593ed1 <_ZN2js8GCMarker12setMarkColorENS_2gc9MarkColorE+353>:	movl   $0x9a5,0x0
   0x555557593edc <_ZN2js8GCMarker12setMarkColorENS_2gc9MarkColorE+364>:	callq  0x555556bd9e68 <abort>

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220427094429-139c89a60b72.
The bug appears to have been introduced in the following build range:

Start: a27f2f698323860d680f0d042d0a833411935057 (20220202172237)
End: 26438f963a5ffab579ede7738679bc2ae34102e2 (20220202173131)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a27f2f698323860d680f0d042d0a833411935057&tochange=26438f963a5ffab579ede7738679bc2ae34102e2

Comment 4

[Jon Coppeard (:jonco)]

Looks like an issue with processing the mark queue (this is a debug only test feature).

Comment 5

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1766648 - Disallow marking black if marking will revert to gray

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

:sfink, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1751959

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0ac2dc764de
Disallow marking black if marking will revert to gray r=jonco

Comment 9

[Marian-Vasile Laza]

Backed out changeset d0ac2dc764de (Bug 1766648) for causing sm bustage on bug-1766648-markQueue.js.
Backout link
Push with failures <--> cgc
Failure Log

Comment 10

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1766648 - Disallow marking black if marking will revert to gray

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e530e167af3
Disallow marking black if marking will revert to gray r=jonco

Comment 12

[Atila Butkovits]

Backed out for causing SM bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/90203d1ec1aa31c4ea8fdd166c19cb246d1a8f1c

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=0e530e167af31096cb6c9e1c68d56f35f6d940e2&selectedTaskRun=dq6HNSC-R5iEc9c7__xBDw.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=376512219&repo=autoland&lineNumber=21084

Comment 13

[Steve Fink [:sfink] [:s:]]

Sorry, I ended up pushing the same broken patch again instead of my fixed version, because the fixed version somehow ended up with a different phabricator revision and I pushed with the old (correct) one.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f33ea198bbb7
Disallow marking black if marking will revert to gray r=jonco

Comment 15

[Cristian Tuns]

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

Comment 16

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20220503094208-03cd547ca0b9.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:sfink, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 18

[Steve Fink [:sfink] [:s:]]

Comment on attachment 9274313 [details]
Bug 1766648 - Disallow marking black if marking will revert to gray

Beta/Release Uplift Approval Request

• User impact if declined: None. It is a bug in test-only code. The reason to take this would be for fuzzing; if someone is fuzzing the code without the fix, it is possible (and not even that unlikely) to hit this assertion (or get bad behavior in a non-DEBUG build) from generated test cases. It wasn't marked as a fuzzblocker, so it may not be that common a problem, but a fuzzer that explored this part of the test-exposed functionality could easily start hitting it.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It can only be triggered by test code.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9274313 [details]
Bug 1766648 - Disallow marking black if marking will revert to gray

Doesn't sound like we need to worry about this if it only affects fuzzers. Thanks!