From fa13331c2f12e7631f84c725470723417c744aa2 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Wed, 2 Jul 2025 09:16:32 +0200 Subject: [PATCH] Use target-based approach for enabling sanitizers The current approach for building with sanitizers is to build all targets in the tree with them, regardless of whether they are from Zeek or from something we vendor. While this seems interesting it also means that everything incurrs potential ASAN overhead, regardless of whether it is actually needed here or not, e.g., in a container on my machine `spicyz` invocations are very slow for some configurations (>30min runtimes for some builtin analyzers), but this seems largely unnecessary since Spicy already does its own sanitizer builds. This patch changes that approach to instead be opt-in. --- CMakeLists.txt | 256 +++++++++++++++++--------------- src/CMakeLists.txt | 1 + src/spicy/spicyz/CMakeLists.txt | 2 + 3 files changed, 137 insertions(+), 122 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 56b8ec4b9e..de4a58078f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -239,6 +239,136 @@ if (ENABLE_CLANG_TIDY) endif () endif () +if (ZEEK_SANITIZERS) + string(REPLACE "," " " _sanitizer_args "${ZEEK_SANITIZERS}") + separate_arguments(_sanitizer_args) + set(ZEEK_SANITIZERS "") + + foreach (_sanitizer ${_sanitizer_args}) + if (ZEEK_SANITIZERS) + set(ZEEK_SANITIZERS "${ZEEK_SANITIZERS},") + endif () + + if (_sanitizer STREQUAL "thread") + set(ZEEK_TSAN true) + endif () + + if (NOT _sanitizer STREQUAL "undefined") + set(ZEEK_SANITIZERS "${ZEEK_SANITIZERS}${_sanitizer}") + continue() + endif () + + if (NOT DEFINED ZEEK_SANITIZER_UB_CHECKS) + if (DEFINED ENV{ZEEK_TAILORED_UB_CHECKS}) + # list(APPEND _check_list "alignment") # TODO: fix associated errors + list(APPEND _check_list "bool") + # list(APPEND _check_list "builtin") # Not implemented in older GCCs + list(APPEND _check_list "bounds") # Covers both array/local bounds + # options below + # list(APPEND _check_list "array-bounds") # Not implemented by GCC + # list(APPEND _check_list "local-bounds") # Not normally part of + # "undefined" + list(APPEND _check_list "enum") + list(APPEND _check_list "float-cast-overflow") + list(APPEND _check_list "float-divide-by-zero") + # list(APPEND _check_list "function") # Not implemented by GCC + # list(APPEND _check_list "implicit-unsigned-integer-truncation") # Not + # truly UB list(APPEND _check_list "implicit-signed-integer-truncation") + # # Not truly UB list(APPEND _check_list "implicit-integer-sign-change") + # # Not truly UB + list(APPEND _check_list "integer-divide-by-zero") + list(APPEND _check_list "nonnull-attribute") + list(APPEND _check_list "null") + # list(APPEND _check_list "nullability-arg") # Not normally part of + # "undefined" list(APPEND _check_list "nullability-assign") # Not + # normally part of "undefined" list(APPEND _check_list + # "nullability-return") # Not normally part of "undefined" list(APPEND + # _check_list "objc-cast") # Not truly UB list(APPEND _check_list + # "pointer-overflow") # Not implemented in older GCCs + list(APPEND _check_list "return") + list(APPEND _check_list "returns-nonnull-attribute") + list(APPEND _check_list "shift") + # list(APPEND _check_list "unsigned-shift-base") # Not implemented by + # GCC + list(APPEND _check_list "signed-integer-overflow") + list(APPEND _check_list "unreachable") + # list(APPEND _check_list "unsigned-integer-overflow") # Not truly UB + list(APPEND _check_list "vla-bound") + list(APPEND _check_list "vptr") + + # Clang complains if this one is defined and the optimizer is set to + # -O0. We only set that optimization level if NO_OPTIMIZATIONS is + # passed, so disable the option if that's set. + if (NOT DEFINED ENV{NO_OPTIMIZATIONS}) + list(APPEND _check_list "object-size") + endif () + + string(REPLACE ";" "," _ub_checks "${_check_list}") + set(ZEEK_SANITIZER_UB_CHECKS "${_ub_checks}" CACHE INTERNAL "" FORCE) + else () + set(ZEEK_SANITIZER_UB_CHECKS "undefined" CACHE INTERNAL "" FORCE) + endif () + endif () + + set(ZEEK_SANITIZERS "${ZEEK_SANITIZERS}${ZEEK_SANITIZER_UB_CHECKS}") + endforeach () + + set(_sanitizer_flags "-fsanitize=${ZEEK_SANITIZERS}") + + # The linker command used by check_cxx_compiler_flag requires you to also pass + # the sanitizer to it or it fails. The best way to do this is to set + # CMAKE_REQUIRED_LINK_OPTIONS, but save off a copy of it so it can be reset + # back to what it was previously afterwards. + set(_temp_link_options ${CMAKE_REQUIRED_LINK_OPTIONS}) + list(APPEND CMAKE_REQUIRED_LINK_OPTIONS ${_sanitizer_flags}) + include(CheckCXXCompilerFlag) + check_cxx_compiler_flag(${_sanitizer_flags} COMPILER_SUPPORTS_SANITIZERS) + if (NOT COMPILER_SUPPORTS_SANITIZERS) + message(FATAL_ERROR "Invalid sanitizer compiler flags: ${_sanitizer_flags}") + endif () + set(CMAKE_REQUIRED_LINK_OPTIONS ${_temp_link_options}) + + if (ZEEK_SANITIZER_UB_CHECKS) + set(_sanitizer_flags + "${_sanitizer_flags} -fno-sanitize-recover=${ZEEK_SANITIZER_UB_CHECKS}") + endif () + + set(_sanitizer_flags "${_sanitizer_flags} -fno-omit-frame-pointer") + set(_sanitizer_flags "${_sanitizer_flags} -fno-optimize-sibling-calls") + + if (NOT DEFINED ZEEK_SANITIZER_OPTIMIZATIONS) + if (DEFINED ENV{NO_OPTIMIZATIONS}) + # Using -O1 is generally the suggestion to get more reasonable + # performance. The one downside is it that the compiler may optimize out + # code that otherwise generates an error/leak in a -O0 build, but that + # should be rare and users mostly will not be running unoptimized builds + # in production anyway. + set(ZEEK_SANITIZER_OPTIMIZATIONS false CACHE INTERNAL "" FORCE) + else () + set(ZEEK_SANITIZER_OPTIMIZATIONS true CACHE INTERNAL "" FORCE) + endif () + endif () + + if (ZEEK_SANITIZER_OPTIMIZATIONS) + set(_sanitizer_flags "${_sanitizer_flags} -O1") + endif () +endif () + +# Convenience function for enabling sanitizers for a target. +# +# Usage: +# +# zeek_target_enable_sanitizers() +# +function (zeek_target_enable_sanitizers target_name) + string(REPLACE " " ";" _flags "${_sanitizer_flags}") + + foreach (flag ${_flags}) + target_compile_options(${target_name} PRIVATE ${flag}) + target_link_options(${target_name} PRIVATE ${flag}) + endforeach () +endfunction (zeek_target_enable_sanitizers) + # ############################################################################## # Main targets and utilities. @@ -267,6 +397,7 @@ add_custom_target(zeek_autogen_files) # reasons and backwards compatibility). if (ZEEK_STANDALONE) add_executable(zeek_exe) + zeek_target_enable_sanitizers(zeek_exe) target_compile_features(zeek_exe PRIVATE ${ZEEK_CXX_STD}) set_target_properties(zeek_exe PROPERTIES CXX_EXTENSIONS OFF) target_link_libraries(zeek_exe PRIVATE $) @@ -290,6 +421,7 @@ if (ZEEK_STANDALONE) endif () else () add_library(zeek_lib STATIC) + zeek_target_enable_sanitizers(zeek_lib) endif () if (TARGET zeek_lib) @@ -317,6 +449,7 @@ if (ZEEK_ENABLE_FUZZERS) target_compile_features(zeek_fuzzer_shared PRIVATE ${ZEEK_CXX_STD}) set_target_properties(zeek_fuzzer_shared PROPERTIES CXX_EXTENSIONS OFF) target_link_libraries(zeek_fuzzer_shared PUBLIC $) + zeek_target_enable_sanitizers(zeek_fuzzer_shared) # Tell zeek_target_link_libraries to add library dependencies as PUBLIC. set(zeek_fuzzer_shared_access PUBLIC) endif () @@ -433,6 +566,7 @@ function (zeek_add_subdir_library name) add_dependencies(${target_name} zeek_autogen_files) target_link_libraries(${target_name} PRIVATE $) target_compile_options(${target_name} PRIVATE ${WERROR_FLAG}) + zeek_target_enable_sanitizers(${target_name}) # Take care of compiling BIFs. if (FN_ARGS_BIFS) @@ -700,128 +834,6 @@ if (NOT BINARY_PACKAGING_MODE) _make_install_dir_symlink("${CMAKE_INSTALL_PREFIX}/lib/bro" "${CMAKE_INSTALL_FULL_LIBDIR}/zeek") endif () -if (ZEEK_SANITIZERS) - string(REPLACE "," " " _sanitizer_args "${ZEEK_SANITIZERS}") - separate_arguments(_sanitizer_args) - set(ZEEK_SANITIZERS "") - - foreach (_sanitizer ${_sanitizer_args}) - if (ZEEK_SANITIZERS) - set(ZEEK_SANITIZERS "${ZEEK_SANITIZERS},") - endif () - - if (_sanitizer STREQUAL "thread") - set(ZEEK_TSAN true) - endif () - - if (NOT _sanitizer STREQUAL "undefined") - set(ZEEK_SANITIZERS "${ZEEK_SANITIZERS}${_sanitizer}") - continue() - endif () - - if (NOT DEFINED ZEEK_SANITIZER_UB_CHECKS) - if (DEFINED ENV{ZEEK_TAILORED_UB_CHECKS}) - # list(APPEND _check_list "alignment") # TODO: fix associated errors - list(APPEND _check_list "bool") - # list(APPEND _check_list "builtin") # Not implemented in older GCCs - list(APPEND _check_list "bounds") # Covers both array/local bounds - # options below - # list(APPEND _check_list "array-bounds") # Not implemented by GCC - # list(APPEND _check_list "local-bounds") # Not normally part of - # "undefined" - list(APPEND _check_list "enum") - list(APPEND _check_list "float-cast-overflow") - list(APPEND _check_list "float-divide-by-zero") - # list(APPEND _check_list "function") # Not implemented by GCC - # list(APPEND _check_list "implicit-unsigned-integer-truncation") # Not - # truly UB list(APPEND _check_list "implicit-signed-integer-truncation") - # # Not truly UB list(APPEND _check_list "implicit-integer-sign-change") - # # Not truly UB - list(APPEND _check_list "integer-divide-by-zero") - list(APPEND _check_list "nonnull-attribute") - list(APPEND _check_list "null") - # list(APPEND _check_list "nullability-arg") # Not normally part of - # "undefined" list(APPEND _check_list "nullability-assign") # Not - # normally part of "undefined" list(APPEND _check_list - # "nullability-return") # Not normally part of "undefined" list(APPEND - # _check_list "objc-cast") # Not truly UB list(APPEND _check_list - # "pointer-overflow") # Not implemented in older GCCs - list(APPEND _check_list "return") - list(APPEND _check_list "returns-nonnull-attribute") - list(APPEND _check_list "shift") - # list(APPEND _check_list "unsigned-shift-base") # Not implemented by - # GCC - list(APPEND _check_list "signed-integer-overflow") - list(APPEND _check_list "unreachable") - # list(APPEND _check_list "unsigned-integer-overflow") # Not truly UB - list(APPEND _check_list "vla-bound") - list(APPEND _check_list "vptr") - - # Clang complains if this one is defined and the optimizer is set to - # -O0. We only set that optimization level if NO_OPTIMIZATIONS is - # passed, so disable the option if that's set. - if (NOT DEFINED ENV{NO_OPTIMIZATIONS}) - list(APPEND _check_list "object-size") - endif () - - string(REPLACE ";" "," _ub_checks "${_check_list}") - set(ZEEK_SANITIZER_UB_CHECKS "${_ub_checks}" CACHE INTERNAL "" FORCE) - else () - set(ZEEK_SANITIZER_UB_CHECKS "undefined" CACHE INTERNAL "" FORCE) - endif () - endif () - - set(ZEEK_SANITIZERS "${ZEEK_SANITIZERS}${ZEEK_SANITIZER_UB_CHECKS}") - endforeach () - - set(_sanitizer_flags "-fsanitize=${ZEEK_SANITIZERS}") - - # The linker command used by check_cxx_compiler_flag requires you to also pass - # the sanitizer to it or it fails. The best way to do this is to set - # CMAKE_REQUIRED_LINK_OPTIONS, but save off a copy of it so it can be reset - # back to what it was previously afterwards. - set(_temp_link_options ${CMAKE_REQUIRED_LINK_OPTIONS}) - list(APPEND CMAKE_REQUIRED_LINK_OPTIONS ${_sanitizer_flags}) - include(CheckCXXCompilerFlag) - check_cxx_compiler_flag(${_sanitizer_flags} COMPILER_SUPPORTS_SANITIZERS) - if (NOT COMPILER_SUPPORTS_SANITIZERS) - message(FATAL_ERROR "Invalid sanitizer compiler flags: ${_sanitizer_flags}") - endif () - set(CMAKE_REQUIRED_LINK_OPTIONS ${_temp_link_options}) - - if (ZEEK_SANITIZER_UB_CHECKS) - set(_sanitizer_flags - "${_sanitizer_flags} -fno-sanitize-recover=${ZEEK_SANITIZER_UB_CHECKS}") - endif () - - set(_sanitizer_flags "${_sanitizer_flags} -fno-omit-frame-pointer") - set(_sanitizer_flags "${_sanitizer_flags} -fno-optimize-sibling-calls") - - if (NOT DEFINED ZEEK_SANITIZER_OPTIMIZATIONS) - if (DEFINED ENV{NO_OPTIMIZATIONS}) - # Using -O1 is generally the suggestion to get more reasonable - # performance. The one downside is it that the compiler may optimize out - # code that otherwise generates an error/leak in a -O0 build, but that - # should be rare and users mostly will not be running unoptimized builds - # in production anyway. - set(ZEEK_SANITIZER_OPTIMIZATIONS false CACHE INTERNAL "" FORCE) - else () - set(ZEEK_SANITIZER_OPTIMIZATIONS true CACHE INTERNAL "" FORCE) - endif () - endif () - - if (ZEEK_SANITIZER_OPTIMIZATIONS) - set(_sanitizer_flags "${_sanitizer_flags} -O1") - endif () - - # Technically, the we also need to use the compiler to drive linking and give - # the sanitizer flags there, too. However, CMake, by default, uses the - # compiler for linking and so the automatically flags get used. See - # https://cmake.org/pipermail/cmake/2014-August/058268.html - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${_sanitizer_flags}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${_sanitizer_flags}") -endif () - # ############################################################################## # Dependency Configuration diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a72e084502..5cacfc6608 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -565,6 +565,7 @@ target_link_libraries(zeek_objs PRIVATE $) target_compile_definitions(zeek_objs PRIVATE ZEEK_CONFIG_SKIP_VERSION_H) add_dependencies(zeek_objs zeek_autogen_files) zeek_target_link_libraries(zeek_objs) +zeek_target_enable_sanitizers(zeek_objs) # Add IWYU and clang-tidy to the target if enabled. zeek_target_add_linters(zeek_objs) diff --git a/src/spicy/spicyz/CMakeLists.txt b/src/spicy/spicyz/CMakeLists.txt index e8844c1ad8..6cbda5f3b5 100644 --- a/src/spicy/spicyz/CMakeLists.txt +++ b/src/spicy/spicyz/CMakeLists.txt @@ -7,6 +7,8 @@ add_executable(spicyz driver.cc glue-compiler.cc main.cc zeek-version.cc) target_compile_options(spicyz PRIVATE "-Wall") target_compile_features(spicyz PRIVATE "${ZEEK_CXX_STD}") set_target_properties(spicyz PROPERTIES CXX_EXTENSIONS OFF) +# NOTE: We do not add `zeek_target_enable_sanitizers` for `spicyz` since this might make `spicyz` very slow. +# zeek_target_enable_sanitizers(spicyz) target_include_directories(spicyz PRIVATE ${CMAKE_CURRENT_BINARY_DIR})