1885775 - Crash [@ bool js::GCMarker::processMarkStackTop] with symbols as weakmap keys

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20240306-099f52f30c39 (opt build, run with --fuzzing-safe --ion-offthread-compile=off --enable-symbols-as-weakmap-keys --more-compartments):

var code = `
var m58 = new WeakMap;
var sym = Symbol();
m58.set(sym, ({ entry16: 0, length: 1 }));
function testCompacting() {
  gcslice(50000);
}
testCompacting(2, 100000, 50000);
`;
for (x = 0; x < 10000; ++x)
  evaluate(code);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x000055555743fcd4 in bool js::GCMarker::processMarkStackTop<4u>(js::SliceBudget&) ()
#1  0x0000555557439039 in js::GCMarker::markUntilBudgetExhausted(js::SliceBudget&, js::gc::ShouldReportMarkTime) ()
#2  0x000055555746ee79 in js::gc::IncrementalProgress js::gc::GCRuntime::markWeakReferences<js::gc::SweepGroupZonesIter>(js::SliceBudget&) ()
#3  0x0000555557470643 in js::gc::GCRuntime::endMarkingSweepGroup(JS::GCContext*, js::SliceBudget&) ()
#4  0x0000555557478a95 in sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&) ()
#5  0x0000555557478279 in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&) ()
#6  0x000055555740c8dc in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget const&, JS::GCReason) ()
#7  0x00005555574096a3 in js::gc::GCRuntime::collect(bool, js::SliceBudget const&, JS::GCReason) ()
#8  0x000055555729a114 in GCSlice(JSContext*, unsigned int, JS::Value*) ()
#9  0x0000555556d869fd in js::Interpret(JSContext*, js::RunState&) ()
#10 0x0000555556d679d6 in js::RunScript(JSContext*, js::RunState&) ()
#11 0x0000555556d675bd in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) ()
#12 0x000055555714b7c6 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) ()
#13 0x00005555570225e4 in Evaluate(JSContext*, unsigned int, JS::Value*) ()
#14 0x0000219141fbe182 in ?? ()
#15 0x00007fffffffc158 in ?? ()
#16 0x00007fffffffbcf0 in ?? ()
#17 0x0000000000000000 in ?? ()
rax	0x16969	92521
rbx	0x7ffff3e13070	140737285009520
rcx	0x969696969696969	678189120356968809
rdx	0x4b4b4b4b4b400000	5425512962855010304
rsi	0x4b4b4b4b4b4b4b4b	5425512962855750475
rdi	0x20000000000	2199023255552
rbp	0x7fffffffa630	140737488332336
rsp	0x7fffffffa5c0	140737488332224
r8	0x1a86ab02	445033218
r9	0x5a5	1445
r10	0x7ffff3e1b000	140737285042176
r11	0x7ffff3e13030	140737285009456
r12	0x1bd686f6a060	30608201261152
r13	0x7ffff3e13090	140737285009552
r14	0x2	2
r15	0x7fffffffa970	140737488333168
rip	0x55555743fcd4 <bool js::GCMarker::processMarkStackTop<4u>(js::SliceBudget&)+548>
=> 0x55555743fcd4 <_ZN2js8GCMarker19processMarkStackTopILj4EEEbRNS_11SliceBudgetE+548>:	mov    -0xc0(%rdx,%r9,8),%r10
   0x55555743fcdc <_ZN2js8GCMarker19processMarkStackTopILj4EEEbRNS_11SliceBudgetE+556>:	bt     %rcx,%r10

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Unable to reproduce bug 1885775 using build mozilla-central 20240306044140-099f52f30c39. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 4

[Jon Coppeard (:jonco)]

Setting severity to S4 as this feature is not enabled by default.

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Hey, given the relationship to Symbols As Weak Map Keys, could you take a look Yoshi?

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

The crash stack looks like bug 1884927

Comment 7

[BugBot [:suhaib / :marco]]

The severity field for this bug is set to S4. However, the bug is flagged with the sec-high keyword.
:allstars.chh, could you consider increasing the severity of this security bug?

For more information, please visit BugBot documentation.

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

This feature is still behind a pref

Comment 9

[Andrew McCreight [:mccr8]]

We mark the ratings for security issues with disabled features as if they were enabled. This does unfortunately rile up the bots, but feel free to ignore them.

Comment 10

[Steven DeTar [:sdetar]]

Setting this back to an S2, P1 as this is a sec-high bug and based off :mccr8 comments in Comment 9.

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Bug 1885775 - addSweepGroupEdgeTo Weakmap's zone if the key is of a JS::Symbol.

Comment 13

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Bug 1885775 - tests.

Comment 14

[BugBot [:suhaib / :marco]]

Copying crash signatures from duplicate bugs.

Comment 15

[Jon Coppeard (:jonco)]

I hadn't realised before but enabling symbols as weakmap keys could make incremental sweeping use much longer slices and so increase jank. This is this feature adds a possible path from any zone to any other via a symbol in the atoms zone. Previously there were no pointers out of the atoms zone.

Comment 16

[BugBot [:suhaib / :marco]]

Copying crash signatures from duplicate bugs.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e3ed14be1b9
addSweepGroupEdgeTo Weakmap's zone if the key is of a JS::Symbol. r=jonco

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5e3ed14be1b9

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a56ecbf359a
tests. r=jonco

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2a56ecbf359a