From 2184d0c3d7c6666c972f62ea28b49852a0390d4a Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Tue, 3 Jan 2023 16:24:49 -0600 Subject: [PATCH 1/2] SWDEV-374138 - Improve ASAN flags Tests overwritten the linker flags resulting in failed build with clang. This change improves ASAN linker flag assignment and fixes test issue. Change-Id: I88f38360d46b20f6cc7298ad0d1fd09ff6ce47d6 Signed-off-by: Galantsev, Dmitrii --- CMakeLists.txt | 15 --------------- cmake_modules/help_package.cmake | 31 +++++++++++++++++++++++++------ tests/amd_smi_test/CMakeLists.txt | 2 +- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 36713e8c33..31aa11d5dc 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -76,21 +76,6 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wformat=2 -fno-common -Wstrict-overflow # Intentionally leave out -Wsign-promo. It causes spurious warnings. set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Woverloaded-virtual -Wreorder") -## Address Sanitize Flag -if(${ADDRESS_SANITIZER}) - if(BUILD_SHARED_LIBS}) - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -shared-libsan") - else() - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libsan") - endif() -else() - ## Security breach mitigation flags - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DFORTIFY_SOURCE=2 -fstack-protector-all -Wcast-align") - ## More security breach mitigation flags - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wl,-z,noexecstack -Wl,-znoexecheap -Wl,-z,relro") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wtrampolines -Wl,-z,now -fPIE") -endif() - set(COMMON_SRC_DIR "${PROJECT_SOURCE_DIR}/src") set(ROCM_SRC_DIR "${PROJECT_SOURCE_DIR}/rocm_smi/src") set(AMDSMI_SRC_DIR "${PROJECT_SOURCE_DIR}/src/amd_smi") diff --git a/cmake_modules/help_package.cmake b/cmake_modules/help_package.cmake index c862678f75..e1d74f4114 100644 --- a/cmake_modules/help_package.cmake +++ b/cmake_modules/help_package.cmake @@ -33,17 +33,36 @@ function(generic_package) endif() # Add address sanitizer + # derived from: + # https://github.com/RadeonOpenCompute/ROCm-OpenCL-Runtime/blob/e176056061bf11fdd98b58dd57deb4ac5625844d/amdocl/CMakeLists.txt#L27 if(${ADDRESS_SANITIZER}) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -g" PARENT_SCOPE) - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address" PARENT_SCOPE) - set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -shared-libasan" PARENT_SCOPE) - message(STATUS "ADDRESS_SANITIZE: CMAKE_CXX_FLAGS=: ${CMAKE_CXX_FLAGS}") - message(STATUS "ADDRESS_SANITIZE: CMAKE_EXE_LINKER_FLAGS=: ${CMAKE_EXE_LINKER_FLAGS}") + set(ASAN_COMPILER_FLAGS "-fno-omit-frame-pointer -fsanitize=address") + 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") + 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) + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${ASAN_LINKER_FLAGS}" PARENT_SCOPE) + else() + ## Security breach mitigation flags + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DFORTIFY_SOURCE=2 -fstack-protector-all -Wcast-align" PARENT_SCOPE) + ## More security breach mitigation flags + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wl,-z,noexecstack -Wl,-znoexecheap -Wl,-z,relro" PARENT_SCOPE) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wtrampolines -Wl,-z,now -fPIE" PARENT_SCOPE) endif() # Clang does not set the build-id + # similar to if(NOT CMAKE_COMPILER_IS_GNUCC) if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--build-id=sha1" PARENT_SCOPE) + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--build-id=sha1" PARENT_SCOPE) + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--build-id=sha1" PARENT_SCOPE) endif() # configure packaging diff --git a/tests/amd_smi_test/CMakeLists.txt b/tests/amd_smi_test/CMakeLists.txt index c5be99ae21..090ce93d7d 100644 --- a/tests/amd_smi_test/CMakeLists.txt +++ b/tests/amd_smi_test/CMakeLists.txt @@ -2,7 +2,7 @@ option(INSTALL_GTEST "Install GTest (only useful if GTest is not already installed)" OFF) # Help tests find libraries at runtime -set(CMAKE_EXE_LINKER_FLAGS "-Wl,--enable-new-dtags") +set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--enable-new-dtags") set(CMAKE_INSTALL_RPATH "\$ORIGIN:\$ORIGIN/../../../lib" CACHE STRING "RUNPATH for tests. Helps find libgtest.so and libamd_smi.so") From 79bd9c1d5f6d08199a897aae959f149fa6bd2242 Mon Sep 17 00:00:00 2001 From: "Bill(Shuzhou) Liu" Date: Wed, 4 Jan 2023 12:30:11 -0600 Subject: [PATCH 2/2] change sensor_type in amdsmi_dev_get_temp_metric() to enum The sensor_type in amdsmi_dev_get_temp_metric() will be changed to amdsmi_temperature_type_t Change-Id: I72a7f271b0a55a025acc2ca523062a3d51cc036d --- README.md | 2 +- example/amd_smi_drm_example.cc | 2 +- example/amd_smi_nodrm_example.cc | 2 +- include/amd_smi/amdsmi.h | 3 ++- py-interface/amdsmi_wrapper.py | 2 +- src/amd_smi/amd_smi.cc | 5 +++-- tests/amd_smi_test/functional/mutual_exclusion.cc | 2 +- tests/amd_smi_test/functional/temp_read.cc | 6 +++--- 8 files changed, 13 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 6cbc6dbd56..a8ca2343f1 100755 --- a/README.md +++ b/README.md @@ -133,7 +133,7 @@ int main() { // Get temperature int64_t val_i64 = 0; - ret = amdsmi_dev_get_temp_metric(device_handles[j], 0, + ret = amdsmi_dev_get_temp_metric(device_handles[j], TEMPERATURE_TYPE_EDGE, AMDSMI_TEMP_CURRENT, &val_i64); std::cout << "\t\tTemperature: " << val_i64/1000 << "C" << std::endl; } diff --git a/example/amd_smi_drm_example.cc b/example/amd_smi_drm_example.cc index 3f2ae4803e..e438124879 100644 --- a/example/amd_smi_drm_example.cc +++ b/example/amd_smi_drm_example.cc @@ -649,7 +649,7 @@ int main() { // Get temperature int64_t val_i64 = 0; - ret = amdsmi_dev_get_temp_metric(device_handles[j], 0, + ret = amdsmi_dev_get_temp_metric(device_handles[j], TEMPERATURE_TYPE_EDGE, AMDSMI_TEMP_CURRENT, &val_i64); CHK_AMDSMI_RET(ret) printf(" Output of amdsmi_dev_get_temp_metric:\n"); diff --git a/example/amd_smi_nodrm_example.cc b/example/amd_smi_nodrm_example.cc index 0220b0e5cc..f747a3c18f 100644 --- a/example/amd_smi_nodrm_example.cc +++ b/example/amd_smi_nodrm_example.cc @@ -299,7 +299,7 @@ int main() { // Get temperature int64_t val_i64 = 0; - ret = amdsmi_dev_get_temp_metric(device_handles[j], 0, + ret = amdsmi_dev_get_temp_metric(device_handles[j], TEMPERATURE_TYPE_EDGE, AMDSMI_TEMP_CURRENT, &val_i64); CHK_AMDSMI_RET(ret) printf(" Output of amdsmi_dev_get_temp_metric:\n"); diff --git a/include/amd_smi/amdsmi.h b/include/amd_smi/amdsmi.h index b1450a856f..7c18a821a6 100644 --- a/include/amd_smi/amdsmi.h +++ b/include/amd_smi/amdsmi.h @@ -2029,7 +2029,8 @@ amdsmi_status_t amdsmi_dev_get_fan_speed_max(amdsmi_device_handle device_handle, * * @return ::amdsmi_status_t | ::AMDSMI_STATUS_SUCCESS on success, non-zero on fail */ -amdsmi_status_t amdsmi_dev_get_temp_metric(amdsmi_device_handle device_handle, uint32_t sensor_type, +amdsmi_status_t amdsmi_dev_get_temp_metric(amdsmi_device_handle device_handle, + amdsmi_temperature_type_t sensor_type, amdsmi_temperature_metric_t metric, int64_t *temperature); /** diff --git a/py-interface/amdsmi_wrapper.py b/py-interface/amdsmi_wrapper.py index f4c614fa53..fc17cd3e43 100644 --- a/py-interface/amdsmi_wrapper.py +++ b/py-interface/amdsmi_wrapper.py @@ -1528,7 +1528,7 @@ amdsmi_dev_get_fan_speed_max.restype = amdsmi_status_t amdsmi_dev_get_fan_speed_max.argtypes = [amdsmi_device_handle, uint32_t, ctypes.POINTER(ctypes.c_uint64)] amdsmi_dev_get_temp_metric = _libraries['libamd_smi.so'].amdsmi_dev_get_temp_metric amdsmi_dev_get_temp_metric.restype = amdsmi_status_t -amdsmi_dev_get_temp_metric.argtypes = [amdsmi_device_handle, uint32_t, amdsmi_temperature_metric_t, ctypes.POINTER(ctypes.c_int64)] +amdsmi_dev_get_temp_metric.argtypes = [amdsmi_device_handle, amdsmi_temperature_type_t, amdsmi_temperature_metric_t, ctypes.POINTER(ctypes.c_int64)] amdsmi_dev_get_volt_metric = _libraries['libamd_smi.so'].amdsmi_dev_get_volt_metric amdsmi_dev_get_volt_metric.restype = amdsmi_status_t amdsmi_dev_get_volt_metric.argtypes = [amdsmi_device_handle, amdsmi_voltage_type_t, amdsmi_voltage_metric_t, ctypes.POINTER(ctypes.c_int64)] diff --git a/src/amd_smi/amd_smi.cc b/src/amd_smi/amd_smi.cc index ce7d41d7fc..98d455b1a7 100644 --- a/src/amd_smi/amd_smi.cc +++ b/src/amd_smi/amd_smi.cc @@ -317,7 +317,7 @@ amdsmi_status_t amdsmi_get_board_info(amdsmi_device_handle device_handle, amdsmi } amdsmi_status_t amdsmi_dev_get_temp_metric(amdsmi_device_handle device_handle, - uint32_t sensor_type, + amdsmi_temperature_type_t sensor_type, amdsmi_temperature_metric_t metric, int64_t *temperature) { AMDSMI_CHECK_INIT(); @@ -338,7 +338,8 @@ amdsmi_status_t amdsmi_dev_get_temp_metric(amdsmi_device_handle device_handle, return r_status; } - return rsmi_wrapper(rsmi_dev_temp_metric_get, device_handle, sensor_type, + return rsmi_wrapper(rsmi_dev_temp_metric_get, device_handle, + static_cast(sensor_type), static_cast(metric), temperature); } diff --git a/tests/amd_smi_test/functional/mutual_exclusion.cc b/tests/amd_smi_test/functional/mutual_exclusion.cc index 3f90da2fa8..0f5b92f81b 100755 --- a/tests/amd_smi_test/functional/mutual_exclusion.cc +++ b/tests/amd_smi_test/functional/mutual_exclusion.cc @@ -229,7 +229,7 @@ void TestMutualExclusion::Run(void) { CHECK_RET(ret, AMDSMI_STATUS_BUSY); ret = amdsmi_dev_get_fan_speed_max(device_handles_[0], 0, &dmy_ui64); CHECK_RET(ret, AMDSMI_STATUS_BUSY); - ret = amdsmi_dev_get_temp_metric(device_handles_[0], dmy_ui32, AMDSMI_TEMP_CURRENT, &dmy_i64); + ret = amdsmi_dev_get_temp_metric(device_handles_[0], TEMPERATURE_TYPE_EDGE, AMDSMI_TEMP_CURRENT, &dmy_i64); CHECK_RET(ret, AMDSMI_STATUS_BUSY); ret = amdsmi_dev_reset_fan(device_handles_[0], 0); CHECK_RET(ret, AMDSMI_STATUS_BUSY); diff --git a/tests/amd_smi_test/functional/temp_read.cc b/tests/amd_smi_test/functional/temp_read.cc index 93a3035c8f..7ddd429c71 100755 --- a/tests/amd_smi_test/functional/temp_read.cc +++ b/tests/amd_smi_test/functional/temp_read.cc @@ -114,7 +114,7 @@ void TestTempRead::Run(void) { auto print_temp_metric = [&](amdsmi_temperature_metric_t met, std::string label) { - err = amdsmi_dev_get_temp_metric(device_handles_[i], type, met, &val_i64); + err = amdsmi_dev_get_temp_metric(device_handles_[i], static_cast(type), met, &val_i64); if (err != AMDSMI_STATUS_SUCCESS) { if (err == AMDSMI_STATUS_NOT_SUPPORTED) { @@ -124,7 +124,7 @@ void TestTempRead::Run(void) { } // Verify api support checking functionality is working - err = amdsmi_dev_get_temp_metric(device_handles_[i], type, met, nullptr); + err = amdsmi_dev_get_temp_metric(device_handles_[i], static_cast(type), met, nullptr); ASSERT_EQ(err, AMDSMI_STATUS_INVAL); return; } else { @@ -132,7 +132,7 @@ void TestTempRead::Run(void) { } } // Verify api support checking functionality is working - err = amdsmi_dev_get_temp_metric(device_handles_[i], type, met, nullptr); + err = amdsmi_dev_get_temp_metric(device_handles_[i], static_cast(type), met, nullptr); ASSERT_EQ(err, AMDSMI_STATUS_INVAL); IF_VERB(STANDARD) {