From 669797f002a90e8c56b47b8287e2c46b2bbe7513 Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 12:55:26 +0100 Subject: [PATCH 1/9] Coverity: Fix 1574354 Uninitialized scalar field Always zero initialize the `ArrayDesc` data member of `SurfaceMem` in the CUDA adapter. Simplify other construction logic. --- source/adapters/cuda/memory.hpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/source/adapters/cuda/memory.hpp b/source/adapters/cuda/memory.hpp index aa992f44bf..8deea536bf 100644 --- a/source/adapters/cuda/memory.hpp +++ b/source/adapters/cuda/memory.hpp @@ -197,20 +197,15 @@ struct SurfaceMem { void *HostPtr) : Arrays(Context->Devices.size(), CUarray{0}), SurfObjs(Context->Devices.size(), CUsurfObject{0}), - OuterMemStruct{OuterMemStruct}, - ImageFormat{ImageFormat}, ImageDesc{ImageDesc}, HostPtr{HostPtr} { + OuterMemStruct{OuterMemStruct}, ImageDesc{ImageDesc}, ArrayDesc{}, + HostPtr{HostPtr} { // We have to use hipArray3DCreate, which has some caveats. The height and // depth parameters must be set to 0 produce 1D or 2D arrays. image_desc // gives a minimum value of 1, so we need to convert the answer. ArrayDesc.NumChannels = 4; // Only support 4 channel image - ArrayDesc.Flags = 0; // No flags required ArrayDesc.Width = ImageDesc.width; - if (ImageDesc.type == UR_MEM_TYPE_IMAGE1D) { - ArrayDesc.Height = 0; - ArrayDesc.Depth = 0; - } else if (ImageDesc.type == UR_MEM_TYPE_IMAGE2D) { + if (ImageDesc.type == UR_MEM_TYPE_IMAGE2D) { ArrayDesc.Height = ImageDesc.height; - ArrayDesc.Depth = 0; } else if (ImageDesc.type == UR_MEM_TYPE_IMAGE3D) { ArrayDesc.Height = ImageDesc.height; ArrayDesc.Depth = ImageDesc.depth; From d08fc6a88000569d260cf6af688077f0367ff236 Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 13:39:01 +0100 Subject: [PATCH 2/9] Coverity: Fix 1594027 Uncaught exception The `UR_CHECK_ERROR()` utility macro in the CUDA adapter calls the `checkErrorUR()` utility function, this throws a `ur_result_t` which was not being caught. --- source/adapters/cuda/memory.hpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/source/adapters/cuda/memory.hpp b/source/adapters/cuda/memory.hpp index 8deea536bf..6dcaa28414 100644 --- a/source/adapters/cuda/memory.hpp +++ b/source/adapters/cuda/memory.hpp @@ -409,10 +409,14 @@ struct ur_mem_handle_t_ { } ur_result_t clear() { - if (isBuffer()) { - return std::get(Mem).clear(); + try { + if (isBuffer()) { + return std::get(Mem).clear(); + } + return std::get(Mem).clear(); + } catch (const ur_result_t &error) { + return error; } - return std::get(Mem).clear(); } ur_context_handle_t getContext() const noexcept { return Context; } From ee749e47b1a2d036c7dd656a5201e7834f346515 Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 14:46:03 +0100 Subject: [PATCH 3/9] Coverity: Fix 1595568, 1595570 Use of auto that causes a copy Use `const auto &` instead of `auto` in the mock parameter struct accesses. --- source/adapters/mock/ur_mock.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source/adapters/mock/ur_mock.cpp b/source/adapters/mock/ur_mock.cpp index b1fc9c8c29..c72c1e30ed 100644 --- a/source/adapters/mock/ur_mock.cpp +++ b/source/adapters/mock/ur_mock.cpp @@ -17,13 +17,14 @@ namespace driver { context_t d_context; ur_result_t mock_urPlatformGetApiVersion(void *pParams) { - auto params = *static_cast(pParams); + const auto ¶ms = + *static_cast(pParams); **params.ppVersion = d_context.version; return UR_RESULT_SUCCESS; } ur_result_t mock_urPlatformGetInfo(void *pParams) { - auto params = *static_cast(pParams); + const auto ¶ms = *static_cast(pParams); if (!*params.phPlatform) { return UR_RESULT_ERROR_INVALID_NULL_HANDLE; } @@ -49,7 +50,7 @@ ur_result_t mock_urPlatformGetInfo(void *pParams) { ////////////////////////////////////////////////////////////////////////// ur_result_t mock_urDeviceGetInfo(void *pParams) { - auto params = *static_cast(pParams); + const auto ¶ms = *static_cast(pParams); switch (*params.ppropName) { case UR_DEVICE_INFO_TYPE: if (*params.ppPropValue != nullptr) { From 7a370a4cdbc5c88fde6ab7ecb7e300c7965262f9 Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 15:15:19 +0100 Subject: [PATCH 4/9] Coverity: Fix 1595594 Copy instead of move --- source/common/ur_util.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/ur_util.hpp b/source/common/ur_util.hpp index e24c1153c5..0475cf31e4 100644 --- a/source/common/ur_util.hpp +++ b/source/common/ur_util.hpp @@ -462,7 +462,7 @@ template class AtomicSingleton { static int release(std::function deleter) { auto val = instance.acquire(); - int ret = val->release(deleter); + int ret = val->release(std::move(deleter)); instance.release(); return ret; From 132349c42e0a4be8d349803367f86d279f10fdad Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 15:36:16 +0100 Subject: [PATCH 5/9] Coverity: Fix 1595785 Use of auto that causes a copy --- source/loader/layers/sanitizer/asan_options.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/loader/layers/sanitizer/asan_options.hpp b/source/loader/layers/sanitizer/asan_options.hpp index ab6ee0c26b..72cfd74a3b 100644 --- a/source/loader/layers/sanitizer/asan_options.hpp +++ b/source/loader/layers/sanitizer/asan_options.hpp @@ -112,7 +112,7 @@ struct AsanOptions { KV = OptionsEnvMap->find("redzone"); if (KV != OptionsEnvMap->end()) { - auto Value = KV->second.front(); + const auto &Value = KV->second.front(); try { MinRZSize = std::stoul(Value); if (MinRZSize < 16) { From d51935ee12cd2e351a7393dd064ba3e079ddeaae Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 15:43:29 +0100 Subject: [PATCH 6/9] Coverity: Fix 1594597 Dereference after null check --- source/loader/layers/sanitizer/asan_report.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/loader/layers/sanitizer/asan_report.cpp b/source/loader/layers/sanitizer/asan_report.cpp index bdae3284b4..54e1da40e9 100644 --- a/source/loader/layers/sanitizer/asan_report.cpp +++ b/source/loader/layers/sanitizer/asan_report.cpp @@ -32,7 +32,7 @@ void ReportBadFree(uptr Addr, const StackTrace &stack, (void *)Addr); } - assert(!AI->IsReleased && "Chunk must be not released"); + assert(AI && !AI->IsReleased && "Chunk must be not released"); getContext()->logger.always("{} is located inside of {} region [{}, {})", (void *)Addr, ToString(AI->Type), From e150934a0460fbcd074477cf7f41e0c13d364765 Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 16:29:21 +0100 Subject: [PATCH 7/9] Coverity: Fix 1595225 Data race condition --- source/loader/layers/validation/ur_leak_check.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/source/loader/layers/validation/ur_leak_check.hpp b/source/loader/layers/validation/ur_leak_check.hpp index 56998797a3..1b588bc4f1 100644 --- a/source/loader/layers/validation/ur_leak_check.hpp +++ b/source/loader/layers/validation/ur_leak_check.hpp @@ -136,6 +136,7 @@ struct RefCountContext { void clear() { counts.clear(); } template bool isReferenceValid(T handle) { + std::unique_lock lock(mutex); auto it = counts.find(static_cast(handle)); if (it == counts.end() || it->second.refCount < 1) { return false; From 9f2166cce02271f12e2ea67247dd66c601a6a87f Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 17:49:38 +0100 Subject: [PATCH 8/9] Coverity: Fix 1598473 Resource leak --- test/adapters/level_zero/v2/command_list_cache_test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/adapters/level_zero/v2/command_list_cache_test.cpp b/test/adapters/level_zero/v2/command_list_cache_test.cpp index 0ce1b79d98..74bcbf4634 100644 --- a/test/adapters/level_zero/v2/command_list_cache_test.cpp +++ b/test/adapters/level_zero/v2/command_list_cache_test.cpp @@ -192,10 +192,10 @@ TEST_P(CommandListCacheTest, CommandListsAreReusedByQueues) { QueueProps.pNext = &IndexProps; } - ur_queue_handle_t Queue; - ASSERT_EQ( - urQueueCreate(context, device, &QueueProps, &Queue), - UR_RESULT_SUCCESS); + uur::raii::Queue Queue; + ASSERT_EQ(urQueueCreate(context, device, &QueueProps, + Queue.ptr()), + UR_RESULT_SUCCESS); Queues.emplace_back(Queue); } From b33c0e756d9ecfcd212f937131e7c22732cf0db2 Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Tue, 6 Aug 2024 18:07:42 +0100 Subject: [PATCH 9/9] Coverity: Fix 14 instances of Resource leak Addresses the following defect CIDs; 1594026, 1594028, 1594029, 1594030, 1594031, 1594032, 1594033, 1594034, 1594035, 1594036, 1594037, 1595372, 1595373, and 1598546. --- test/conformance/memory/urMemImageCreate.cpp | 77 ++++++++++---------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/test/conformance/memory/urMemImageCreate.cpp b/test/conformance/memory/urMemImageCreate.cpp index ea210a921f..28d5d9c4e3 100644 --- a/test/conformance/memory/urMemImageCreate.cpp +++ b/test/conformance/memory/urMemImageCreate.cpp @@ -26,10 +26,10 @@ struct urMemImageCreateTest : public uur::urContextTest { void SetUp() override { UUR_RETURN_ON_FATAL_FAILURE(uur::urContextTest::SetUp()); - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; auto ret = urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, - &image_desc, nullptr, &image_handle); + &image_desc, nullptr, image_handle.ptr()); if (ret == UR_RESULT_ERROR_UNSUPPORTED_FEATURE) { GTEST_SKIP() << "urMemImageCreate not supported"; @@ -50,10 +50,10 @@ struct urMemImageCreateTestWithParam UUR_RETURN_ON_FATAL_FAILURE( uur::urContextTestWithParam::SetUp()); - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; auto ret = urMemImageCreate(this->context, UR_MEM_FLAG_READ_WRITE, &image_format, &image_desc, nullptr, - &image_handle); + image_handle.ptr()); if (ret == UR_RESULT_ERROR_UNSUPPORTED_FEATURE) { GTEST_SKIP() << "urMemImageCreate not supported"; @@ -89,12 +89,11 @@ TEST_P(urMemImageCreateTestWith1DMemoryTypeParam, Success) { 0 ///< [in] number of samples }; - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ASSERT_SUCCESS(urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &image_desc_with_param, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); ASSERT_NE(nullptr, image_handle); - ASSERT_SUCCESS(urMemRelease(image_handle)); } using urMemImageCreateTestWith2DMemoryTypeParam = @@ -120,12 +119,11 @@ TEST_P(urMemImageCreateTestWith2DMemoryTypeParam, Success) { 0 ///< [in] number of samples }; - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ASSERT_SUCCESS(urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &image_desc_with_param, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); ASSERT_NE(nullptr, image_handle); - ASSERT_SUCCESS(urMemRelease(image_handle)); } TEST_P(urMemImageCreateTest, SuccessWith3DImageType) { @@ -143,28 +141,27 @@ TEST_P(urMemImageCreateTest, SuccessWith3DImageType) { 0 ///< [in] number of samples }; - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ASSERT_SUCCESS(urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &image_desc_with_param, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); ASSERT_NE(nullptr, image_handle); - ASSERT_SUCCESS(urMemRelease(image_handle)); } TEST_P(urMemImageCreateTest, InvalidNullHandleContext) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_HANDLE, urMemImageCreate(nullptr, UR_MEM_FLAG_READ_WRITE, &image_format, &image_desc, nullptr, - &image_handle)); + image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidEnumerationFlags) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_ENUMERATION, urMemImageCreate(context, UR_MEM_FLAG_FORCE_UINT32, &image_format, &image_desc, nullptr, - &image_handle)); + image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidNullPointerBuffer) { @@ -175,23 +172,24 @@ TEST_P(urMemImageCreateTest, InvalidNullPointerBuffer) { } TEST_P(urMemImageCreateTest, InvalidNullPointerImageDesc) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_POINTER, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, nullptr, nullptr, - &image_handle)); + image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidNullPointerImageFormat) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_POINTER, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, nullptr, - &image_desc, nullptr, &image_handle)); + &image_desc, nullptr, + image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidSize) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ur_image_desc_t invalid_image_desc = image_desc; invalid_image_desc.width = std::numeric_limits::max(); @@ -199,7 +197,7 @@ TEST_P(urMemImageCreateTest, InvalidSize) { ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_SIZE, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); invalid_image_desc = image_desc; invalid_image_desc.height = std::numeric_limits::max(); @@ -207,7 +205,7 @@ TEST_P(urMemImageCreateTest, InvalidSize) { ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_SIZE, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); invalid_image_desc = image_desc; invalid_image_desc.depth = std::numeric_limits::max(); @@ -215,21 +213,21 @@ TEST_P(urMemImageCreateTest, InvalidSize) { ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_SIZE, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidImageDescStype) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ur_image_desc_t invalid_image_desc = image_desc; invalid_image_desc.stype = UR_STRUCTURE_TYPE_FORCE_UINT32; ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidImageDescType) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ur_image_desc_t invalid_image_desc = image_desc; invalid_image_desc.type = UR_MEM_TYPE_FORCE_UINT32; @@ -237,11 +235,11 @@ TEST_P(urMemImageCreateTest, InvalidImageDescType) { ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidImageDescNumMipLevel) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ur_image_desc_t invalid_image_desc = image_desc; invalid_image_desc.numMipLevel = 1; /* Must be 0 */ @@ -249,11 +247,11 @@ TEST_P(urMemImageCreateTest, InvalidImageDescNumMipLevel) { ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidImageDescNumSamples) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ur_image_desc_t invalid_image_desc = image_desc; invalid_image_desc.numSamples = 1; /* Must be 0 */ @@ -261,11 +259,11 @@ TEST_P(urMemImageCreateTest, InvalidImageDescNumSamples) { ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidImageDescRowPitch) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ur_image_desc_t invalid_image_desc = image_desc; invalid_image_desc.rowPitch = 1; /* Must be 0 if pHost is NULL */ @@ -273,11 +271,11 @@ TEST_P(urMemImageCreateTest, InvalidImageDescRowPitch) { ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); } TEST_P(urMemImageCreateTest, InvalidImageDescSlicePitch) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ur_image_desc_t invalid_image_desc = image_desc; invalid_image_desc.slicePitch = 1; /* Must be 0 if pHost is NULL */ @@ -285,7 +283,7 @@ TEST_P(urMemImageCreateTest, InvalidImageDescSlicePitch) { ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR, urMemImageCreate(context, UR_MEM_FLAG_READ_WRITE, &image_format, &invalid_image_desc, - nullptr, &image_handle)); + nullptr, image_handle.ptr())); } using urMemImageCreateWithHostPtrFlagsTest = @@ -310,8 +308,9 @@ TEST_P(urMemImageCreateWithHostPtrFlagsTest, Success) { } TEST_P(urMemImageCreateWithHostPtrFlagsTest, InvalidHostPtr) { - ur_mem_handle_t image_handle = nullptr; + uur::raii::Mem image_handle = nullptr; ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_HOST_PTR, urMemImageCreate(context, getParam(), &image_format, - &image_desc, nullptr, &image_handle)); + &image_desc, nullptr, + image_handle.ptr())); }