From 3b865a4d55df2744fc99e1301ca5d00b23b875ff Mon Sep 17 00:00:00 2001 From: "Bernhart, Bryan" Date: Wed, 9 Aug 2023 15:07:48 -0700 Subject: [PATCH] Return error results on validation checks. --- src/gpgmm/common/BuddyMemoryAllocator.cpp | 4 +- src/gpgmm/common/DedicatedMemoryAllocator.cpp | 2 +- src/gpgmm/common/Error.h | 50 +++++++++++++++++-- src/gpgmm/common/MemoryAllocator.cpp | 10 ++-- src/gpgmm/common/MemoryAllocator.h | 2 +- src/gpgmm/common/PooledMemoryAllocator.cpp | 2 +- src/gpgmm/common/SegmentedMemoryAllocator.cpp | 2 +- src/gpgmm/common/SlabMemoryAllocator.cpp | 8 +-- src/gpgmm/vk/DeviceMemoryAllocatorVk.cpp | 2 +- 9 files changed, 61 insertions(+), 21 deletions(-) diff --git a/src/gpgmm/common/BuddyMemoryAllocator.cpp b/src/gpgmm/common/BuddyMemoryAllocator.cpp index af682561..809c4f5d 100644 --- a/src/gpgmm/common/BuddyMemoryAllocator.cpp +++ b/src/gpgmm/common/BuddyMemoryAllocator.cpp @@ -48,13 +48,13 @@ namespace gpgmm { std::lock_guard lock(mMutex); - GPGMM_RETURN_ERROR_IF(!ValidateRequest(request)); + GPGMM_RETURN_IF_ERROR(ValidateRequest(request)); // Round allocation size to nearest power-of-two. const uint64_t allocationSize = UpperPowerOfTwo(request.SizeInBytes); // Request cannot exceed memory size. - GPGMM_RETURN_ERROR_IF(allocationSize > mMemorySize); + GPGMM_INVALID_IF(allocationSize > mMemorySize); // Attempt to sub-allocate a block of the requested size. std::unique_ptr subAllocation; diff --git a/src/gpgmm/common/DedicatedMemoryAllocator.cpp b/src/gpgmm/common/DedicatedMemoryAllocator.cpp index 112aee9d..9dcfc7c6 100644 --- a/src/gpgmm/common/DedicatedMemoryAllocator.cpp +++ b/src/gpgmm/common/DedicatedMemoryAllocator.cpp @@ -33,7 +33,7 @@ namespace gpgmm { std::lock_guard lock(mMutex); - GPGMM_RETURN_ERROR_IF(!ValidateRequest(request)); + GPGMM_RETURN_IF_ERROR(ValidateRequest(request)); MemoryAllocationRequest memoryRequest = request; memoryRequest.Alignment = mMemoryAlignment; diff --git a/src/gpgmm/common/Error.h b/src/gpgmm/common/Error.h index 86bcc898..a2459899 100644 --- a/src/gpgmm/common/Error.h +++ b/src/gpgmm/common/Error.h @@ -30,11 +30,21 @@ for (;;) \ break -#define GPGMM_RETURN_ERROR_IF(expr) \ - if (GPGMM_UNLIKELY(expr)) { \ - return {}; \ - } \ - for (;;) \ +#define GPGMM_RETURN_IF_ERROR(expr) \ + { \ + auto result = expr; \ + if (GPGMM_UNLIKELY(!result.IsSuccess())) { \ + return {}; \ + } \ + } \ + for (;;) \ + break + +#define GPGMM_INVALID_IF(expr) \ + if (GPGMM_UNLIKELY(expr)) { \ + return {}; \ + } \ + for (;;) \ break namespace gpgmm { @@ -96,6 +106,36 @@ namespace gpgmm { ResultT mResult; }; + // Specialization of Result where the ResultT is void. + // Used when a void function must return a Result with only error code. + template + class Result { + public: + Result() : mErrorCode(kInternalSuccessResult) { + } + + Result(ErrorT&& error) : mErrorCode(std::move(error)) { + } + + Result(Result&& other) : mErrorCode(std::move(other.mErrorCode)) { + } + + Result& operator=(Result&& other) { + mErrorCode = std::move(other.mErrorCode); + return *this; + } + + bool IsSuccess() const { + return mErrorCode == kInternalSuccessResult; + } + + private: + ErrorT mErrorCode; + }; + + // Result with only an error code. + using MaybeError = Result; + // Alias of Result + error code to avoid having to always specify error type. template using ResultOrError = Result; diff --git a/src/gpgmm/common/MemoryAllocator.cpp b/src/gpgmm/common/MemoryAllocator.cpp index 6b15d58e..65a133ca 100644 --- a/src/gpgmm/common/MemoryAllocator.cpp +++ b/src/gpgmm/common/MemoryAllocator.cpp @@ -156,7 +156,7 @@ namespace gpgmm { return mStats; } - bool MemoryAllocatorBase::ValidateRequest(const MemoryAllocationRequest& request) const { + MaybeError MemoryAllocatorBase::ValidateRequest(const MemoryAllocationRequest& request) const { ASSERT(request.SizeInBytes > 0 && request.Alignment > 0); // Check request size cannot overflow. @@ -164,7 +164,7 @@ namespace gpgmm { DebugLog(MessageId::kSizeExceeded, false, GetTypename(), this) << "Requested size rejected due to overflow: " + std::to_string(request.SizeInBytes) << " bytes."; - return false; + return std::move(ErrorCodeType(kInternalFailureResult)); } // Check request size cannot overflow |this| memory allocator. @@ -173,7 +173,7 @@ namespace gpgmm { DebugLog(MessageId::kSizeExceeded, false, GetTypename(), this) << "Requested size exceeds memory size (" + std::to_string(alignedSize) + " vs " + std::to_string(GetMemorySize()) + " bytes)."; - return false; + return std::move(ErrorCodeType(kInternalFailureResult)); } // Check request size has compatible alignment with |this| memory allocator. @@ -184,10 +184,10 @@ namespace gpgmm { << "Requested alignment exceeds memory alignment (" + std::to_string(request.Alignment) + " vs " + std::to_string(GetMemoryAlignment()) + " bytes)."; - return false; + return std::move(ErrorCodeType(kInternalFailureResult)); } - return true; + return {}; } MemoryAllocatorBase* MemoryAllocatorBase::GetNextInChain() const { diff --git a/src/gpgmm/common/MemoryAllocator.h b/src/gpgmm/common/MemoryAllocator.h index bc9dafb7..a837bd9a 100644 --- a/src/gpgmm/common/MemoryAllocator.h +++ b/src/gpgmm/common/MemoryAllocator.h @@ -192,7 +192,7 @@ namespace gpgmm { virtual MemoryAllocatorStats GetStats() const; // Checks if the request is valid. - bool ValidateRequest(const MemoryAllocationRequest& request) const; + MaybeError ValidateRequest(const MemoryAllocationRequest& request) const; // Return the next MemoryAllocatorBase. MemoryAllocatorBase* GetNextInChain() const; diff --git a/src/gpgmm/common/PooledMemoryAllocator.cpp b/src/gpgmm/common/PooledMemoryAllocator.cpp index c2057e67..813271a4 100644 --- a/src/gpgmm/common/PooledMemoryAllocator.cpp +++ b/src/gpgmm/common/PooledMemoryAllocator.cpp @@ -42,7 +42,7 @@ namespace gpgmm { std::lock_guard lock(mMutex); - GPGMM_RETURN_ERROR_IF(!ValidateRequest(request)); + GPGMM_RETURN_IF_ERROR(ValidateRequest(request)); MemoryAllocationBase allocation = mPool->AcquireFromPool(); if (allocation == GPGMM_INVALID_ALLOCATION) { diff --git a/src/gpgmm/common/SegmentedMemoryAllocator.cpp b/src/gpgmm/common/SegmentedMemoryAllocator.cpp index f9cf258c..ba9d8530 100644 --- a/src/gpgmm/common/SegmentedMemoryAllocator.cpp +++ b/src/gpgmm/common/SegmentedMemoryAllocator.cpp @@ -126,7 +126,7 @@ namespace gpgmm { std::lock_guard lock(mMutex); - GPGMM_RETURN_ERROR_IF(!ValidateRequest(request)); + GPGMM_RETURN_IF_ERROR(ValidateRequest(request)); const uint64_t memorySize = AlignTo(request.SizeInBytes, mMemoryAlignment); MemorySegment* segment = GetOrCreateFreeSegment(memorySize); diff --git a/src/gpgmm/common/SlabMemoryAllocator.cpp b/src/gpgmm/common/SlabMemoryAllocator.cpp index 5c924c8d..b0e11bde 100644 --- a/src/gpgmm/common/SlabMemoryAllocator.cpp +++ b/src/gpgmm/common/SlabMemoryAllocator.cpp @@ -202,7 +202,7 @@ namespace gpgmm { std::lock_guard lock(mMutex); - GPGMM_RETURN_ERROR_IF(request.SizeInBytes > mBlockSize); + GPGMM_INVALID_IF(request.SizeInBytes > mBlockSize); uint64_t slabSize = ComputeSlabSize(request.SizeInBytes, std::max(mMinSlabSize, mLastUsedSlabSize), @@ -229,7 +229,7 @@ namespace gpgmm { uint64_t newSlabSize = ComputeSlabSize( request.SizeInBytes, static_cast(slabSize * mSlabGrowthFactor), request.AvailableForAllocation); - GPGMM_RETURN_ERROR_IF(newSlabSize == kInvalidSize); + GPGMM_INVALID_IF(newSlabSize == kInvalidSize); // If the new slab size exceeds the limit, then re-use the previous, smaller size. if (newSlabSize > mMaxSlabSize) { @@ -515,10 +515,10 @@ namespace gpgmm { std::lock_guard lock(mMutex); - GPGMM_RETURN_ERROR_IF(!ValidateRequest(request)); + GPGMM_RETURN_IF_ERROR(ValidateRequest(request)); const uint64_t blockSize = AlignTo(request.SizeInBytes, request.Alignment); - GPGMM_RETURN_ERROR_IF(blockSize > mMaxSlabSize); + GPGMM_INVALID_IF(blockSize > mMaxSlabSize); // Create a slab allocator for the new entry. auto entry = diff --git a/src/gpgmm/vk/DeviceMemoryAllocatorVk.cpp b/src/gpgmm/vk/DeviceMemoryAllocatorVk.cpp index 0f53f116..da6b9fcc 100644 --- a/src/gpgmm/vk/DeviceMemoryAllocatorVk.cpp +++ b/src/gpgmm/vk/DeviceMemoryAllocatorVk.cpp @@ -47,7 +47,7 @@ namespace gpgmm::vk { return {}; } - GPGMM_RETURN_ERROR_IF(!ValidateRequest(request)); + GPGMM_RETURN_IF_ERROR(ValidateRequest(request)); VkMemoryAllocateInfo allocateInfo = {}; allocateInfo.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO;