[dawn] Replace some raw allocations with HeapArray - Replace AllocNoThrow - Replace std::unique_ptr<T[]>(new T[]) / std::make_unique<T[]> - Replace malloc (in some places with nothrow, others without) - Still need to: Replace other `new T[]` Bug: 512465980 Change-Id: Iedc83d61f3b8ebe22c67808a8622d734ec770a11 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/302496 Commit-Queue: Kai Ninomiya <kainino@chromium.org> Reviewed-by: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/native/BindGroupTracker.h b/src/dawn/native/BindGroupTracker.h index 3842795..79b0dd2 100644 --- a/src/dawn/native/BindGroupTracker.h +++ b/src/dawn/native/BindGroupTracker.h
@@ -70,8 +70,9 @@ mBindGroups[index] = bindGroup; mDynamicOffsets[index].count = BindingIndex(dynamicOffsetCount); - std::copy(dynamicOffsets, DAWN_UNSAFE_TODO(dynamicOffsets + dynamicOffsetCount), - mDynamicOffsets[index].offsets.begin()); + auto dynamicOffsetsSpan = + DAWN_UNSAFE_TODO(Span<uint32_t>(dynamicOffsets, dynamicOffsetCount)); + std::ranges::copy(dynamicOffsetsSpan, mDynamicOffsets[index].offsets.begin()); } void OnSetPipeline(PipelineBase* pipeline) { mPipeline = pipeline; }
diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp index 9ba2162..d8071f2 100644 --- a/src/dawn/native/Buffer.cpp +++ b/src/dawn/native/Buffer.cpp
@@ -40,7 +40,6 @@ #include "dawn/native/ValidationUtils_autogen.h" #include "dawn/platform/DawnPlatform.h" #include "partition_alloc/pointers/raw_ptr.h" -#include "src/dawn/common/Alloc.h" #include "src/dawn/common/Constants.h" #include "src/dawn/common/StringViewUtils.h" #include "src/dawn/native/Adapter.h" @@ -60,6 +59,7 @@ #include "src/dawn/platform/tracing/TraceEvent.h" #include "src/utils/assert.h" #include "src/utils/compiler.h" +#include "src/utils/heap_array.h" #include "src/utils/log.h" namespace dawn::native { @@ -81,19 +81,16 @@ bool IsCPUWritableAtCreation() const override { return true; } MaybeError MapAtCreationImpl() override { - DAWN_CHECK(mFakeMappedData == nullptr); + DAWN_CHECK(!mFakeMappedData); - // Check that the size can be used to allocate mFakeMappedData. A malloc(0) - // is invalid, and on 32bit systems we should avoid a narrowing conversion that - // would make size = 1 << 32 + 1 allocate one byte. uint64_t size = GetSize(); - bool isValidSize = size != 0 && size < uint64_t(std::numeric_limits<size_t>::max()); - - if (isValidSize) { - mFakeMappedData = std::unique_ptr<uint8_t[]>(AllocNoThrow<uint8_t>(size)); + if (size < uint64_t(std::numeric_limits<size_t>::max())) { + mFakeMappedData = + // SAFETY: Frontend is responsible for initializing MapAtCreation memory. + DAWN_UNSAFE_BUFFERS(HeapArray<uint8_t>::Uninit(size, std::nothrow)); } - if (mFakeMappedData == nullptr) { + if (!mFakeMappedData) { return DAWN_OUT_OF_MEMORY_ERROR( "Failed to allocate memory to map ErrorBuffer at creation."); } @@ -107,11 +104,11 @@ DAWN_UNREACHABLE(); } - void* GetMappedPointerImpl() override { return mFakeMappedData.get(); } + void* GetMappedPointerImpl() override { return mFakeMappedData.data(); } - void UnmapImpl(BufferState oldState, BufferState newState) override { mFakeMappedData.reset(); } + void UnmapImpl(BufferState oldState, BufferState newState) override { mFakeMappedData = {}; } - std::unique_ptr<uint8_t[]> mFakeMappedData = nullptr; + HeapArray<uint8_t> mFakeMappedData; }; // GetMappedRange on a zero-sized buffer returns a pointer to this value.
diff --git a/src/dawn/native/CommandAllocator.cpp b/src/dawn/native/CommandAllocator.cpp index 2575865..ae9220a 100644 --- a/src/dawn/native/CommandAllocator.cpp +++ b/src/dawn/native/CommandAllocator.cpp
@@ -99,7 +99,7 @@ *commandId = detail::kEndOfBlock; return false; } - mCurrentPtr = AlignPtr(mBlocks[mCurrentBlock].block.get(), alignof(uint32_t)); + mCurrentPtr = AlignPtr(mBlocks[mCurrentBlock].data(), alignof(uint32_t)); return NextCommandId(commandId); } @@ -111,7 +111,7 @@ // the iteration immediately, without special casing the initialization. mCurrentPtr = reinterpret_cast<char*>(&mEndOfBlock); } else { - mCurrentPtr = AlignPtr(mBlocks[0].block.get(), alignof(uint32_t)); + mCurrentPtr = AlignPtr(mBlocks[0].data(), alignof(uint32_t)); } } @@ -222,11 +222,12 @@ // Allocate blocks doubling sizes each time, to a maximum of 16k (or at least minimumSize). mLastAllocationSize = std::max(minimumSize, std::min(mLastAllocationSize * 2, size_t(16384))); - auto block = std::unique_ptr<char[]>(new char[mLastAllocationSize]); + // SAFETY: This is a pool allocation that will be initialized when it's suballocated. + auto block = DAWN_UNSAFE_BUFFERS(HeapArray<char>::Uninit(mLastAllocationSize)); - mCurrentPtr = AlignPtr(block.get(), alignof(uint32_t)); - mEndPtr = DAWN_UNSAFE_TODO(block.get() + mLastAllocationSize); - mBlocks.push_back({mLastAllocationSize, std::move(block)}); + mCurrentPtr = AlignPtr(block.data(), alignof(uint32_t)); + mEndPtr = std::to_address(block.end()); + mBlocks.push_back(std::move(block)); } void CommandAllocator::ResetPointers() {
diff --git a/src/dawn/native/CommandAllocator.h b/src/dawn/native/CommandAllocator.h index eb9402d..2547a4c 100644 --- a/src/dawn/native/CommandAllocator.h +++ b/src/dawn/native/CommandAllocator.h
@@ -40,6 +40,7 @@ #include "src/dawn/common/Math.h" #include "src/utils/assert.h" #include "src/utils/compiler.h" +#include "src/utils/heap_array.h" #include "src/utils/non_copyable.h" namespace dawn::native { @@ -75,10 +76,7 @@ // These are the lists of blocks, should not be used directly, only through CommandAllocator // and CommandIterator -struct BlockDef { - size_t size = 0; - std::unique_ptr<char[]> block = nullptr; -}; +using BlockDef = HeapArray<char>; using CommandBlocks = std::vector<BlockDef>; namespace detail { @@ -127,10 +125,10 @@ DAWN_FORCE_INLINE bool NextCommandId(uint32_t* commandId) { char* idPtr = AlignPtr(mCurrentPtr, alignof(uint32_t)); - DAWN_UNSAFE_TODO( - DAWN_RELEASE_ASSUME(idPtr == reinterpret_cast<char*>(&mEndOfBlock) || - idPtr + sizeof(uint32_t) <= mBlocks[mCurrentBlock].block.get() + - mBlocks[mCurrentBlock].size)); + DAWN_ASSERT(idPtr == reinterpret_cast<char*>(&mEndOfBlock) || + DAWN_UNSAFE_TODO(idPtr + sizeof(uint32_t)) <= + std::to_address(mBlocks[mCurrentBlock].end())); + uint32_t id = *reinterpret_cast<uint32_t*>(idPtr); if (id != detail::kEndOfBlock) { @@ -145,9 +143,8 @@ DAWN_FORCE_INLINE void* NextCommand(size_t commandSize, size_t commandAlignment) { char* commandPtr = AlignPtr(mCurrentPtr, commandAlignment); - DAWN_UNSAFE_TODO( - DAWN_RELEASE_ASSUME(commandPtr + sizeof(commandSize) <= - mBlocks[mCurrentBlock].block.get() + mBlocks[mCurrentBlock].size)); + DAWN_ASSERT(DAWN_UNSAFE_TODO(commandPtr + sizeof(commandSize)) <= + std::to_address(mBlocks[mCurrentBlock].end())); mCurrentPtr = DAWN_UNSAFE_TODO(commandPtr + commandSize); return commandPtr;
diff --git a/src/dawn/native/IndirectDrawValidationEncoder.cpp b/src/dawn/native/IndirectDrawValidationEncoder.cpp index c879ef5..783db5e 100644 --- a/src/dawn/native/IndirectDrawValidationEncoder.cpp +++ b/src/dawn/native/IndirectDrawValidationEncoder.cpp
@@ -470,7 +470,7 @@ IndirectDrawMetadata::DrawType drawType; uint64_t outputParamsSize = 0; uint64_t batchDataSize = 0; - std::unique_ptr<void, void (*)(void*)> batchData{nullptr, std::free}; + HeapArray<uint8_t> batchData; std::vector<Batch> batches; }; @@ -647,13 +647,11 @@ // Now we allocate and populate host-side batch data to be copied to the GPU. for (Pass& pass : passes) { - // We use std::malloc here because it guarantees maximal scalar alignment. - pass.batchData = {std::malloc(pass.batchDataSize), std::free}; - DAWN_UNSAFE_TODO(memset(pass.batchData.get(), 0, pass.batchDataSize)); - uint8_t* batchData = static_cast<uint8_t*>(pass.batchData.get()); + // batchData is maximally-aligned, so we can suballocate it. + pass.batchData = HeapArray<uint8_t>{checked_cast<size_t>(pass.batchDataSize)}; for (Batch& batch : pass.batches) { - batch.batchInfo = - new (&DAWN_UNSAFE_TODO(batchData[batch.dataBufferOffset])) BatchInfo(); + auto placement = pass.batchData.subspan(batch.dataBufferOffset, sizeof(BatchInfo)); + batch.batchInfo = new (placement.data()) BatchInfo(); batch.batchInfo->numDraws = static_cast<uint32_t>(batch.metadata->draws.size()); batch.batchInfo->flags = pass.flags; @@ -718,8 +716,7 @@ // compute pass. The compute pass encodes a separate SetBindGroup and Dispatch command // for each batch. for (const Pass& pass : passes) { - commandEncoder->APIWriteBuffer(batchDataBuffer.GetBuffer(), 0, - static_cast<const uint8_t*>(pass.batchData.get()), + commandEncoder->APIWriteBuffer(batchDataBuffer.GetBuffer(), 0, pass.batchData.data(), pass.batchDataSize); Ref<ComputePassEncoder> passEncoder = commandEncoder->BeginComputePass();
diff --git a/src/dawn/native/SubresourceStorage.h b/src/dawn/native/SubresourceStorage.h index e8d74cb..8707f6a 100644 --- a/src/dawn/native/SubresourceStorage.h +++ b/src/dawn/native/SubresourceStorage.h
@@ -39,6 +39,7 @@ #include "src/dawn/native/Error.h" #include "src/dawn/native/Subresource.h" #include "src/utils/assert.h" +#include "src/utils/heap_array.h" namespace dawn::native { @@ -239,12 +240,12 @@ std::array<T, kMaxAspects> mInlineAspectData = {}; // Indexed as mLayerCompressed[aspectIndex * mArrayLayerCount + layer]. - std::unique_ptr<bool[]> mLayerCompressed; + HeapArray<bool> mLayerCompressed; // Indexed as mData[(aspectIndex * mArrayLayerCount + layer) * mMipLevelCount + level]. // The data for a compressed aspect is stored in the slot for (aspect, 0, 0). Similarly // the data for a compressed layer of aspect if in the slot for (aspect, layer, 0). - std::unique_ptr<T[]> mData; + HeapArray<T> mData; }; template <typename T> @@ -492,12 +493,12 @@ mAspectCompressed[aspectIndex] = false; // Extra allocations are only needed when aspects are decompressed. Create them lazily. - if (mData == nullptr) { - DAWN_ASSERT(mLayerCompressed == nullptr); + if (!mData) { + DAWN_ASSERT(!mLayerCompressed); uint32_t aspectCount = GetAspectCount(mAspects); - mLayerCompressed = std::make_unique<bool[]>(aspectCount * mArrayLayerCount); - mData = std::make_unique<T[]>(aspectCount * mArrayLayerCount * mMipLevelCount); + mLayerCompressed = HeapArray<bool>{aspectCount * mArrayLayerCount}; + mData = HeapArray<T>{aspectCount * mArrayLayerCount * mMipLevelCount}; for (uint32_t layerIndex = 0; layerIndex < aspectCount * mArrayLayerCount; layerIndex++) { mLayerCompressed[layerIndex] = true;
diff --git a/src/dawn/native/null/DeviceNull.cpp b/src/dawn/native/null/DeviceNull.cpp index fe92425..bb844b7 100644 --- a/src/dawn/native/null/DeviceNull.cpp +++ b/src/dawn/native/null/DeviceNull.cpp
@@ -344,20 +344,15 @@ // BindGroupDataHolder -BindGroupDataHolder::BindGroupDataHolder(size_t size) - : mBindingDataAllocation(malloc(size)) // malloc is guaranteed to return a - // pointer aligned enough for the allocation -{} +BindGroupDataHolder::BindGroupDataHolder(size_t size) : mBindingDataAllocation{size} {} -BindGroupDataHolder::~BindGroupDataHolder() { - free(mBindingDataAllocation.ExtractAsDangling()); -} +BindGroupDataHolder::~BindGroupDataHolder() = default; // BindGroup BindGroup::BindGroup(DeviceBase* device, const UnpackedPtr<BindGroupDescriptor>& descriptor) : BindGroupDataHolder(descriptor->layout->GetInternalBindGroupLayout()->GetBindingDataSize()), - BindGroupBase(device, descriptor, mBindingDataAllocation) {} + BindGroupBase(device, descriptor, mBindingDataAllocation.data()) {} MaybeError BindGroup::InitializeImpl() { return {}; @@ -373,7 +368,8 @@ Buffer::Buffer(Device* device, const UnpackedPtr<BufferDescriptor>& descriptor) : BufferBase(device, descriptor) { - mBackingData = std::unique_ptr<uint8_t[]>(new uint8_t[GetSize()]); + // SAFETY: Frontend is responsible for initializing mapped memory. + mBackingData = DAWN_UNSAFE_BUFFERS(HeapArray<uint8_t>::Uninit(GetSize())); mAllocatedSize = GetSize(); } @@ -392,13 +388,17 @@ uint64_t destinationOffset, uint64_t size) { uint8_t* ptr = reinterpret_cast<uint8_t*>(staging->GetMappedPointer()); - DAWN_UNSAFE_TODO(memcpy(mBackingData.get() + destinationOffset, ptr + sourceOffset, size)); + auto src = DAWN_UNSAFE_TODO(Span<uint8_t>{ptr + sourceOffset, checked_cast<size_t>(size)}); + // TODO(https://crbug.com/524406299): Use Span::CopyFrom. + std::ranges::copy(src, mBackingData.begin()); } void Buffer::DoWriteBuffer(uint64_t bufferOffset, const void* data, size_t size) { DAWN_ASSERT(bufferOffset + size <= GetSize()); DAWN_ASSERT(mBackingData); - DAWN_UNSAFE_TODO(memcpy(mBackingData.get() + bufferOffset, data, size)); + auto src = DAWN_UNSAFE_TODO(Span<const uint8_t>{static_cast<const uint8_t*>(data)), size}; + // TODO(https://crbug.com/524406299): Use Span::CopyFrom. + std::ranges::copy(src, mBackingData.subspan(bufferOffset).begin()); } MaybeError Buffer::MapAsyncImpl(wgpu::MapMode mode, size_t offset, size_t size) { @@ -411,7 +411,7 @@ } void* Buffer::GetMappedPointerImpl() { - return mBackingData.get(); + return mBackingData.data(); } void Buffer::UnmapImpl(BufferState oldState, BufferState newState) {}
diff --git a/src/dawn/native/null/DeviceNull.h b/src/dawn/native/null/DeviceNull.h index 6e047cd..22280fa 100644 --- a/src/dawn/native/null/DeviceNull.h +++ b/src/dawn/native/null/DeviceNull.h
@@ -52,6 +52,7 @@ #include "src/dawn/native/Texture.h" #include "src/dawn/native/ToBackend.h" #include "src/dawn/native/dawn_platform.h" +#include "src/utils/heap_array.h" namespace dawn::native::null { @@ -235,7 +236,7 @@ explicit BindGroupDataHolder(size_t size); ~BindGroupDataHolder(); - raw_ptr<void> mBindingDataAllocation; + HeapArray<std::byte> mBindingDataAllocation; }; // We don't have the complexity of placement-allocation of bind group data in @@ -278,7 +279,7 @@ MaybeError MapAtCreationImpl() override; void* GetMappedPointerImpl() override; - std::unique_ptr<uint8_t[]> mBackingData; + HeapArray<uint8_t> mBackingData; }; class CommandBuffer final : public CommandBufferBase {
diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn index dbf1b27..0e1ba6d 100644 --- a/src/dawn/tests/BUILD.gn +++ b/src/dawn/tests/BUILD.gn
@@ -178,6 +178,7 @@ deps = [ ":test_infra_sources", "${dawn_root}/src/dawn:proc", + "${dawn_root}/src/dawn/common", "${dawn_root}/src/dawn/native:sources", "${dawn_root}/src/dawn/native:static", "${dawn_root}/src/utils:gmock_and_gtest",
diff --git a/src/dawn/tests/end2end/ComputeDispatchTests.cpp b/src/dawn/tests/end2end/ComputeDispatchTests.cpp index 0f9bdfd..9cd1822 100644 --- a/src/dawn/tests/end2end/ComputeDispatchTests.cpp +++ b/src/dawn/tests/end2end/ComputeDispatchTests.cpp
@@ -504,7 +504,7 @@ indirectBufferData[indirectStart] > maxComputeWorkgroupsPerDimension || indirectBufferData[indirectStart + 1] > maxComputeWorkgroupsPerDimension || indirectBufferData[indirectStart + 2] > maxComputeWorkgroupsPerDimension) { - std::copy(kSentinelData.begin(), kSentinelData.end(), expected.begin() + o); + std::ranges::copy(kSentinelData, expected.begin() + o); } else { expected[o] = indirectBufferData[indirectStart]; expected[o + 1] = indirectBufferData[indirectStart + 1];
diff --git a/src/dawn/tests/unittests/PlacementAllocatedTests.cpp b/src/dawn/tests/unittests/PlacementAllocatedTests.cpp index 8502e01..89a4d01 100644 --- a/src/dawn/tests/unittests/PlacementAllocatedTests.cpp +++ b/src/dawn/tests/unittests/PlacementAllocatedTests.cpp
@@ -30,6 +30,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "src/dawn/common/PlacementAllocated.h" +#include "src/utils/heap_array.h" namespace dawn { namespace { @@ -65,9 +66,9 @@ // Test that deletion calls the destructor and does not free memory. TEST_F(PlacementAllocatedTests, DeletionDoesNotFreeMemory) { - void* ptr = malloc(sizeof(Foo)); + HeapArray<std::byte> placement{sizeof(Foo)}; - Foo* foo = new (ptr) Foo(); + Foo* foo = new (placement.data()) Foo(); EXPECT_CALL(*mockDestructor, Call(foo, DestructedClass::Foo)); delete foo; @@ -75,16 +76,14 @@ // Touch the memory, this shouldn't crash. static_assert(sizeof(Foo) >= sizeof(uint32_t)); *reinterpret_cast<uint32_t*>(foo) = 42; - - free(ptr); } // Test that destructing an instance of a derived class calls the derived, then base destructor, and // does not free memory. TEST_F(PlacementAllocatedTests, DeletingDerivedClassCallsBaseDestructor) { - void* ptr = malloc(sizeof(Bar)); + HeapArray<std::byte> placement{sizeof(Bar)}; - Bar* bar = new (ptr) Bar(); + Bar* bar = new (placement.data()) Bar(); { InSequence s; @@ -96,16 +95,14 @@ // Touch the memory, this shouldn't crash. static_assert(sizeof(Bar) >= sizeof(uint32_t)); *reinterpret_cast<uint32_t*>(bar) = 42; - - free(ptr); } // Test that destructing an instance of a base class calls the derived, then base destructor, and // does not free memory. TEST_F(PlacementAllocatedTests, DeletingBaseClassCallsDerivedDestructor) { - void* ptr = malloc(sizeof(Bar)); + HeapArray<std::byte> placement{sizeof(Bar)}; - Foo* foo = new (ptr) Bar(); + Foo* foo = new (placement.data()) Bar(); { InSequence s; @@ -117,8 +114,6 @@ // Touch the memory, this shouldn't crash. static_assert(sizeof(Bar) >= sizeof(uint32_t)); *reinterpret_cast<uint32_t*>(foo) = 42; - - free(ptr); } } // anonymous namespace
diff --git a/src/dawn/tests/unittests/native/mocks/BindGroupMock.cpp b/src/dawn/tests/unittests/native/mocks/BindGroupMock.cpp index f5676f2..6240006 100644 --- a/src/dawn/tests/unittests/native/mocks/BindGroupMock.cpp +++ b/src/dawn/tests/unittests/native/mocks/BindGroupMock.cpp
@@ -31,7 +31,7 @@ BindGroupMock::BindGroupMock(DeviceMock* device, const UnpackedPtr<BindGroupDescriptor>& descriptor) : BindGroupDataHolder(descriptor->layout->GetInternalBindGroupLayout()->GetBindingDataSize()), - BindGroupBase(device, descriptor, mBindingDataAllocation) { + BindGroupBase(device, descriptor, mBindingDataAllocation.data()) { ON_CALL(*this, InitializeImpl).WillByDefault([]() -> MaybeError { return {}; }); ON_CALL(*this, DestroyImpl).WillByDefault([this](DestroyReason reason) { this->BindGroupBase::DestroyImpl(reason);
diff --git a/src/dawn/tests/unittests/native/mocks/BufferMock.cpp b/src/dawn/tests/unittests/native/mocks/BufferMock.cpp index 40b7ead..cfa963e 100644 --- a/src/dawn/tests/unittests/native/mocks/BufferMock.cpp +++ b/src/dawn/tests/unittests/native/mocks/BufferMock.cpp
@@ -30,6 +30,7 @@ #include <memory> #include "src/dawn/native/ChainUtils.h" +#include "src/utils/heap_array.h" namespace dawn::native { @@ -41,12 +42,13 @@ : BufferBase(device, descriptor) { mAllocatedSize = allocatedSizeOverride.value_or(GetSize()); DAWN_ASSERT(mAllocatedSize >= GetSize()); - mBackingData = std::unique_ptr<uint8_t[]>(new uint8_t[mAllocatedSize.value()]); + // SAFETY: Test-only code. + mBackingData = DAWN_UNSAFE_BUFFERS(HeapArray<uint8_t>::Uninit(mAllocatedSize.value())); ON_CALL(*this, DestroyImpl).WillByDefault([this](DestroyReason reason) { this->BufferBase::DestroyImpl(reason); }); - ON_CALL(*this, GetMappedPointerImpl).WillByDefault(Return(mBackingData.get())); + ON_CALL(*this, GetMappedPointerImpl).WillByDefault(Return(mBackingData.data())); ON_CALL(*this, IsCPUWritableAtCreation).WillByDefault([this] { return (GetInternalUsage() & (wgpu::BufferUsage::MapRead | wgpu::BufferUsage::MapWrite)) != 0;
diff --git a/src/dawn/tests/unittests/native/mocks/BufferMock.h b/src/dawn/tests/unittests/native/mocks/BufferMock.h index 4cabb03..17c705d 100644 --- a/src/dawn/tests/unittests/native/mocks/BufferMock.h +++ b/src/dawn/tests/unittests/native/mocks/BufferMock.h
@@ -33,6 +33,7 @@ #include "gmock/gmock.h" #include "src/dawn/native/Buffer.h" #include "src/dawn/tests/unittests/native/mocks/DeviceMock.h" +#include "src/utils/heap_array.h" namespace dawn::native { @@ -60,7 +61,7 @@ MOCK_METHOD(bool, IsCPUWritableAtCreation, (), (const, override)); private: - std::unique_ptr<uint8_t[]> mBackingData; + HeapArray<uint8_t> mBackingData; }; } // namespace dawn::native