1628440 - Crash [@ js::gc::TenuredCell::writeBarrierPre(js::gc::TenuredCell*)] with weak refs and use-after-free

Status: VERIFIED FIXED

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

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200408-6663d3dc883b (build with --enable-debug, run with --fuzzing-safe --no-threads --enable-weak-refs):

gczeal(10, 2);
let cleanup = function(iter) {}
let key = {"k": "this is my key"};
let fg = new FinalizationRegistry(cleanup);
let object = {};
fg.register(object, {}, key);
let success = fg.unregister(key);
throw "x";

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00005555557d2a93 in js::gc::TenuredCell::writeBarrierPre(js::gc::TenuredCell*) ()
#1  0x00005555559c45e5 in mozilla::detail::VectorImpl<js::HeapPtr<js::FinalizationRecordObject*>, 1ul, js::ZoneAllocPolicy, false>::destroy(js::HeapPtr<js::FinalizationRecordObject*>*, js::HeapPtr<js::FinalizationRecordObject*>*) ()
#2  0x00005555559e2d83 in mozilla::Vector<js::HeapPtr<js::FinalizationRecordObject*>, 1ul, js::ZoneAllocPolicy>::~Vector() ()
#3  0x00005555559895f9 in js::FinalizationRecordVectorObject::finalize(JSFreeOp*, JSObject*) ()
#4  0x00005555562bc24a in JSObject::finalize(JSFreeOp*) ()
#5  0x00005555562bb8ef in unsigned long js::gc::Arena::finalize<JSObject>(JSFreeOp*, js::gc::AllocKind, unsigned long) ()
#6  0x00005555562bb5ba in bool FinalizeTypedArenas<JSObject>(JSFreeOp*, js::gc::Arena**, js::gc::SortedArenaList&, js::gc::AllocKind, js::SliceBudget&) ()
#7  0x00005555562809ed in FinalizeArenas(JSFreeOp*, js::gc::Arena**, js::gc::SortedArenaList&, js::gc::AllocKind, js::SliceBudget&) ()
#8  0x000055555627fe92 in js::gc::ArenaLists::backgroundFinalize(JSFreeOp*, js::gc::Arena*, js::gc::Arena**) ()
#9  0x000055555628547a in js::gc::GCRuntime::sweepBackgroundThings(js::gc::ZoneList&, js::LifoAlloc&) ()
#10 0x0000555556286309 in js::gc::GCRuntime::sweepFromBackgroundThread(js::AutoLockHelperThreadState&) ()
#11 0x0000555556286081 in js::gc::BackgroundSweepTask::run() ()
#12 0x00005555562b430c in js::GCParallelTask::runTask() ()
#13 0x0000555556285d3d in js::gc::GCRuntime::queueZonesAndStartBackgroundSweep(js::gc::ZoneList&) ()
#14 0x000055555629c8f8 in js::gc::GCRuntime::endSweepingSweepGroup(JSFreeOp*, js::SliceBudget&) ()
#15 0x00005555562dbc41 in sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&) ()
#16 0x00005555562caf87 in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&) ()
#17 0x000055555629fca5 in js::gc::GCRuntime::performSweepActions(js::SliceBudget&) ()
#18 0x00005555562a4949 in js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason, js::gc::AutoGCSession&) ()
#19 0x00005555562a779a in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) ()
#20 0x00005555562a9410 in js::gc::GCRuntime::collect(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) ()
#21 0x00005555562711bd in js::gc::GCRuntime::finishGC(JS::GCReason) ()
#22 0x0000555556278dcc in js::gc::FinishGC(JSContext*, JS::GCReason) ()
#23 0x00005555557914ab in mozilla::ScopeExit<main::$_5>::~ScopeExit() ()
#24 0x00005555557882aa in main ()
rax	0xbefb96a7000	13124235849728
rbx	0xbefb96a7040	13124235849792
rcx	0xa7040	684096
rdx	0xbefb9600000	13124235165696
rsi	0x1	1
rdi	0x1	1
rbp	0x7fffffffbb30	140737488337712
rsp	0x7fffffffbb10	140737488337680
r8	0x7fffffffbdd0	140737488338384
r9	0x0	0
r10	0x7ffff5e00000	140737318486016
r11	0x246	582
r12	0x7fffffffceb0	140737488342704
r13	0xfffe4b4b4b4b4b4b	-480163195565237
r14	0xfffe1b9b9b9b9b9b	-532594808874085
r15	0x7fffffffbb48	140737488337736
rip	0x5555557d2a93 <js::gc::TenuredCell::writeBarrierPre(js::gc::TenuredCell*)+99>
=> 0x5555557d2a93 <_ZN2js2gc11TenuredCell15writeBarrierPreEPS1_+99>:	cmpl   $0x0,0x10(%r14)
   0x5555557d2a98 <_ZN2js2gc11TenuredCell15writeBarrierPreEPS1_+104>:	je     0x5555557d2b28 <_ZN2js2gc11TenuredCell15writeBarrierPreEPS1_+248>

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1628440 - Use WeakHeapPtr for weakly-held vector elements in finalization registry registrations weakmap r?sfink

The problem is that HeapPtr<> has a prebarrier in its destructor. This isn't necessary for weakly held values, and cause problems if the values have already been finalized. The patch also renames FinalizationRecordVectorObject to FinalizationRegistrationsObject to better describe its purpose.

Comment 3

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

https://hg.mozilla.org/integration/autoland/rev/d4be324b80a403215e60c3462272fbc46241d3cd
https://hg.mozilla.org/mozilla-central/rev/d4be324b80a4

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 5

[Jon Coppeard (:jonco)]

This feature is not exposed on beta so I think we can leave this.

Comment 6

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

The patch landed in nightly and beta is affected.
:jonco, 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 7

[Julien Cristau [:jcristau] (back April 22)]

marking as disabled in beta per comment 5.

Comment 8

[Jason Kratzer [:jkratzer]]

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