Skip to content

Commit c9880de

Browse files
Mark LamJonWBedard
authored andcommitted
CloneDeserializer::deserialize() should store cell pointers in a MarkedVector.
https://bugs.webkit.org/show_bug.cgi?id=254797 rdar://107459160 Reviewed by Justin Michaud. Previously, CloneDeserializer::deserialize() was storing pointers to newly created objects in a few Vectors. This is problematic because the GC is not aware of Vectors, and cannot scan them. In this patch, we refactor the MarkedArgumentBuffer class into a MarkedVector template class that offer 2 enhancements: 1. It can be configured to store specific types of cell pointer types. This avoids us having to constantly cast JSValues into these pointers. 2. It allows us to specify the type of OverflowHandler we want to use. In this case, we want to use CrashOnOverflow. The previous MarkedArgumentBuffer always assumes RecordOnOverflow. This allows us to avoid having to manually check for overflows, or have to use appendWithCrashOnOverflow. For our current needs, MarkedVector can be used as a drop in replacement for Vector. And we fix the CloneDeserializer::deserialize() issue by replacing the use of Vectors with MarkedVector instead. * Source/JavaScriptCore/heap/Heap.cpp: (JSC::Heap::addCoreConstraints): * Source/JavaScriptCore/heap/Heap.h: * Source/JavaScriptCore/heap/HeapInlines.h: * Source/JavaScriptCore/runtime/ArgList.cpp: (JSC::MarkedVectorBase::addMarkSet): (JSC::MarkedVectorBase::markLists): (JSC::MarkedVectorBase::slowEnsureCapacity): (JSC::MarkedVectorBase::expandCapacity): (JSC::MarkedVectorBase::slowAppend): (JSC::MarkedArgumentBufferBase::addMarkSet): Deleted. (JSC::MarkedArgumentBufferBase::markLists): Deleted. (JSC::MarkedArgumentBufferBase::slowEnsureCapacity): Deleted. (JSC::MarkedArgumentBufferBase::expandCapacity): Deleted. (JSC::MarkedArgumentBufferBase::slowAppend): Deleted. * Source/JavaScriptCore/runtime/ArgList.h: (JSC::MarkedVectorWithSize::MarkedVectorWithSize): (JSC::MarkedVectorWithSize::at const): (JSC::MarkedVectorWithSize::clear): (JSC::MarkedVectorWithSize::append): (JSC::MarkedVectorWithSize::appendWithCrashOnOverflow): (JSC::MarkedVectorWithSize::last const): (JSC::MarkedVectorWithSize::takeLast): (JSC::MarkedVectorWithSize::ensureCapacity): (JSC::MarkedVectorWithSize::hasOverflowed): (JSC::MarkedVectorWithSize::fill): (JSC::MarkedArgumentBufferWithSize::MarkedArgumentBufferWithSize): Deleted. * Source/WebCore/Modules/webaudio/AudioWorkletProcessor.cpp: (WebCore::AudioWorkletProcessor::buildJSArguments): * Source/WebCore/Modules/webaudio/AudioWorkletProcessor.h: * Source/WebCore/bindings/js/SerializedScriptValue.cpp: (WebCore::CloneDeserializer::deserialize): Originally-landed-as: 259548.530@safari-7615-branch (2c49ff7). rdar://108145916 Canonical link: https://commits.webkit.org/263041@main
1 parent 2ce73ef commit c9880de

File tree

8 files changed

+158
-130
lines changed

8 files changed

+158
-130
lines changed

Source/JavaScriptCore/heap/Heap.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2003-2022 Apple Inc. All rights reserved.
2+
* Copyright (C) 2003-2023 Apple Inc. All rights reserved.
33
* Copyright (C) 2007 Eric Seidel <eric@webkit.org>
44
*
55
* This library is free software; you can redistribute it and/or
@@ -2864,7 +2864,7 @@ void Heap::addCoreConstraints()
28642864

28652865
if (!m_markListSet.isEmpty()) {
28662866
SetRootMarkReasonScope rootScope(visitor, RootMarkReason::ConservativeScan);
2867-
MarkedArgumentBufferBase::markLists(visitor, m_markListSet);
2867+
MarkedVectorBase::markLists(visitor, m_markListSet);
28682868
}
28692869

28702870
{

Source/JavaScriptCore/heap/Heap.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
* Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
33
* Copyright (C) 2001 Peter Kelly (pmk@post.com)
4-
* Copyright (C) 2003-2022 Apple Inc. All rights reserved.
4+
* Copyright (C) 2003-2023 Apple Inc. All rights reserved.
55
*
66
* This library is free software; you can redistribute it and/or
77
* modify it under the terms of the GNU Lesser General Public
@@ -85,7 +85,7 @@ class MarkStackArray;
8585
class MarkStackMergingConstraint;
8686
class MarkedJSValueRefArray;
8787
class BlockDirectory;
88-
class MarkedArgumentBufferBase;
88+
class MarkedVectorBase;
8989
class MarkingConstraint;
9090
class MarkingConstraintSet;
9191
class MutatorScheduler;
@@ -413,7 +413,7 @@ class Heap {
413413
JS_EXPORT_PRIVATE std::unique_ptr<TypeCountSet> protectedObjectTypeCounts();
414414
JS_EXPORT_PRIVATE std::unique_ptr<TypeCountSet> objectTypeCounts();
415415

416-
HashSet<MarkedArgumentBufferBase*>& markListSet();
416+
HashSet<MarkedVectorBase*>& markListSet();
417417
void addMarkedJSValueRefArray(MarkedJSValueRefArray*);
418418

419419
template<typename Functor> void forEachProtectedCell(const Functor&);
@@ -782,7 +782,7 @@ class Heap {
782782
size_t m_deprecatedExtraMemorySize { 0 };
783783

784784
ProtectCountSet m_protectedValues;
785-
HashSet<MarkedArgumentBufferBase*> m_markListSet;
785+
HashSet<MarkedVectorBase*> m_markListSet;
786786
SentinelLinkedList<MarkedJSValueRefArray, BasicRawSentinelNode<MarkedJSValueRefArray>> m_markedJSValueRefArrays;
787787

788788
std::unique_ptr<MachineThreads> m_machineThreads;

Source/JavaScriptCore/heap/HeapInlines.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ inline void Heap::decrementDeferralDepthAndGCIfNeeded()
205205
}
206206
}
207207

208-
inline HashSet<MarkedArgumentBufferBase*>& Heap::markListSet()
208+
inline HashSet<MarkedVectorBase*>& Heap::markListSet()
209209
{
210210
return m_markListSet;
211211
}

Source/JavaScriptCore/runtime/ArgList.cpp

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2003-2021 Apple Inc. All rights reserved.
2+
* Copyright (C) 2003-2023 Apple Inc. All rights reserved.
33
*
44
* This library is free software; you can redistribute it and/or
55
* modify it under the terms of the GNU Library General Public
@@ -27,7 +27,7 @@ using std::min;
2727

2828
namespace JSC {
2929

30-
void MarkedArgumentBufferBase::addMarkSet(JSValue v)
30+
void MarkedVectorBase::addMarkSet(JSValue v)
3131
{
3232
if (m_markSet)
3333
return;
@@ -52,47 +52,47 @@ void ArgList::getSlice(int startIndex, ArgList& result) const
5252
}
5353

5454
template<typename Visitor>
55-
void MarkedArgumentBufferBase::markLists(Visitor& visitor, ListSet& markSet)
55+
void MarkedVectorBase::markLists(Visitor& visitor, ListSet& markSet)
5656
{
5757
ListSet::iterator end = markSet.end();
5858
for (ListSet::iterator it = markSet.begin(); it != end; ++it) {
59-
MarkedArgumentBufferBase* list = *it;
59+
MarkedVectorBase* list = *it;
6060
for (int i = 0; i < list->m_size; ++i)
6161
visitor.appendUnbarriered(JSValue::decode(list->slotFor(i)));
6262
}
6363
}
6464

65-
template void MarkedArgumentBufferBase::markLists(AbstractSlotVisitor&, ListSet&);
66-
template void MarkedArgumentBufferBase::markLists(SlotVisitor&, ListSet&);
65+
template void MarkedVectorBase::markLists(AbstractSlotVisitor&, ListSet&);
66+
template void MarkedVectorBase::markLists(SlotVisitor&, ListSet&);
6767

68-
void MarkedArgumentBufferBase::slowEnsureCapacity(size_t requestedCapacity)
68+
auto MarkedVectorBase::slowEnsureCapacity(size_t requestedCapacity) -> Status
6969
{
7070
setNeedsOverflowCheck();
7171
auto checkedNewCapacity = CheckedInt32(requestedCapacity);
7272
if (UNLIKELY(checkedNewCapacity.hasOverflowed()))
73-
return this->overflowed();
74-
expandCapacity(checkedNewCapacity);
73+
return Status::Overflowed;
74+
return expandCapacity(checkedNewCapacity);
7575
}
7676

77-
void MarkedArgumentBufferBase::expandCapacity()
77+
auto MarkedVectorBase::expandCapacity() -> Status
7878
{
7979
setNeedsOverflowCheck();
8080
auto checkedNewCapacity = CheckedInt32(m_capacity) * 2;
8181
if (UNLIKELY(checkedNewCapacity.hasOverflowed()))
82-
return this->overflowed();
83-
expandCapacity(checkedNewCapacity);
82+
return Status::Overflowed;
83+
return expandCapacity(checkedNewCapacity);
8484
}
8585

86-
void MarkedArgumentBufferBase::expandCapacity(int newCapacity)
86+
auto MarkedVectorBase::expandCapacity(int newCapacity) -> Status
8787
{
8888
setNeedsOverflowCheck();
8989
ASSERT(m_capacity < newCapacity);
9090
auto checkedSize = CheckedSize(newCapacity) * sizeof(EncodedJSValue);
9191
if (UNLIKELY(checkedSize.hasOverflowed()))
92-
return this->overflowed();
92+
return Status::Overflowed;
9393
EncodedJSValue* newBuffer = static_cast<EncodedJSValue*>(Gigacage::tryMalloc(Gigacage::JSValue, checkedSize));
9494
if (!newBuffer)
95-
return this->overflowed();
95+
return Status::Overflowed;
9696
for (int i = 0; i < m_size; ++i) {
9797
newBuffer[i] = m_buffer[i];
9898
addMarkSet(JSValue::decode(m_buffer[i]));
@@ -103,21 +103,23 @@ void MarkedArgumentBufferBase::expandCapacity(int newCapacity)
103103

104104
m_buffer = newBuffer;
105105
m_capacity = newCapacity;
106+
return Status::Success;
106107
}
107108

108-
void MarkedArgumentBufferBase::slowAppend(JSValue v)
109+
auto MarkedVectorBase::slowAppend(JSValue v) -> Status
109110
{
110111
ASSERT(m_size <= m_capacity);
111-
if (m_size == m_capacity)
112-
expandCapacity();
113-
if (UNLIKELY(Base::hasOverflowed())) {
114-
ASSERT(m_needsOverflowCheck);
115-
return;
112+
if (m_size == m_capacity) {
113+
auto status = expandCapacity();
114+
if (status == Status::Overflowed) {
115+
ASSERT(m_needsOverflowCheck);
116+
return status;
117+
}
116118
}
117-
118119
slotFor(m_size) = JSValue::encode(v);
119120
++m_size;
120121
addMarkSet(v);
122+
return Status::Success;
121123
}
122124

123125
} // namespace JSC

0 commit comments

Comments
 (0)