From c9f813604a077c3c7730be6147b5a9169ba81e84 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 10 Dec 2021 23:59:05 -0500 Subject: [PATCH] ConcurrentReadableArray: Use std::uninitialized_copy_n() instead of std::copy() to avoid calling destructors on uninitialized memory --- include/swift/Runtime/Concurrent.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/include/swift/Runtime/Concurrent.h b/include/swift/Runtime/Concurrent.h index b293a7229e08a..4aa74a534cdd2 100644 --- a/include/swift/Runtime/Concurrent.h +++ b/include/swift/Runtime/Concurrent.h @@ -548,7 +548,7 @@ template struct ConcurrentReadableArray { auto newCapacity = std::max((size_t)16, count * 2); auto *newStorage = Storage::allocate(newCapacity); if (storage) { - std::copy(storage->data(), storage->data() + count, newStorage->data()); + std::uninitialized_copy_n(storage->data(), count, newStorage->data()); newStorage->Count.store(count, std::memory_order_release); ConcurrentFreeListNode::add(&FreeList, storage); } @@ -622,10 +622,7 @@ using llvm::hash_value; /// outstanding readers, but this won't destroy the static mutex it uses. template struct ConcurrentReadableHashMap { - // We use memcpy and don't call destructors. Make sure the elements will put - // up with this. - static_assert(std::is_trivially_copyable::value, - "Elements must be trivially copyable."); + // We don't call destructors. Make sure the elements will put up with this. static_assert(std::is_trivially_destructible::value, "Elements must not have destructors (they won't be called)."); @@ -884,8 +881,13 @@ struct ConcurrentReadableHashMap { auto *newElements = ElementStorage::allocate(newCapacity); if (elements) { - memcpy(newElements->data(), elements->data(), - elementCount * sizeof(ElemTy)); + if constexpr (std::is_trivially_copyable::value) { + memcpy(newElements->data(), elements->data(), + elementCount * sizeof(ElemTy)); + } else { + std::uninitialized_copy_n(elements->data(), elementCount, + newElements->data()); + } ConcurrentFreeListNode::add(&FreeList, elements); }