From 32884a6ac014ea2dc8eae78c4e2026719fd4bd40 Mon Sep 17 00:00:00 2001 From: Aaron Greig Date: Mon, 16 Jun 2025 17:47:18 +0100 Subject: [PATCH 1/2] [UR] Fix some issues in CL PlatformGet turned up by static analysis. --- .../source/adapters/opencl/platform.cpp | 72 ++++++++++--------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/unified-runtime/source/adapters/opencl/platform.cpp b/unified-runtime/source/adapters/opencl/platform.cpp index bec8b8a805991..cd487fbd549da 100644 --- a/unified-runtime/source/adapters/opencl/platform.cpp +++ b/unified-runtime/source/adapters/opencl/platform.cpp @@ -72,41 +72,49 @@ urPlatformGet(ur_adapter_handle_t, uint32_t NumEntries, static std::mutex adapterPopulationMutex{}; ur_adapter_handle_t Adapter = nullptr; UR_RETURN_ON_FAILURE(urAdapterGet(1, &Adapter, nullptr)); - if (Adapter && !(Adapter->NumPlatforms)) { - std::lock_guard guard{adapterPopulationMutex}; - - // It's possible for urPlatformGet, if ran on multiple threads, to enter - // this branch simultaneously. This check ensures that only one sees that - // Adapter->NumPlatforms is zero. - if (Adapter->NumPlatforms == 0) { - uint32_t NumPlatforms = 0; - cl_int Res = clGetPlatformIDs(0, nullptr, &NumPlatforms); + if (!Adapter) { + // The only operation urAdapterGet really performs is allocating the adapter + // handle via new, so no adapter handle here almost certainly means memory + // problems. + return UR_RESULT_ERROR_OUT_OF_RESOURCES; + } - if (NumPlatforms == 0 || Res == CL_PLATFORM_NOT_FOUND_KHR) { - if (pNumPlatforms) { - *pNumPlatforms = 0; + { + std::lock_guard guard{adapterPopulationMutex}; + if (!Adapter->NumPlatforms) { + // It's possible for urPlatformGet, if ran on multiple threads, to enter + // this branch simultaneously. This check ensures that only one sees that + // Adapter->NumPlatforms is zero. + if (Adapter->NumPlatforms == 0) { + uint32_t NumPlatforms = 0; + cl_int Res = clGetPlatformIDs(0, nullptr, &NumPlatforms); + + if (NumPlatforms == 0 || Res == CL_PLATFORM_NOT_FOUND_KHR) { + if (pNumPlatforms) { + *pNumPlatforms = 0; + } + return UR_RESULT_SUCCESS; } - return UR_RESULT_SUCCESS; - } - CL_RETURN_ON_FAILURE(Res); - - std::vector CLPlatforms(NumPlatforms); - Res = clGetPlatformIDs(static_cast(NumPlatforms), - CLPlatforms.data(), nullptr); - CL_RETURN_ON_FAILURE(Res); - - try { - for (uint32_t i = 0; i < NumPlatforms; i++) { - auto URPlatform = - std::make_unique(CLPlatforms[i]); - UR_RETURN_ON_FAILURE(URPlatform->InitDevices()); - Adapter->URPlatforms.emplace_back(URPlatform.release()); + CL_RETURN_ON_FAILURE(Res); + + std::vector CLPlatforms(NumPlatforms); + Res = clGetPlatformIDs(static_cast(NumPlatforms), + CLPlatforms.data(), nullptr); + CL_RETURN_ON_FAILURE(Res); + + try { + for (uint32_t i = 0; i < NumPlatforms; i++) { + auto URPlatform = + std::make_unique(CLPlatforms[i]); + UR_RETURN_ON_FAILURE(URPlatform->InitDevices()); + Adapter->URPlatforms.emplace_back(URPlatform.release()); + } + Adapter->NumPlatforms = NumPlatforms; + } catch (std::bad_alloc &) { + return UR_RESULT_ERROR_OUT_OF_RESOURCES; + } catch (...) { + return UR_RESULT_ERROR_INVALID_PLATFORM; } - Adapter->NumPlatforms = NumPlatforms; - } catch (std::bad_alloc &) { - return UR_RESULT_ERROR_OUT_OF_RESOURCES; - } catch (...) { - return UR_RESULT_ERROR_INVALID_PLATFORM; } } } From 0aae3c9cfde242a37a0f03f148e9a93b3e5c187f Mon Sep 17 00:00:00 2001 From: Aaron Greig Date: Tue, 17 Jun 2025 10:12:32 +0100 Subject: [PATCH 2/2] Revent lock change. --- .../source/adapters/opencl/platform.cpp | 65 +++++++++---------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/unified-runtime/source/adapters/opencl/platform.cpp b/unified-runtime/source/adapters/opencl/platform.cpp index cd487fbd549da..b1140aadd43c4 100644 --- a/unified-runtime/source/adapters/opencl/platform.cpp +++ b/unified-runtime/source/adapters/opencl/platform.cpp @@ -79,42 +79,41 @@ urPlatformGet(ur_adapter_handle_t, uint32_t NumEntries, return UR_RESULT_ERROR_OUT_OF_RESOURCES; } - { + if (Adapter->NumPlatforms == 0) { std::lock_guard guard{adapterPopulationMutex}; - if (!Adapter->NumPlatforms) { - // It's possible for urPlatformGet, if ran on multiple threads, to enter - // this branch simultaneously. This check ensures that only one sees that - // Adapter->NumPlatforms is zero. - if (Adapter->NumPlatforms == 0) { - uint32_t NumPlatforms = 0; - cl_int Res = clGetPlatformIDs(0, nullptr, &NumPlatforms); - - if (NumPlatforms == 0 || Res == CL_PLATFORM_NOT_FOUND_KHR) { - if (pNumPlatforms) { - *pNumPlatforms = 0; - } - return UR_RESULT_SUCCESS; + + // It's possible for urPlatformGet, if ran on multiple threads, to enter + // this branch simultaneously. This check ensures that only one sees that + // Adapter->NumPlatforms is zero. + if (Adapter->NumPlatforms == 0) { + uint32_t NumPlatforms = 0; + cl_int Res = clGetPlatformIDs(0, nullptr, &NumPlatforms); + + if (NumPlatforms == 0 || Res == CL_PLATFORM_NOT_FOUND_KHR) { + if (pNumPlatforms) { + *pNumPlatforms = 0; } - CL_RETURN_ON_FAILURE(Res); - - std::vector CLPlatforms(NumPlatforms); - Res = clGetPlatformIDs(static_cast(NumPlatforms), - CLPlatforms.data(), nullptr); - CL_RETURN_ON_FAILURE(Res); - - try { - for (uint32_t i = 0; i < NumPlatforms; i++) { - auto URPlatform = - std::make_unique(CLPlatforms[i]); - UR_RETURN_ON_FAILURE(URPlatform->InitDevices()); - Adapter->URPlatforms.emplace_back(URPlatform.release()); - } - Adapter->NumPlatforms = NumPlatforms; - } catch (std::bad_alloc &) { - return UR_RESULT_ERROR_OUT_OF_RESOURCES; - } catch (...) { - return UR_RESULT_ERROR_INVALID_PLATFORM; + return UR_RESULT_SUCCESS; + } + CL_RETURN_ON_FAILURE(Res); + + std::vector CLPlatforms(NumPlatforms); + Res = clGetPlatformIDs(static_cast(NumPlatforms), + CLPlatforms.data(), nullptr); + CL_RETURN_ON_FAILURE(Res); + + try { + for (uint32_t i = 0; i < NumPlatforms; i++) { + auto URPlatform = + std::make_unique(CLPlatforms[i]); + UR_RETURN_ON_FAILURE(URPlatform->InitDevices()); + Adapter->URPlatforms.emplace_back(URPlatform.release()); } + Adapter->NumPlatforms = NumPlatforms; + } catch (std::bad_alloc &) { + return UR_RESULT_ERROR_OUT_OF_RESOURCES; + } catch (...) { + return UR_RESULT_ERROR_INVALID_PLATFORM; } } }