Replace size_t with uint64_t in ringbuffer.
Adds overflow check in RingBufferAllocator + unit-test.
Also, update clients to use uint64_t to avoid casts or narrowing.
BUG=dawn:233
Change-Id: I652e3142407006d082491add600371f95d44741a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/12380
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
diff --git a/src/dawn_native/DynamicUploader.cpp b/src/dawn_native/DynamicUploader.cpp
index 876b789..9996ee1 100644
--- a/src/dawn_native/DynamicUploader.cpp
+++ b/src/dawn_native/DynamicUploader.cpp
@@ -18,7 +18,7 @@
namespace dawn_native {
- DynamicUploader::DynamicUploader(DeviceBase* device, size_t size) : mDevice(device) {
+ DynamicUploader::DynamicUploader(DeviceBase* device, uint64_t size) : mDevice(device) {
mRingBuffers.emplace_back(
std::unique_ptr<RingBuffer>(new RingBuffer{nullptr, RingBufferAllocator(size)}));
}
@@ -28,7 +28,7 @@
mDevice->GetPendingCommandSerial());
}
- ResultOrError<UploadHandle> DynamicUploader::Allocate(size_t allocationSize, Serial serial) {
+ ResultOrError<UploadHandle> DynamicUploader::Allocate(uint64_t allocationSize, Serial serial) {
// Note: Validation ensures size is already aligned.
// First-fit: find next smallest buffer large enough to satisfy the allocation request.
RingBuffer* targetRingBuffer = mRingBuffers.back().get();
@@ -36,7 +36,7 @@
const RingBufferAllocator& ringBufferAllocator = ringBuffer->mAllocator;
// Prevent overflow.
ASSERT(ringBufferAllocator.GetSize() >= ringBufferAllocator.GetUsedSize());
- const size_t remainingSize =
+ const uint64_t remainingSize =
ringBufferAllocator.GetSize() - ringBufferAllocator.GetUsedSize();
if (allocationSize <= remainingSize) {
targetRingBuffer = ringBuffer.get();
@@ -44,7 +44,7 @@
}
}
- size_t startOffset = RingBufferAllocator::kInvalidOffset;
+ uint64_t startOffset = RingBufferAllocator::kInvalidOffset;
if (targetRingBuffer != nullptr) {
startOffset = targetRingBuffer->mAllocator.Allocate(allocationSize, serial);
}
diff --git a/src/dawn_native/DynamicUploader.h b/src/dawn_native/DynamicUploader.h
index f0d4510..068eebd 100644
--- a/src/dawn_native/DynamicUploader.h
+++ b/src/dawn_native/DynamicUploader.h
@@ -25,13 +25,13 @@
struct UploadHandle {
uint8_t* mappedBuffer = nullptr;
- size_t startOffset = 0;
+ uint64_t startOffset = 0;
StagingBufferBase* stagingBuffer = nullptr;
};
class DynamicUploader {
public:
- DynamicUploader(DeviceBase* device, size_t size = kBaseUploadBufferSize);
+ DynamicUploader(DeviceBase* device, uint64_t size = kBaseUploadBufferSize);
~DynamicUploader() = default;
// We add functions to Release StagingBuffers to the DynamicUploader as there's
@@ -40,12 +40,12 @@
// implemented.
void ReleaseStagingBuffer(std::unique_ptr<StagingBufferBase> stagingBuffer);
- ResultOrError<UploadHandle> Allocate(size_t allocationSize, Serial serial);
+ ResultOrError<UploadHandle> Allocate(uint64_t allocationSize, Serial serial);
void Deallocate(Serial lastCompletedSerial);
private:
// TODO(bryan.bernhart@intel.com): Figure out this value.
- static constexpr size_t kBaseUploadBufferSize = 64000;
+ static constexpr uint64_t kBaseUploadBufferSize = 64000;
struct RingBuffer {
std::unique_ptr<StagingBufferBase> mStagingBuffer;
diff --git a/src/dawn_native/RingBufferAllocator.cpp b/src/dawn_native/RingBufferAllocator.cpp
index 6cb94b7..6c3eef0 100644
--- a/src/dawn_native/RingBufferAllocator.cpp
+++ b/src/dawn_native/RingBufferAllocator.cpp
@@ -29,7 +29,7 @@
// TODO(bryan.bernhart@intel.com): Follow-up with ringbuffer optimization.
namespace dawn_native {
- RingBufferAllocator::RingBufferAllocator(size_t maxSize) : mMaxBlockSize(maxSize) {
+ RingBufferAllocator::RingBufferAllocator(uint64_t maxSize) : mMaxBlockSize(maxSize) {
}
void RingBufferAllocator::Deallocate(Serial lastCompletedSerial) {
@@ -43,11 +43,11 @@
mInflightRequests.ClearUpTo(lastCompletedSerial);
}
- size_t RingBufferAllocator::GetSize() const {
+ uint64_t RingBufferAllocator::GetSize() const {
return mMaxBlockSize;
}
- size_t RingBufferAllocator::GetUsedSize() const {
+ uint64_t RingBufferAllocator::GetUsedSize() const {
return mUsedSize;
}
@@ -62,7 +62,7 @@
// queue, which identifies an existing (or new) frames-worth of resources. Internally, the
// ring-buffer maintains offsets of 3 "memory" states: Free, Reclaimed, and Used. This is done
// in FIFO order as older frames would free resources before newer ones.
- size_t RingBufferAllocator::Allocate(size_t allocationSize, Serial serial) {
+ uint64_t RingBufferAllocator::Allocate(uint64_t allocationSize, Serial serial) {
// Check if the buffer is full by comparing the used size.
// If the buffer is not split where waste occurs (e.g. cannot fit new sub-alloc in front), a
// subsequent sub-alloc could fail where the used size was previously adjusted to include
@@ -71,7 +71,13 @@
return kInvalidOffset;
}
- size_t startOffset = kInvalidOffset;
+ // Ensure adding allocationSize does not overflow.
+ const uint64_t remainingSize = (mMaxBlockSize - mUsedSize);
+ if (allocationSize > remainingSize) {
+ return kInvalidOffset;
+ }
+
+ uint64_t startOffset = kInvalidOffset;
// Check if the buffer is NOT split (i.e sub-alloc on ends)
if (mUsedStartOffset <= mUsedEndOffset) {
@@ -86,7 +92,7 @@
} else if (allocationSize <= mUsedStartOffset) { // Try to sub-alloc at front.
// Count the space at the end so that a subsequent
// sub-alloc cannot not succeed when the buffer is full.
- const size_t requestSize = (mMaxBlockSize - mUsedEndOffset) + allocationSize;
+ const uint64_t requestSize = (mMaxBlockSize - mUsedEndOffset) + allocationSize;
startOffset = 0;
mUsedEndOffset = allocationSize;
diff --git a/src/dawn_native/RingBufferAllocator.h b/src/dawn_native/RingBufferAllocator.h
index 60ee639..e437632 100644
--- a/src/dawn_native/RingBufferAllocator.h
+++ b/src/dawn_native/RingBufferAllocator.h
@@ -26,32 +26,32 @@
class RingBufferAllocator {
public:
RingBufferAllocator() = default;
- RingBufferAllocator(size_t maxSize);
+ RingBufferAllocator(uint64_t maxSize);
~RingBufferAllocator() = default;
- size_t Allocate(size_t allocationSize, Serial serial);
+ uint64_t Allocate(uint64_t allocationSize, Serial serial);
void Deallocate(Serial lastCompletedSerial);
- size_t GetSize() const;
+ uint64_t GetSize() const;
bool Empty() const;
- size_t GetUsedSize() const;
+ uint64_t GetUsedSize() const;
- static constexpr size_t kInvalidOffset = std::numeric_limits<size_t>::max();
+ static constexpr uint64_t kInvalidOffset = std::numeric_limits<uint64_t>::max();
private:
struct Request {
- size_t endOffset;
- size_t size;
+ uint64_t endOffset;
+ uint64_t size;
};
SerialQueue<Request> mInflightRequests; // Queue of the recorded sub-alloc requests (e.g.
// frame of resources).
- size_t mUsedEndOffset = 0; // Tail of used sub-alloc requests (in bytes).
- size_t mUsedStartOffset = 0; // Head of used sub-alloc requests (in bytes).
- size_t mMaxBlockSize = 0; // Max size of the ring buffer (in bytes).
- size_t mUsedSize = 0; // Size of the sub-alloc requests (in bytes) of the ring buffer.
- size_t mCurrentRequestSize =
+ uint64_t mUsedEndOffset = 0; // Tail of used sub-alloc requests (in bytes).
+ uint64_t mUsedStartOffset = 0; // Head of used sub-alloc requests (in bytes).
+ uint64_t mMaxBlockSize = 0; // Max size of the ring buffer (in bytes).
+ uint64_t mUsedSize = 0; // Size of the sub-alloc requests (in bytes) of the ring buffer.
+ uint64_t mCurrentRequestSize =
0; // Size of the sub-alloc requests (in bytes) of the current serial.
};
} // namespace dawn_native
diff --git a/src/dawn_native/d3d12/DescriptorHeapAllocator.cpp b/src/dawn_native/d3d12/DescriptorHeapAllocator.cpp
index a69641a..1980452 100644
--- a/src/dawn_native/d3d12/DescriptorHeapAllocator.cpp
+++ b/src/dawn_native/d3d12/DescriptorHeapAllocator.cpp
@@ -25,7 +25,7 @@
DescriptorHeapHandle::DescriptorHeapHandle(ComPtr<ID3D12DescriptorHeap> descriptorHeap,
uint32_t sizeIncrement,
- uint32_t offset)
+ uint64_t offset)
: mDescriptorHeap(descriptorHeap), mSizeIncrement(sizeIncrement), mOffset(offset) {
}
@@ -68,12 +68,11 @@
DescriptorHeapInfo* heapInfo,
D3D12_DESCRIPTOR_HEAP_FLAGS flags) {
const Serial pendingSerial = mDevice->GetPendingCommandSerial();
- size_t startOffset = (heapInfo->heap == nullptr)
- ? RingBufferAllocator::kInvalidOffset
- : heapInfo->allocator.Allocate(count, pendingSerial);
+ uint64_t startOffset = (heapInfo->heap == nullptr)
+ ? RingBufferAllocator::kInvalidOffset
+ : heapInfo->allocator.Allocate(count, pendingSerial);
if (startOffset != RingBufferAllocator::kInvalidOffset) {
- return DescriptorHeapHandle{heapInfo->heap, mSizeIncrements[type],
- static_cast<uint32_t>(startOffset)};
+ return DescriptorHeapHandle{heapInfo->heap, mSizeIncrements[type], startOffset};
}
// If the pool has no more space, replace the pool with a new one of the specified size
diff --git a/src/dawn_native/d3d12/DescriptorHeapAllocator.h b/src/dawn_native/d3d12/DescriptorHeapAllocator.h
index e4949a6..bcb6ff5 100644
--- a/src/dawn_native/d3d12/DescriptorHeapAllocator.h
+++ b/src/dawn_native/d3d12/DescriptorHeapAllocator.h
@@ -33,7 +33,7 @@
DescriptorHeapHandle();
DescriptorHeapHandle(ComPtr<ID3D12DescriptorHeap> descriptorHeap,
uint32_t sizeIncrement,
- uint32_t offset);
+ uint64_t offset);
ID3D12DescriptorHeap* Get() const;
D3D12_CPU_DESCRIPTOR_HANDLE GetCPUHandle(uint32_t index) const;
@@ -42,7 +42,7 @@
private:
ComPtr<ID3D12DescriptorHeap> mDescriptorHeap;
uint32_t mSizeIncrement;
- uint32_t mOffset;
+ uint64_t mOffset;
};
class DescriptorHeapAllocator {
diff --git a/src/tests/unittests/RingBufferAllocatorTests.cpp b/src/tests/unittests/RingBufferAllocatorTests.cpp
index 2455e8d..a3e5a62 100644
--- a/src/tests/unittests/RingBufferAllocatorTests.cpp
+++ b/src/tests/unittests/RingBufferAllocatorTests.cpp
@@ -18,11 +18,11 @@
using namespace dawn_native;
-constexpr size_t RingBufferAllocator::kInvalidOffset;
+constexpr uint64_t RingBufferAllocator::kInvalidOffset;
// Number of basic tests for Ringbuffer
TEST(RingBufferAllocatorTests, BasicTest) {
- constexpr size_t sizeInBytes = 64000;
+ constexpr uint64_t sizeInBytes = 64000;
RingBufferAllocator allocator(sizeInBytes);
// Ensure no requests exist on empty buffer.
@@ -43,8 +43,8 @@
// Tests that several ringbuffer allocations do not fail.
TEST(RingBufferAllocatorTests, RingBufferManyAlloc) {
- constexpr size_t maxNumOfFrames = 64000;
- constexpr size_t frameSizeInBytes = 4;
+ constexpr uint64_t maxNumOfFrames = 64000;
+ constexpr uint64_t frameSizeInBytes = 4;
RingBufferAllocator allocator(maxNumOfFrames * frameSizeInBytes);
@@ -57,8 +57,8 @@
// Tests ringbuffer sub-allocations of the same serial are correctly tracked.
TEST(RingBufferAllocatorTests, AllocInSameFrame) {
- constexpr size_t maxNumOfFrames = 3;
- constexpr size_t frameSizeInBytes = 4;
+ constexpr uint64_t maxNumOfFrames = 3;
+ constexpr uint64_t frameSizeInBytes = 4;
RingBufferAllocator allocator(maxNumOfFrames * frameSizeInBytes);
@@ -87,8 +87,8 @@
// Tests ringbuffer sub-allocation at various offsets.
TEST(RingBufferAllocatorTests, RingBufferSubAlloc) {
- constexpr size_t maxNumOfFrames = 10;
- constexpr size_t frameSizeInBytes = 4;
+ constexpr uint64_t maxNumOfFrames = 10;
+ constexpr uint64_t frameSizeInBytes = 4;
RingBufferAllocator allocator(maxNumOfFrames * frameSizeInBytes);
@@ -164,3 +164,14 @@
EXPECT_TRUE(allocator.Empty());
}
+
+// Checks if ringbuffer sub-allocation does not overflow.
+TEST(RingBufferAllocatorTests, RingBufferOverflow) {
+ Serial serial = 1;
+
+ RingBufferAllocator allocator(std::numeric_limits<uint64_t>::max());
+
+ ASSERT_EQ(allocator.Allocate(1, serial), 0u);
+ ASSERT_EQ(allocator.Allocate(std::numeric_limits<uint64_t>::max(), serial + 1),
+ RingBufferAllocator::kInvalidOffset);
+}
\ No newline at end of file
