Skip to content

Commit cd6755b

Browse files
authored
Merge pull request #40521 from apple/jgrynspan/ConcurrentReadableArray-copy-bug
`ConcurrentReadableArray`: Use `std::uninitialized_copy_n()` instead of `std::copy()` to avoid calling destructors on uninitialized memory
2 parents 26640ac + c9f8136 commit cd6755b

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

include/swift/Runtime/Concurrent.h

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ template <class ElemTy> struct ConcurrentReadableArray {
548548
auto newCapacity = std::max((size_t)16, count * 2);
549549
auto *newStorage = Storage::allocate(newCapacity);
550550
if (storage) {
551-
std::copy(storage->data(), storage->data() + count, newStorage->data());
551+
std::uninitialized_copy_n(storage->data(), count, newStorage->data());
552552
newStorage->Count.store(count, std::memory_order_release);
553553
ConcurrentFreeListNode::add(&FreeList, storage);
554554
}
@@ -622,10 +622,7 @@ using llvm::hash_value;
622622
/// outstanding readers, but this won't destroy the static mutex it uses.
623623
template <class ElemTy, class MutexTy = StaticMutex>
624624
struct ConcurrentReadableHashMap {
625-
// We use memcpy and don't call destructors. Make sure the elements will put
626-
// up with this.
627-
static_assert(std::is_trivially_copyable<ElemTy>::value,
628-
"Elements must be trivially copyable.");
625+
// We don't call destructors. Make sure the elements will put up with this.
629626
static_assert(std::is_trivially_destructible<ElemTy>::value,
630627
"Elements must not have destructors (they won't be called).");
631628

@@ -884,8 +881,13 @@ struct ConcurrentReadableHashMap {
884881
auto *newElements = ElementStorage::allocate(newCapacity);
885882

886883
if (elements) {
887-
memcpy(newElements->data(), elements->data(),
888-
elementCount * sizeof(ElemTy));
884+
if constexpr (std::is_trivially_copyable<ElemTy>::value) {
885+
memcpy(newElements->data(), elements->data(),
886+
elementCount * sizeof(ElemTy));
887+
} else {
888+
std::uninitialized_copy_n(elements->data(), elementCount,
889+
newElements->data());
890+
}
889891
ConcurrentFreeListNode::add(&FreeList, elements);
890892
}
891893

0 commit comments

Comments
 (0)