Skip to content

Commit efcdcc2

Browse files
ConstellationConstellation
committed
Unreviewed, reverting 303015@main as it causes assertion failures in JSC stress tests
rdar://164678753 https://bugs.webkit.org/show_bug.cgi?id=302503 Causes 8630 JSC test failures on JSCOnly ports (using RunLoopGeneric). Test: Tools/TestWebKitAPI/Tests/WTF/RedBlackTree.cpp * Source/JavaScriptCore/jit/ExecutableAllocator.cpp: * Source/WTF/SaferCPPExpectations/NoUncountedMemberCheckerExpectations: * Source/WTF/wtf/MetaAllocator.cpp: (WTF::MetaAllocator::~MetaAllocator): (WTF::MetaAllocator::findAndRemoveFreeSpace): (WTF::MetaAllocator::debugFreeSpaceSize): (WTF::MetaAllocator::addFreeSpace): * Source/WTF/wtf/MetaAllocator.h: * Source/WTF/wtf/MetaAllocatorHandle.h: (): Deleted. * Source/WTF/wtf/RedBlackTree.h: * Source/WTF/wtf/generic/RunLoopGeneric.cpp: * Tools/TestWebKitAPI/Tests/WTF/RedBlackTree.cpp: (TestWebKitAPI::TEST_F(RedBlackTreeTest, Iterate)): (): Deleted. Canonical link: https://commits.webkit.org/303026@main
1 parent 7e06217 commit efcdcc2

8 files changed

Lines changed: 88 additions & 101 deletions

File tree

‎Source/JavaScriptCore/jit/ExecutableAllocator.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ class FixedVMPoolExecutableAllocator final {
779779
visitRight = true;
780780
});
781781

782-
SUPPRESS_UNCHECKED_LOCAL for (Islands* islands : toRemove)
782+
for (Islands* islands : toRemove)
783783
freeIslands(locker, islands);
784784

785785
if (ASSERT_ENABLED) {
@@ -834,16 +834,16 @@ class FixedVMPoolExecutableAllocator final {
834834

835835
void* islandForJumpLocation(const Locker<Lock>& locker, uintptr_t jumpLocation, uintptr_t target, bool concurrently, bool useMemcpy)
836836
{
837-
CheckedPtr islands = m_islandsForJumpSourceLocation.findExact(std::bit_cast<void*>(jumpLocation));
837+
Islands* islands = m_islandsForJumpSourceLocation.findExact(std::bit_cast<void*>(jumpLocation));
838838
if (islands) {
839839
// FIXME: We could create some method of reusing already allocated islands here, but it's
840840
// unlikely to matter in practice.
841841
if (!concurrently)
842-
freeJumpIslands(locker, islands.get());
842+
freeJumpIslands(locker, islands);
843843
} else {
844844
islands = new Islands;
845845
islands->jumpSourceLocation = CodeLocationLabel<ExecutableMemoryPtrTag>(tagCodePtr<ExecutableMemoryPtrTag>(std::bit_cast<void*>(jumpLocation)));
846-
m_islandsForJumpSourceLocation.insert(islands.get());
846+
m_islandsForJumpSourceLocation.insert(islands);
847847
}
848848

849849
RegionAllocator* allocator = findRegion(jumpLocation > target ? jumpLocation - m_regionSize : jumpLocation);
@@ -1099,9 +1099,8 @@ class FixedVMPoolExecutableAllocator final {
10991099
}
11001100

11011101
#if ENABLE(JUMP_ISLANDS)
1102-
class Islands final : public RedBlackTree<Islands, void*>::Node {
1102+
class Islands : public RedBlackTree<Islands, void*>::Node {
11031103
WTF_MAKE_TZONE_ALLOCATED(Islands);
1104-
WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR(Islands);
11051104
public:
11061105
void* key() { return jumpSourceLocation.dataLocation(); }
11071106
CodeLocationLabel<ExecutableMemoryPtrTag> jumpSourceLocation;

‎Source/WTF/SaferCPPExpectations/NoUncountedMemberCheckerExpectations‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
wtf/HashTable.h
22
wtf/JSONValues.h
33
wtf/RecursiveLockAdapter.h
4+
wtf/RedBlackTree.h

‎Source/WTF/wtf/MetaAllocator.cpp‎

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
#include <wtf/MetaAllocator.h>
3131

3232
#include <wtf/NeverDestroyed.h>
33-
#include <wtf/TZoneMallocInlines.h>
3433
#include <wtf/WTFConfig.h>
3534

3635
WTF_ALLOW_UNSAFE_BUFFER_USAGE_BEGIN
@@ -42,16 +41,13 @@ DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(MetaAllocatorHandle);
4241
DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER_AND_EXPORT(MetaAllocatorFreeSpace, WTF_INTERNAL);
4342
DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(MetaAllocatorFreeSpace);
4443

45-
WTF_MAKE_TZONE_ALLOCATED_IMPL(MetaAllocator::FreeSpaceNode);
46-
4744
MetaAllocator::~MetaAllocator()
4845
{
49-
for (CheckedPtr node = m_freeSpaceSizeMap.first(); node;) {
50-
CheckedPtr next = node->successor();
51-
m_freeSpaceSizeMap.remove(node.get());
52-
SUPPRESS_UNCHECKED_LOCAL auto* nodePtr = std::exchange(node, nullptr).unsafeGet(); // NOLINT
53-
freeFreeSpaceNode(nodePtr);
54-
node = WTFMove(next);
46+
for (FreeSpaceNode* node = m_freeSpaceSizeMap.first(); node;) {
47+
FreeSpaceNode* next = node->successor();
48+
m_freeSpaceSizeMap.remove(node);
49+
freeFreeSpaceNode(node);
50+
node = next;
5551
}
5652
#ifndef NDEBUG
5753
ASSERT(!m_mallocBalance);
@@ -215,15 +211,15 @@ MetaAllocator::Statistics MetaAllocator::currentStatistics(const Locker<Lock>&)
215211

216212
MetaAllocator::FreeSpacePtr MetaAllocator::findAndRemoveFreeSpace(size_t sizeInBytes)
217213
{
218-
CheckedPtr node = m_freeSpaceSizeMap.findLeastGreaterThanOrEqual(sizeInBytes);
214+
FreeSpaceNode* node = m_freeSpaceSizeMap.findLeastGreaterThanOrEqual(sizeInBytes);
219215

220216
if (!node)
221217
return nullptr;
222218

223219
size_t nodeSizeInBytes = node->sizeInBytes();
224220
RELEASE_ASSERT(nodeSizeInBytes >= sizeInBytes);
225221

226-
m_freeSpaceSizeMap.remove(node.get());
222+
m_freeSpaceSizeMap.remove(node);
227223

228224
FreeSpacePtr result;
229225

@@ -233,8 +229,7 @@ MetaAllocator::FreeSpacePtr MetaAllocator::findAndRemoveFreeSpace(size_t sizeInB
233229

234230
m_freeSpaceStartAddressMap.remove(node->m_start);
235231
m_freeSpaceEndAddressMap.remove(node->m_end);
236-
SUPPRESS_UNCHECKED_LOCAL auto* nodePtr = std::exchange(node, nullptr).unsafeGet(); // NOLINT
237-
freeFreeSpaceNode(nodePtr);
232+
freeFreeSpaceNode(node);
238233
} else {
239234
// Try to be a good citizen and ensure that the returned chunk of memory
240235
// straddles as few pages as possible, but only insofar as doing so will
@@ -259,8 +254,8 @@ MetaAllocator::FreeSpacePtr MetaAllocator::findAndRemoveFreeSpace(size_t sizeInB
259254
node->m_start += sizeInBytes;
260255
RELEASE_ASSERT(nodeStartAsInt < node->m_start.untaggedPtr<uintptr_t>() && node->m_start.untaggedPtr<uintptr_t>() < node->m_end.untaggedPtr<uintptr_t>());
261256

262-
m_freeSpaceSizeMap.insert(node.get());
263-
m_freeSpaceStartAddressMap.add(node->m_start, node.get());
257+
m_freeSpaceSizeMap.insert(node);
258+
m_freeSpaceStartAddressMap.add(node->m_start, node);
264259
} else {
265260
// Allocate in the right size of the returned chunk, and slide the node to the left;
266261

@@ -270,8 +265,8 @@ MetaAllocator::FreeSpacePtr MetaAllocator::findAndRemoveFreeSpace(size_t sizeInB
270265

271266
node->m_end = result;
272267

273-
m_freeSpaceSizeMap.insert(node.get());
274-
m_freeSpaceEndAddressMap.add(result, node.get());
268+
m_freeSpaceSizeMap.insert(node);
269+
m_freeSpaceEndAddressMap.add(result, node);
275270
}
276271
}
277272

@@ -304,7 +299,7 @@ size_t MetaAllocator::debugFreeSpaceSize()
304299
#ifndef NDEBUG
305300
Locker locker { m_lock };
306301
size_t result = 0;
307-
for (CheckedPtr node = m_freeSpaceSizeMap.first(); node; node = node->successor())
302+
for (FreeSpaceNode* node = m_freeSpaceSizeMap.first(); node; node = node->successor())
308303
result += node->sizeInBytes();
309304
return result;
310305
#else
@@ -317,22 +312,22 @@ void MetaAllocator::addFreeSpace(FreeSpacePtr start, size_t sizeInBytes)
317312
{
318313
FreeSpacePtr end = start + sizeInBytes;
319314

320-
auto leftNeighbor = m_freeSpaceEndAddressMap.find(start);
321-
auto rightNeighbor = m_freeSpaceStartAddressMap.find(end);
315+
UncheckedKeyHashMap<FreeSpacePtr, FreeSpaceNode*>::iterator leftNeighbor = m_freeSpaceEndAddressMap.find(start);
316+
UncheckedKeyHashMap<FreeSpacePtr, FreeSpaceNode*>::iterator rightNeighbor = m_freeSpaceStartAddressMap.find(end);
322317

323318
if (leftNeighbor != m_freeSpaceEndAddressMap.end()) {
324319
// We have something we can coalesce with on the left. Remove it from the tree, and
325320
// remove its end from the end address map.
326321

327322
ASSERT(leftNeighbor->value->m_end == leftNeighbor->key);
328323

329-
CheckedPtr leftNode = leftNeighbor->value;
324+
FreeSpaceNode* leftNode = leftNeighbor->value;
330325

331326
FreeSpacePtr leftEnd = leftNode->m_end;
332327

333328
ASSERT(leftEnd == start);
334329

335-
m_freeSpaceSizeMap.remove(leftNode.get());
330+
m_freeSpaceSizeMap.remove(leftNode);
336331
m_freeSpaceEndAddressMap.remove(leftEnd);
337332

338333
// Now check if there is also something to coalesce with on the right.
@@ -342,58 +337,58 @@ void MetaAllocator::addFreeSpace(FreeSpacePtr start, size_t sizeInBytes)
342337

343338
ASSERT(rightNeighbor->value->m_start == rightNeighbor->key);
344339

345-
CheckedPtr rightNode = rightNeighbor->value;
340+
FreeSpaceNode* rightNode = rightNeighbor->value;
346341
FreeSpacePtr rightStart = rightNeighbor->key;
347342
size_t rightSize = rightNode->sizeInBytes();
348343
FreeSpacePtr rightEnd = rightNode->m_end;
349344

350345
ASSERT(rightStart == end);
351346
ASSERT(leftNode->m_start + (leftNode->sizeInBytes() + sizeInBytes + rightSize) == rightEnd);
352347

353-
m_freeSpaceSizeMap.remove(rightNode.get());
348+
m_freeSpaceSizeMap.remove(rightNode);
354349
m_freeSpaceStartAddressMap.remove(rightStart);
355350
m_freeSpaceEndAddressMap.remove(rightEnd);
356351

357-
freeFreeSpaceNode(rightNode.get());
352+
freeFreeSpaceNode(rightNode);
358353

359354
leftNode->m_end += (sizeInBytes + rightSize);
360355

361-
m_freeSpaceSizeMap.insert(leftNode.get());
362-
m_freeSpaceEndAddressMap.add(rightEnd, leftNode.get());
356+
m_freeSpaceSizeMap.insert(leftNode);
357+
m_freeSpaceEndAddressMap.add(rightEnd, leftNode);
363358
} else {
364359
leftNode->m_end += sizeInBytes;
365360

366-
m_freeSpaceSizeMap.insert(leftNode.get());
367-
m_freeSpaceEndAddressMap.add(end, leftNode.get());
361+
m_freeSpaceSizeMap.insert(leftNode);
362+
m_freeSpaceEndAddressMap.add(end, leftNode);
368363
}
369364
} else {
370365
// Cannot coalesce with left; try to see if we can coalesce with right.
371366

372367
if (rightNeighbor != m_freeSpaceStartAddressMap.end()) {
373-
CheckedPtr rightNode = rightNeighbor->value;
368+
FreeSpaceNode* rightNode = rightNeighbor->value;
374369
FreeSpacePtr rightStart = rightNeighbor->key;
375370

376371
ASSERT(rightStart == end);
377372
ASSERT(start + (sizeInBytes + rightNode->sizeInBytes()) == rightNode->m_end);
378373

379-
m_freeSpaceSizeMap.remove(rightNode.get());
374+
m_freeSpaceSizeMap.remove(rightNode);
380375
m_freeSpaceStartAddressMap.remove(rightStart);
381376

382377
rightNode->m_start = start;
383378

384-
m_freeSpaceSizeMap.insert(rightNode.get());
385-
m_freeSpaceStartAddressMap.add(start, rightNode.get());
379+
m_freeSpaceSizeMap.insert(rightNode);
380+
m_freeSpaceStartAddressMap.add(start, rightNode);
386381
} else {
387382
// Nothing to coalesce with, so create a new free space node and add it.
388383

389-
CheckedPtr node = allocFreeSpaceNode();
384+
FreeSpaceNode* node = allocFreeSpaceNode();
390385

391386
node->m_start = start;
392387
node->m_end = start + sizeInBytes;
393388

394-
m_freeSpaceSizeMap.insert(node.get());
395-
m_freeSpaceStartAddressMap.add(start, node.get());
396-
m_freeSpaceEndAddressMap.add(end, node.get());
389+
m_freeSpaceSizeMap.insert(node);
390+
m_freeSpaceStartAddressMap.add(start, node);
391+
m_freeSpaceEndAddressMap.add(end, node);
397392
}
398393
}
399394

‎Source/WTF/wtf/MetaAllocator.h‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
#include <wtf/PageBlock.h>
3737
#include <wtf/RedBlackTree.h>
3838
#include <wtf/RefPtr.h>
39-
#include <wtf/TZoneMalloc.h>
4039

4140
namespace WTF {
4241

@@ -140,9 +139,7 @@ class MetaAllocator {
140139

141140
friend class MetaAllocatorHandle;
142141

143-
class FreeSpaceNode final : public RedBlackTree<FreeSpaceNode, size_t>::Node {
144-
WTF_MAKE_TZONE_ALLOCATED(FreeSpaceNode);
145-
WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR(FreeSpaceNode);
142+
class FreeSpaceNode : public RedBlackTree<FreeSpaceNode, size_t>::Node {
146143
public:
147144
size_t sizeInBytes()
148145
{
@@ -190,8 +187,8 @@ class MetaAllocator {
190187
unsigned m_logPageSize;
191188

192189
Tree m_freeSpaceSizeMap;
193-
UncheckedKeyHashMap<FreeSpacePtr, CheckedPtr<FreeSpaceNode>> m_freeSpaceStartAddressMap;
194-
UncheckedKeyHashMap<FreeSpacePtr, CheckedPtr<FreeSpaceNode>> m_freeSpaceEndAddressMap;
190+
UncheckedKeyHashMap<FreeSpacePtr, FreeSpaceNode*> m_freeSpaceStartAddressMap;
191+
UncheckedKeyHashMap<FreeSpacePtr, FreeSpaceNode*> m_freeSpaceEndAddressMap;
195192
UncheckedKeyHashMap<uintptr_t, size_t> m_pageOccupancyMap;
196193

197194
size_t m_bytesAllocated;

‎Source/WTF/wtf/MetaAllocatorHandle.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ class MetaAllocator;
3939
class PrintStream;
4040

4141
DECLARE_COMPACT_ALLOCATOR_WITH_HEAP_IDENTIFIER(MetaAllocatorHandle);
42-
class MetaAllocatorHandle final : public ThreadSafeRefCounted<MetaAllocatorHandle>, public RedBlackTree<MetaAllocatorHandle, void*>::Node {
42+
class MetaAllocatorHandle : public ThreadSafeRefCounted<MetaAllocatorHandle>, public RedBlackTree<MetaAllocatorHandle, void*>::Node {
4343
WTF_DEPRECATED_MAKE_FAST_COMPACT_ALLOCATED_WITH_HEAP_IDENTIFIER(MetaAllocatorHandle, MetaAllocatorHandle);
44-
WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR(MetaAllocatorHandle);
44+
4545
public:
4646
using MemoryPtr = CodePtr<HandleMemoryPtrTag>;
4747

0 commit comments

Comments
 (0)