From aad452948477e5531d515fe984eb55bde33fa7e6 Mon Sep 17 00:00:00 2001 From: "Bernhart, Bryan" Date: Wed, 29 Mar 2023 15:25:57 -0700 Subject: [PATCH] Misc coverity fixes. --- src/gpgmm/common/BlockAllocator.h | 3 +- src/gpgmm/common/Error.h | 2 ++ src/gpgmm/common/EventTraceWriter.cpp | 4 ++- src/gpgmm/d3d12/CapsD3D12.h | 2 +- src/gpgmm/d3d12/ErrorD3D12.cpp | 6 ++-- src/gpgmm/d3d12/FenceD3D12.h | 3 +- src/gpgmm/d3d12/ResidencyManagerD3D12.cpp | 12 ++++--- src/gpgmm/d3d12/ResidencyManagerD3D12.h | 1 - src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp | 5 +-- src/gpgmm/utils/JSONEncoder.cpp | 14 ++++++-- src/gpgmm/utils/JSONEncoder.h | 1 + src/gpgmm/utils/LinkedList.h | 37 ++++++++++++++++++++-- src/gpgmm/utils/Utils.h | 6 ++-- src/gpgmm/utils/WindowsTime.cpp | 2 +- 14 files changed, 75 insertions(+), 23 deletions(-) diff --git a/src/gpgmm/common/BlockAllocator.h b/src/gpgmm/common/BlockAllocator.h index 8d636be45..231028b7d 100644 --- a/src/gpgmm/common/BlockAllocator.h +++ b/src/gpgmm/common/BlockAllocator.h @@ -17,11 +17,12 @@ #include "gpgmm/common/MemoryBlock.h" #include "gpgmm/common/Object.h" +#include "gpgmm/utils/NonCopyable.h" namespace gpgmm { // Allocates a sub-range [offset, offset + size) in usually a byte-addressable range. - class BlockAllocator : public ObjectBase { + class BlockAllocator : public ObjectBase, public NonCopyable { public: ~BlockAllocator() override = default; diff --git a/src/gpgmm/common/Error.h b/src/gpgmm/common/Error.h index 38e456d4c..715ac0823 100644 --- a/src/gpgmm/common/Error.h +++ b/src/gpgmm/common/Error.h @@ -34,10 +34,12 @@ namespace gpgmm { public: // Empty result Result() : mErrorCode(kInternalFailureResult) { + mResult = {}; } // Error only result Result(ErrorT&& error) : mErrorCode(std::move(error)) { + mResult = {}; } // Result but with no error diff --git a/src/gpgmm/common/EventTraceWriter.cpp b/src/gpgmm/common/EventTraceWriter.cpp index d233c084d..a6114b8a5 100644 --- a/src/gpgmm/common/EventTraceWriter.cpp +++ b/src/gpgmm/common/EventTraceWriter.cpp @@ -65,7 +65,9 @@ namespace gpgmm { }; EventTraceWriter::EventTraceWriter() - : mTraceFile(kDefaultTraceFile), mPlatformTime(CreatePlatformTime()) { + : mTraceFile(kDefaultTraceFile), + mIgnoreMask(TraceEventPhase::None), + mPlatformTime(CreatePlatformTime()) { } void EventTraceWriter::SetConfiguration(const char* traceFile, diff --git a/src/gpgmm/d3d12/CapsD3D12.h b/src/gpgmm/d3d12/CapsD3D12.h index bfade67f9..c057dd99a 100644 --- a/src/gpgmm/d3d12/CapsD3D12.h +++ b/src/gpgmm/d3d12/CapsD3D12.h @@ -52,7 +52,7 @@ namespace gpgmm::d3d12 { uint64_t mMaxResourceSize = 0; uint64_t mMaxResourceHeapSize = 0; - D3D12_RESOURCE_HEAP_TIER mMaxResourceHeapTier; + D3D12_RESOURCE_HEAP_TIER mMaxResourceHeapTier = D3D12_RESOURCE_HEAP_TIER_1; bool mIsCreateHeapNotResidentSupported = false; bool mIsResourceAllocationWithinCoherent = false; bool mIsAdapterUMA = false; diff --git a/src/gpgmm/d3d12/ErrorD3D12.cpp b/src/gpgmm/d3d12/ErrorD3D12.cpp index b7af14d92..275221af5 100644 --- a/src/gpgmm/d3d12/ErrorD3D12.cpp +++ b/src/gpgmm/d3d12/ErrorD3D12.cpp @@ -37,9 +37,9 @@ namespace gpgmm::d3d12 { return "Device was not found but removed " + GetErrorMessage(error); } - HRESULT removedReason = device->GetDeviceRemovedReason(); - std::string removedReasonStr; - switch (error) { + const HRESULT removedReason = device->GetDeviceRemovedReason(); + std::string removedReasonStr = "Unknown."; + switch (removedReason) { case DXGI_ERROR_DEVICE_HUNG: { removedReasonStr = "HUNG"; break; diff --git a/src/gpgmm/d3d12/FenceD3D12.h b/src/gpgmm/d3d12/FenceD3D12.h index 9b93eb7eb..22b0d1b95 100644 --- a/src/gpgmm/d3d12/FenceD3D12.h +++ b/src/gpgmm/d3d12/FenceD3D12.h @@ -16,12 +16,13 @@ #define GPGMM_D3D12_FENCED3D12_H_ #include "gpgmm/d3d12/d3d12_platform.h" +#include "gpgmm/utils/NonCopyable.h" #include namespace gpgmm::d3d12 { - class Fence { + class Fence : public NonCopyable { public: static HRESULT CreateFence(ID3D12Device* device, uint64_t initialValue, Fence** fenceOut); diff --git a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp index 3eba30f07..2aa7c1966 100644 --- a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp +++ b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp @@ -52,6 +52,11 @@ namespace gpgmm::d3d12 { mBudgetNotificationUpdateEvent, &mCookie); } + ~BudgetUpdateTask() override { + CloseHandle(mUnregisterAndExitEvent); + CloseHandle(mBudgetNotificationUpdateEvent); + } + void operator()() override { HRESULT hr = GetLastError(); bool isExiting = false; @@ -273,8 +278,6 @@ namespace gpgmm::d3d12 { : descriptor.EvictSizeInBytes), mIsUMA(caps->IsAdapterUMA() && !(descriptor.Flags & RESIDENCY_FLAG_DISABLE_UNIFIED_MEMORY)), - mIsBudgetChangeEventsDisabled(descriptor.Flags & - RESIDENCY_FLAG_NEVER_UPDATE_BUDGET_ON_WORKER_THREAD), mFlushEventBuffersOnDestruct(descriptor.RecordOptions.EventScope & EventRecordScope::kPerInstance), mInitialFenceValue(descriptor.InitialFenceValue) { @@ -956,9 +959,8 @@ namespace gpgmm::d3d12 { } bool ResidencyManager::IsBudgetNotificationUpdatesDisabled() const { - return mIsBudgetChangeEventsDisabled || - (mBudgetNotificationUpdateEvent != nullptr && - FAILED(mBudgetNotificationUpdateEvent->GetLastError())); + return (mBudgetNotificationUpdateEvent == nullptr) || + FAILED(mBudgetNotificationUpdateEvent->GetLastError()); } void ResidencyManager::StopBudgetNotificationUpdates() { diff --git a/src/gpgmm/d3d12/ResidencyManagerD3D12.h b/src/gpgmm/d3d12/ResidencyManagerD3D12.h index 0f4234228..9fc5daff6 100644 --- a/src/gpgmm/d3d12/ResidencyManagerD3D12.h +++ b/src/gpgmm/d3d12/ResidencyManagerD3D12.h @@ -135,7 +135,6 @@ namespace gpgmm::d3d12 { const bool mIsBudgetRestricted; const uint64_t mEvictSizeInBytes; const bool mIsUMA; - const bool mIsBudgetChangeEventsDisabled; const bool mFlushEventBuffersOnDestruct; const uint64_t mInitialFenceValue; diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp index 67b7f508e..7d9e3c030 100644 --- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp +++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp @@ -1617,8 +1617,9 @@ namespace gpgmm::d3d12 { } data.IsResourceAllocationWithinCoherent = mCaps->IsResourceAllocationWithinCoherent(); - memcpy_s(pFeatureSupportData, sizeof(pFeatureSupportData), &data, - featureSupportDataSize); + FEATURE_DATA_RESOURCE_ALLOCATION_SUPPORT* pFeatureData = + static_cast(pFeatureSupportData); + memcpy_s(pFeatureData, featureSupportDataSize, &data, sizeof(data)); return S_OK; } default: { diff --git a/src/gpgmm/utils/JSONEncoder.cpp b/src/gpgmm/utils/JSONEncoder.cpp index 1212da77c..fc2c233c4 100644 --- a/src/gpgmm/utils/JSONEncoder.cpp +++ b/src/gpgmm/utils/JSONEncoder.cpp @@ -33,8 +33,10 @@ namespace gpgmm { } JSONDict& JSONDict::operator=(const JSONDict& other) { - mSS = std::stringstream(other.mSS.str()); - mHasItem = other.mHasItem; + if (this != &other) { + mSS = std::stringstream(other.mSS.str()); + mHasItem = other.mHasItem; + } return *this; } @@ -113,6 +115,14 @@ namespace gpgmm { mHasItem = other.mHasItem; } + JSONArray& JSONArray::operator=(const JSONArray& other) { + if (this != &other) { + mSS = std::stringstream(other.mSS.str()); + mHasItem = other.mHasItem; + } + return *this; + } + bool JSONArray::IsEmpty() const { return !mHasItem; } diff --git a/src/gpgmm/utils/JSONEncoder.h b/src/gpgmm/utils/JSONEncoder.h index 5a3a2ba6c..99902f3a4 100644 --- a/src/gpgmm/utils/JSONEncoder.h +++ b/src/gpgmm/utils/JSONEncoder.h @@ -57,6 +57,7 @@ namespace gpgmm { public: JSONArray(); JSONArray(const JSONArray& other); + JSONArray& operator=(const JSONArray& other); std::string ToString() const; bool IsEmpty() const; diff --git a/src/gpgmm/utils/LinkedList.h b/src/gpgmm/utils/LinkedList.h index 633dceaa7..c5e9b19c3 100644 --- a/src/gpgmm/utils/LinkedList.h +++ b/src/gpgmm/utils/LinkedList.h @@ -117,6 +117,26 @@ namespace gpgmm { } } + LinkNode& operator=(LinkNode&& rhs) { + if (this == &rhs) { + return *this; + } + + next_ = rhs.next_; + rhs.next_ = nullptr; + previous_ = rhs.previous_; + rhs.previous_ = nullptr; + + // If the node belongs to a list, next_ and previous_ are both non-null. + // Otherwise, they are both null. + if (next_) { + next_->previous_ = this; + previous_->next_ = this; + } + + return *this; + } + // Insert |this| into the linked list, before |e|. void InsertBefore(LinkNode* e) { this->next_ = e; @@ -239,6 +259,13 @@ namespace gpgmm { LinkedList(LinkedList&& other) noexcept : root_(std::move(other.root_)) { } + LinkedList& operator=(LinkedList&& rhs) { + if (this != &rhs) { + root_ = std::move(rhs.root_); + } + return *this; + } + LinkNode* head() const { return root_.next(); } @@ -255,12 +282,18 @@ namespace gpgmm { // ~T must check if IsInList and call RemoveFromList to unlink itself or clear // will ASSERT to indicate programmer error. void clear() { - for (auto& curr : *this) { - SafeDelete(curr.value()); + iterator curr = begin(); + while (curr != end()) { + curr = erase(curr); } ASSERT(empty()); } + iterator erase(iterator i) { + SafeDelete(i->value()); + return ++i; + } + iterator begin() { return iterator(head()); } diff --git a/src/gpgmm/utils/Utils.h b/src/gpgmm/utils/Utils.h index 9588b863f..eea067974 100644 --- a/src/gpgmm/utils/Utils.h +++ b/src/gpgmm/utils/Utils.h @@ -65,9 +65,9 @@ namespace gpgmm { return ToString(object) + ToString(args...); } - template - constexpr auto ConstexprConcat(A1 lhs, A2 rhs) { - std::array sizeArray{}; + template + constexpr auto ConstexprConcat(std::array lhs, std::array rhs) { + std::array sizeArray{}; size_t i = 0; for (auto& item : lhs) { diff --git a/src/gpgmm/utils/WindowsTime.cpp b/src/gpgmm/utils/WindowsTime.cpp index 532ca4fcc..b8767de9a 100644 --- a/src/gpgmm/utils/WindowsTime.cpp +++ b/src/gpgmm/utils/WindowsTime.cpp @@ -22,7 +22,7 @@ namespace gpgmm { class WindowsTime final : public PlatformTime { public: - WindowsTime() : PlatformTime(), mFrequency(0) { + WindowsTime() : PlatformTime(), mFrequency(0), mCounterStart(0) { } double GetAbsoluteTime() override {