From 3cd88eecd8abb693b65a11edd73efa81b4c47708 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 9 Nov 2026 14:54:08 -0600 Subject: [PATCH] Apply sanitizers to the library and add a UBSan CI job The ENABLE_SANITIZER_* options only instrumented the test executables: libclass_loader and the test plugins were built without -fsanitize, so AddressSanitizer and ThreadSanitizer couldn't see memory errors or races inside the library itself. Set the sanitizer options on the top-level directory before any target is defined, so the library, test plugins and tests are all instrumented. Directory options aren't exported, so the installed library's consumers are unaffected. UndefinedBehaviorSanitizer is made fatal so it fails the test instead of only printing a diagnostic, and gets its own CI job. enable_sanitizers() is kept for compatibility, since the file is installed. Signed-off-by: Michael Carroll --- .github/workflows/ci.yaml | 2 ++ CMakeLists.txt | 4 +++ cmake/sanitizers.cmake | 69 ++++++++++++++++++++++++++------------- test/CMakeLists.txt | 9 ++--- 4 files changed, 55 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9bb14cd..28a90d2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -20,6 +20,8 @@ jobs: NAME: Address Sanitizer - TARGET_CMAKE_ARGS: -DENABLE_SANITIZER_THREAD=ON -DCMAKE_BUILD_TYPE=Debug NAME: Thread Sanitizer + - TARGET_CMAKE_ARGS: -DENABLE_SANITIZER_UNDEFINED_BEHAVIOR=ON -DCMAKE_BUILD_TYPE=Debug + NAME: Undefined Behavior Sanitizer env: ROS_DISTRO: rolling runs-on: ubuntu-latest diff --git a/CMakeLists.txt b/CMakeLists.txt index 67df721..78ae257 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,6 +9,10 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-Wall -Wextra -Wpedantic) endif() +# ENABLE_SANITIZER_* options: instrument the library, test plugins and tests alike. +include(cmake/sanitizers.cmake) +class_loader_enable_sanitizers() + set(CLASS_LOADER_IGNORE_AMENT FALSE CACHE BOOL "Do not use ament when building this package.") if(NOT CLASS_LOADER_IGNORE_AMENT) diff --git a/cmake/sanitizers.cmake b/cmake/sanitizers.cmake index b264fbe..4e94447 100644 --- a/cmake/sanitizers.cmake +++ b/cmake/sanitizers.cmake @@ -1,15 +1,9 @@ -function(enable_sanitizers project_name) +# Sets LIST_OF_SANITIZERS in the caller's scope to the comma-separated sanitizers selected by the +# ENABLE_SANITIZER_* options (empty if none, or if the compiler isn't GCC or Clang). +function(class_loader_get_sanitizers) + set(SANITIZERS "") if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES ".*Clang") - option(ENABLE_COVERAGE "Enable coverage reporting for gcc/clang" OFF) - - if(ENABLE_COVERAGE) - target_compile_options(${project_name} INTERFACE --coverage -O0 -g -fno-omit-frame-pointer) - target_link_libraries(${project_name} INTERFACE --coverage) - endif() - - set(SANITIZERS "") - option(ENABLE_SANITIZER_ADDRESS "Enable address sanitizer" OFF) if(ENABLE_SANITIZER_ADDRESS) list(APPEND SANITIZERS "address") @@ -46,23 +40,54 @@ function(enable_sanitizers project_name) list(APPEND SANITIZERS "memory") endif() endif() + endif() - list( - JOIN - SANITIZERS - "," - LIST_OF_SANITIZERS) + list(JOIN SANITIZERS "," LIST_OF_SANITIZERS) + set(LIST_OF_SANITIZERS "${LIST_OF_SANITIZERS}" PARENT_SCOPE) +endfunction() +# Sets the compile and link options for the selected sanitizers (and ENABLE_COVERAGE) on the +# current directory, so they apply to every target defined after this call, here and in +# subdirectories. Call it before defining any targets, so the library is instrumented as well as +# the tests and test plugins. Directory options aren't exported, so consumers of the installed +# library are unaffected. +function(class_loader_enable_sanitizers) + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES ".*Clang") + option(ENABLE_COVERAGE "Enable coverage reporting for gcc/clang" OFF) + if(ENABLE_COVERAGE) + add_compile_options(--coverage -O0 -g -fno-omit-frame-pointer) + add_link_options(--coverage) + endif() endif() - if(LIST_OF_SANITIZERS) - if(NOT - "${LIST_OF_SANITIZERS}" - STREQUAL - "") - target_compile_options(${project_name} INTERFACE -fsanitize=${LIST_OF_SANITIZERS}) - target_link_options(${project_name} INTERFACE -fsanitize=${LIST_OF_SANITIZERS}) + class_loader_get_sanitizers() + if(NOT LIST_OF_SANITIZERS STREQUAL "") + message(STATUS "class_loader: building with -fsanitize=${LIST_OF_SANITIZERS}") + set(flags -fsanitize=${LIST_OF_SANITIZERS} -fno-omit-frame-pointer) + if(ENABLE_SANITIZER_UNDEFINED_BEHAVIOR) + # Make undefined behavior fail the test instead of only printing a diagnostic. + list(APPEND flags -fno-sanitize-recover=undefined) endif() + add_compile_options(${flags}) + add_link_options(${flags}) endif() +endfunction() +# Adds the options for the selected sanitizers (and ENABLE_COVERAGE) to the interface of target +# project_name. Kept for compatibility, since this file is installed; this package itself uses +# class_loader_enable_sanitizers(). +function(enable_sanitizers project_name) + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES ".*Clang") + option(ENABLE_COVERAGE "Enable coverage reporting for gcc/clang" OFF) + if(ENABLE_COVERAGE) + target_compile_options(${project_name} INTERFACE --coverage -O0 -g -fno-omit-frame-pointer) + target_link_libraries(${project_name} INTERFACE --coverage) + endif() + endif() + + class_loader_get_sanitizers() + if(NOT LIST_OF_SANITIZERS STREQUAL "") + target_compile_options(${project_name} INTERFACE -fsanitize=${LIST_OF_SANITIZERS}) + target_link_options(${project_name} INTERFACE -fsanitize=${LIST_OF_SANITIZERS}) + endif() endfunction() diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8c72c68..5b2961e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -2,11 +2,6 @@ find_package(ament_cmake_gtest REQUIRED) include("../cmake/class_loader_hide_library_symbols.cmake") -# Link this 'library' to set the compile-time options requested -add_library(project_options INTERFACE) -include(../cmake/sanitizers.cmake) -enable_sanitizers(project_options) - add_library(${PROJECT_NAME}_TestPlugins1 EXCLUDE_FROM_ALL SHARED plugins1.cpp) target_link_libraries(${PROJECT_NAME}_TestPlugins1 ${PROJECT_NAME}) class_loader_hide_library_symbols(${PROJECT_NAME}_TestPlugins1) @@ -46,7 +41,7 @@ if(TARGET ${PROJECT_NAME}_utest) PUBLIC "../include" ${console_bridge_INCLUDE_DIRS}) target_link_libraries(${PROJECT_NAME}_utest) endif() - target_link_libraries(${PROJECT_NAME}_utest ${PROJECT_NAME} project_options) + target_link_libraries(${PROJECT_NAME}_utest ${PROJECT_NAME}) if(NOT WIN32) target_link_libraries(${PROJECT_NAME}_utest pthread) @@ -63,7 +58,7 @@ ament_add_gtest(${PROJECT_NAME}_unmanaged_test unmanaged_test.cpp ) if(TARGET ${PROJECT_NAME}_unmanaged_test) target_include_directories(${PROJECT_NAME}_unmanaged_test PUBLIC "../include") - target_link_libraries(${PROJECT_NAME}_unmanaged_test ${PROJECT_NAME} project_options) + target_link_libraries(${PROJECT_NAME}_unmanaged_test ${PROJECT_NAME}) add_dependencies(${PROJECT_NAME}_unmanaged_test ${PROJECT_NAME}_TestPlugins1 ${PROJECT_NAME}_TestPlugins3)