1652425 - Assertion failure: lock_.ownedByCurrentThread(), at gc/StoreBuffer.h:397

Status: VERIFIED FIXED

(Core :: JavaScript: GC, defect, P1)

Description

[Christian Holler (:decoder)]

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

function varying(mapColor, keyColor) {
  enqueueMark(`set-color-${keyColor}`);
  enqueueMark("yield");
  startgc(100000);
}
for (const mapColor of ['gray', 'black']) {
  for (const keyColor of ['gray', 'black', 'unmarked'])
    varying(mapColor, keyColor);
}

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00005555559629f1 in void js::gc::StoreBuffer::put<js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::ValueEdge>, js::gc::StoreBuffer::ValueEdge>(js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::ValueEdge>&, js::gc::StoreBuffer::ValueEdge const&) ()
#1  0x00005555559626c9 in js::InternalBarrierMethods<JS::Value>::postBarrier(JS::Value*, JS::Value const&, JS::Value const&) ()
#2  0x0000555555d51160 in JS::GCVector<JS::Heap<JS::Value>, 0ul, js::SystemAllocPolicy>::sweep() ()
#3  0x0000555555d50e5d in JS::WeakCache<JS::GCVector<JS::Heap<JS::Value>, 0ul, js::SystemAllocPolicy> >::sweep(js::gc::StoreBuffer*) ()
#4  0x000055555626aa67 in ImmediateSweepWeakCacheTask::run() ()
#5  0x000055555625403c in js::GCParallelTask::runTask() ()
#6  0x0000555556253dba in js::GCParallelTask::runFromHelperThread(js::AutoLockHelperThreadState&) ()
#7  0x0000555555b848ae in js::HelperThread::handleGCParallelWorkload(js::AutoLockHelperThreadState&) ()
#8  0x0000555555b86925 in js::HelperThread::threadLoop() ()
#9  0x0000555555b81695 in js::HelperThread::ThreadMain(void*) ()
#10 0x0000555555ba79f4 in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start(void*) ()
#11 0x00007ffff7bc16ba in start_thread (arg=0x7ffff520d700) at pthread_create.c:333
#12 0x00007ffff6e4641d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
rax	0x555556fcc147	93825019986247
rbx	0x7ffff602c488	140737320764552
rcx	0x5555583ca840	93825040951360
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7ffff520c680	140737305953920
rsp	0x7ffff520c660	140737305953888
r8	0x7ffff7105770	140737338431344
r9	0x7ffff520d700	140737305958144
r10	0x0	0
r11	0x0	0
r12	0x7ffff520c6d8	140737305954008
r13	0xfff9800000000000	-1829587348619264
r14	0x7ffff602c4e0	140737320764640
r15	0x7ffff520c698	140737305953944
rip	0x5555559629f1 <void js::gc::StoreBuffer::put<js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::ValueEdge>, js::gc::StoreBuffer::ValueEdge>(js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::ValueEdge>&, js::gc::StoreBuffer::ValueEdge const&)+81>
=> 0x5555559629f1 <_ZN2js2gc11StoreBuffer3putINS1_14MonoTypeBufferINS1_9ValueEdgeEEES4_EEvRT_RKT0_+81>:	movl   $0x18d,0x0
   0x5555559629fc <_ZN2js2gc11StoreBuffer3putINS1_14MonoTypeBufferINS1_9ValueEdgeEEES4_EEvRT_RKT0_+92>:	callq  0x5555558492fe <abort>

Could be a shell-only problem with enqueueMark but filing s-s for now until investigated further.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Jon Coppeard (:jonco)]

This is only a problem with enqueMark because that's the only thing that uses WeakCache<GCVector<>> at the moment.

Comment 3

[Jon Coppeard (:jonco)]

Shell-only so not security sensitive.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1652425 - Make WeakCache::sweep conservatively take the store buffer lock r?sfink

Comment 5

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

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

Comment 6

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200713095122-2c8bc998c107.
The bug appears to have been introduced in the following build range:
> Start: 74f73190afad2bbde97bd6009430b87445718a01 (20200616184413)
> End: 934e959205abe817c23015c326cff2413e1f040c (20200616184513)
> Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=74f73190afad2bbde97bd6009430b87445718a01&tochange=934e959205abe817c23015c326cff2413e1f040c

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a624af0525cc
Make WeakCache::sweep conservatively take the store buffer lock r=sfink

Comment 8

[Dorel Luca [:dluca]]

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

Comment 9

[Jason Kratzer [:jkratzer]]

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

Comment 10

[(PTO June 19-July 7) Ryan VanderMeulen]

I'm assuming this doesn't need uplift to 79. Please let me know ASAP if you disagree.

Comment 11

[Jon Coppeard (:jonco)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
This is shell only so no uplift required.