From fee59b2c58455b298bfdf3cd1c3037fa381e8506 Mon Sep 17 00:00:00 2001 From: Charis Poag Date: Mon, 27 Oct 2025 13:07:49 -0500 Subject: [PATCH] [SWDEV-562726] Fix clang + ASAN errors * Updates: - [ASAN] GCC does not support `-shared-libsan flags`, so removed this one - [Clang] Fixed refernces to local binding errors (name collision) & other strict scope/structure/lamda binding errors - [Clang] Fix rsmi_wrapper error: \"error: missing default argument on parameter \'args\'\" - [ASAN] Fixed stack-buffer-overflow found in `amdsmi_get_gpu_accelerator_partition_profile()` Change-Id: I854007efb75d828dbb8088c0d56dbc125081f0f2 Signed-off-by: Charis Poag [ROCm/amdsmi commit: 00a04f581085b647671a928f596ba0374b521a38] --- .../amdsmi/cmake_modules/help_package.cmake | 8 +- .../rocm_smi/src/rocm_smi_gpu_metrics.cc | 85 ++++++++----------- projects/amdsmi/src/amd_smi/amd_smi.cc | 6 +- .../functional/dynamic_metrics_test.cc | 7 +- 4 files changed, 48 insertions(+), 58 deletions(-) diff --git a/projects/amdsmi/cmake_modules/help_package.cmake b/projects/amdsmi/cmake_modules/help_package.cmake index b6748c8d21..1d235463f8 100644 --- a/projects/amdsmi/cmake_modules/help_package.cmake +++ b/projects/amdsmi/cmake_modules/help_package.cmake @@ -40,12 +40,14 @@ function(generic_package) set(ASAN_LINKER_FLAGS "-fsanitize=address") if(BUILD_SHARED_LIBS) - set(ASAN_COMPILER_FLAGS "${ASAN_COMPILER_FLAGS} -shared-libsan") - set(ASAN_LINKER_FLAGS "${ASAN_LINKER_FLAGS} -shared-libsan") + # Clang-specific flag for shared ASAN library + if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + set(ASAN_COMPILER_FLAGS "${ASAN_COMPILER_FLAGS} -shared-libsan") + set(ASAN_LINKER_FLAGS "${ASAN_LINKER_FLAGS} -shared-libsan") + endif() else() set(ASAN_LINKER_FLAGS "${ASAN_LINKER_FLAGS} -static-libsan") endif() - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${ASAN_COMPILER_FLAGS}" PARENT_SCOPE) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${ASAN_COMPILER_FLAGS}" PARENT_SCOPE) set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${ASAN_LINKER_FLAGS}" PARENT_SCOPE) diff --git a/projects/amdsmi/rocm_smi/src/rocm_smi_gpu_metrics.cc b/projects/amdsmi/rocm_smi/src/rocm_smi_gpu_metrics.cc index f89dac2b71..c86aa3087c 100644 --- a/projects/amdsmi/rocm_smi/src/rocm_smi_gpu_metrics.cc +++ b/projects/amdsmi/rocm_smi/src/rocm_smi_gpu_metrics.cc @@ -478,66 +478,53 @@ template struct is_vector : std::false_type {}; template struct is_vector> : std::true_type {}; template -AMDGpuDynamicMetricTblValues_t format_metric_row(const T& metric, const std::string& value_title) -{ +AMDGpuDynamicMetricTblValues_t format_metric_row(const T& metric, const std::string& value_title) { auto multi_values = AMDGpuDynamicMetricTblValues_t{}; - auto get_data_type_info = [&]() { - auto data_type(AMDGpuMetricsDataType_t::kUInt64); - if constexpr (std::is_array_v) { - const uint8_t kCheckUint8[]={1}; - const uint16_t kCheckUint16[]={2}; - const uint32_t kCheckUint32[]={3}; - const uint64_t kCheckUint64[]={4}; - if constexpr (std::is_same_v) { - data_type = AMDGpuMetricsDataType_t::kUInt8; - } - if constexpr (std::is_same_v) { - data_type = AMDGpuMetricsDataType_t::kUInt16; - } - if constexpr (std::is_same_v) { - data_type = AMDGpuMetricsDataType_t::kUInt32; - } - if constexpr (std::is_same_v) { - data_type = AMDGpuMetricsDataType_t::kUInt64; - } - return std::make_tuple(data_type, static_cast(std::end(metric) - std::begin(metric))); - } + // Determine data type and count inline + AMDGpuMetricsDataType_t inferred_data_type = AMDGpuMetricsDataType_t::kUInt64; + uint16_t num_values = 1; - const uint16_t kSingleValue(1); - if constexpr (std::is_same_v) { - data_type = AMDGpuMetricsDataType_t::kUInt8; + if constexpr (std::is_array_v) { + using ElementType = std::remove_extent_t; + if constexpr (std::is_same_v) { + inferred_data_type = AMDGpuMetricsDataType_t::kUInt8; + } else if constexpr (std::is_same_v) { + inferred_data_type = AMDGpuMetricsDataType_t::kUInt16; + } else if constexpr (std::is_same_v) { + inferred_data_type = AMDGpuMetricsDataType_t::kUInt32; + } else if constexpr (std::is_same_v) { + inferred_data_type = AMDGpuMetricsDataType_t::kUInt64; } - if constexpr (std::is_same_v) { - data_type = AMDGpuMetricsDataType_t::kUInt16; + num_values = static_cast(std::extent_v); + } else { + // Scalar case + if constexpr (std::is_same_v) { + inferred_data_type = AMDGpuMetricsDataType_t::kUInt8; + } else if constexpr (std::is_same_v) { + inferred_data_type = AMDGpuMetricsDataType_t::kUInt16; + } else if constexpr (std::is_same_v) { + inferred_data_type = AMDGpuMetricsDataType_t::kUInt32; + } else if constexpr (std::is_same_v) { + inferred_data_type = AMDGpuMetricsDataType_t::kUInt64; } - if constexpr (std::is_same_v) { - data_type = AMDGpuMetricsDataType_t::kUInt32; - } - if constexpr (std::is_same_v) { - data_type = AMDGpuMetricsDataType_t::kUInt64; - } - return std::make_tuple(data_type, kSingleValue); - }; + } - const auto [data_type, num_values] = get_data_type_info(); - for (auto idx = uint16_t(0); idx < num_values; ++idx) { - auto value = uint64_t(0); + // Populate the values + for (uint16_t idx = 0; idx < num_values; ++idx) { + uint64_t value = 0; if constexpr (std::is_array_v) { - value = (metric[idx]); + value = static_cast(metric[idx]); } else { - value = (metric); + value = static_cast(metric); } - auto amdgpu_dynamic_metric_value = [&]() { - AMDGpuDynamicMetricsValue_t amdgpu_dynamic_metric_value_init{}; - amdgpu_dynamic_metric_value_init.m_value = value; - amdgpu_dynamic_metric_value_init.m_info = (value_title + " : " + std::to_string(idx)); - amdgpu_dynamic_metric_value_init.m_original_type = data_type; - return amdgpu_dynamic_metric_value_init; - }(); + AMDGpuDynamicMetricsValue_t amdgpu_dynamic_metric_value_init{}; + amdgpu_dynamic_metric_value_init.m_value = value; + amdgpu_dynamic_metric_value_init.m_info = (value_title + " : " + std::to_string(idx)); + amdgpu_dynamic_metric_value_init.m_original_type = inferred_data_type; - multi_values.emplace_back(amdgpu_dynamic_metric_value); + multi_values.emplace_back(amdgpu_dynamic_metric_value_init); } return multi_values; diff --git a/projects/amdsmi/src/amd_smi/amd_smi.cc b/projects/amdsmi/src/amd_smi/amd_smi.cc index c6490dcdfb..42aaa21f21 100644 --- a/projects/amdsmi/src/amd_smi/amd_smi.cc +++ b/projects/amdsmi/src/amd_smi/amd_smi.cc @@ -145,7 +145,7 @@ static amdsmi_status_t get_gpu_device_from_handle(amdsmi_processor_handle proces template amdsmi_status_t rsmi_wrapper(F && f, - amdsmi_processor_handle processor_handle, uint32_t increment_gpu_id = 0, Args &&... args) { + amdsmi_processor_handle processor_handle, uint32_t increment_gpu_id, Args &&... args) { AMDSMI_CHECK_INIT(); @@ -3057,8 +3057,8 @@ amdsmi_get_gpu_accelerator_partition_profile(amdsmi_processor_handle processor_h std::end(tokens), amd::smi::make_ostream_joiner(&ss_1, ", ")); - constexpr uint32_t kCurrentPartitionSize = 5; - char current_partition[kCurrentPartitionSize]; + constexpr uint32_t kCurrentPartitionSize = 16; + char current_partition[kCurrentPartitionSize] = {0}; std::string current_partition_str = "N/A"; amdsmi_status_t compute_status = amdsmi_get_gpu_compute_partition(processor_handle, current_partition, kCurrentPartitionSize); diff --git a/projects/amdsmi/tests/amd_smi_test/functional/dynamic_metrics_test.cc b/projects/amdsmi/tests/amd_smi_test/functional/dynamic_metrics_test.cc index f1806ed113..7cc4476b32 100644 --- a/projects/amdsmi/tests/amd_smi_test/functional/dynamic_metrics_test.cc +++ b/projects/amdsmi/tests/amd_smi_test/functional/dynamic_metrics_test.cc @@ -76,7 +76,8 @@ auto GetExpectedMetricVersionFlag(uint16_t major, uint16_t minor, bool is_partit } // pass a header we want to test against -auto BuildFakeMetricsBlob(amd::smi::AMDGpuMetricsHeader_v1_t new_header) -> std::vector { +auto BuildFakeMetricsBlob(amd::smi::AMDGpuMetricsHeader_v1_t new_header) + -> std::vector { if (new_header.m_structure_size < sizeof(new_header)) { throw std::runtime_error("Header size too small"); } @@ -119,7 +120,7 @@ TEST(AmdSmiDynamicMetricTest, GPUMetricDynamicVersionSupported) { const auto blob = BuildFakeMetricsBlob(amd::smi::AMDGpuMetricsHeader_v1_t{ .m_structure_size = sizeof(amd::smi::AMDGpuMetricsHeader_v1_t), .m_format_revision = 1, - .m_content_revision = static_cast(ver), // Known minor versions + .m_content_revision = static_cast(ver), // Known minor versions }); const auto fake_path = WriteBlobToTempFile(blob, "amdsmi_fake_gpu_metrics_v1" + std::to_string(ver) + ".bin"); @@ -170,7 +171,7 @@ TEST(AmdSmiDynamicMetricTest, XCPMetricDynamicVersionSupported) { const auto blob = BuildFakeMetricsBlob(amd::smi::AMDGpuMetricsHeader_v1_t{ .m_structure_size = sizeof(amd::smi::AMDGpuMetricsHeader_v1_t), .m_format_revision = 1, - .m_content_revision = static_cast(ver), // Known minor versions + .m_content_revision = static_cast(ver), // Known minor versions }); const auto fake_path = WriteBlobToTempFile(blob, "amdsmi_fake_xcp_metrics_v1" + std::to_string(ver) + ".bin");