From a5a0adeca68892ab4f2d992df37a5f8a50a2bd51 Mon Sep 17 00:00:00 2001 From: Eric Hughes Date: Wed, 30 Jun 2021 13:29:12 -0600 Subject: [PATCH 1/2] Rewrite sanitizer invocation. Allow special cases where there's platform variation. --- bootstrap | 2 +- cmake/Modules/Sanitizers.cmake | 137 +++++++++++++++++++++++++++++++++ test/CMakeLists.txt | 7 ++ tiledb/CMakeLists.txt | 29 +++---- 4 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 cmake/Modules/Sanitizers.cmake diff --git a/bootstrap b/bootstrap index ac5dc9a688e..7def05051e0 100755 --- a/bootstrap +++ b/bootstrap @@ -44,7 +44,7 @@ Configuration: --disable-stats disables internal TileDB statistics --disable-avx2 disables use of AVX2 instructions --enable-static-tiledb enables building TileDB as a static library - --enable-sanitizer=SAN enable sanitizer (clang only) + --enable-sanitizer=SAN enable sanitizer (address|memory|leak|thread|undefined) --enable-debug enable debug build --enable-release-symbols enable create symbols for release build diff --git a/cmake/Modules/Sanitizers.cmake b/cmake/Modules/Sanitizers.cmake new file mode 100644 index 00000000000..9faef80e039 --- /dev/null +++ b/cmake/Modules/Sanitizers.cmake @@ -0,0 +1,137 @@ +# +# Sanitizers.cmake +# +# The MIT License +# +# Copyright (c) 2021 TileDB, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. +# +# Sanitizers +# https://github.com/google/sanitizers +# https://github.com/google/sanitizers/wiki/AddressSanitizer +# +# GCC +# all: https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html +# All the sanitizer options are here, together with all the similar options gcc offers. +# +# Clang +# address: https://clang.llvm.org/docs/AddressSanitizer.html +# Recommends linking with `clang++`. No warning if otherwise at this time. +# undefined behavior: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html +# memory: https://clang.llvm.org/docs/MemorySanitizer.html +# +# MSVC +# Only supports the address sanitizer +# https://docs.microsoft.com/en-us/cpp/sanitizers/asan +# https://docs.microsoft.com/en-us/cpp/sanitizers/asan-building +# https://devblogs.microsoft.com/cppblog/addresssanitizer-asan-for-windows-with-msvc/ +# https://devblogs.microsoft.com/cppblog/address-sanitizer-for-msvc-now-generally-available/ + +include_guard() + +# +# target_sanitizer: compile and link a target with a single sanitizer +# +# Syntax mimics that of CMake project commands `target_compile_features` etc. +# Sanitization requires compile and link options to be set, so its name is a +# bit different. +# +# Arguments +# the_target: CMake target +# scope: PUBLIC INTERFACE PRIVATE, passed through for all option-setting +# sanitizer: a single sanitizer use on the target +# +function(target_sanitizer the_target scope sanitizer) + # The basic sanitizer option is standard across compilers + target_compile_options(${the_target} ${scope} -fsanitize=${SANITIZER}) + + # Check that the sanitizer name is well-formed + if (NOT sanitizer MATCHES "^[-A-Za-z]*$") + message(FATAL_ERROR "target_sanitizer: bad sanitizer specification \"${sanitizer}\";" + " permissible characters are only alphabetic and hyphen") + endif() + + # Verify that the sanitizer is one that some compiler supports + string(TOLOWER ${sanitizer} sanitizer) + if (NOT sanitizer MATCHES "^(address|memory|leak|thread|undefined)$") + message(FATAL_ERROR "target_sanitizer: unsupported sanitizer ${sanitizer}") + endif() + + # Validate the scope + if (NOT scope MATCHES "^(PUBLIC|PRIVATE|INTERFACE)$") + message(FATAL_ERROR "target_sanitizer: scope \"${scope}\" is not one of PUBLIC, PRIVATE, or INTERFACE.") + endif() + + # For known compilers, check that the sanitizer is supported. + # If we know it's not supported, we'll fail so that we avoid false confidence. + # If we don't know, we'll warn that it might not work. + if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + if (sanitizer MATCHES "^address$") + # MSVC support for the address sanitizer began with Visual Studio 2019 Version 16.4 + # and was announced as "fully supported" in version 16.9 + if (MSVC_VERSION LESS 1924) + message(FATAL_ERROR "MSVC version ${MSVC_VERSION} too early to support address sanitizer." ) + endif() + if (MSVC_VERSION LESS 1929) + message(WARNING "MSVC version ${MSVC_VERSION} may only partially support address sanitizer." ) + endif() + # Catch has a conflict with ASAN on Windows. Disable the SEH handler in Catch to avoid the conflict. + target_compile_definitions(${the_target} ${scope} CATCH_CONFIG_NO_WINDOWS_SEH) + else() + # MSVC support only the address sanitizer + message(FATAL_ERROR "MSVC only supports sanitizer \"address\"") + endif() + # Certain compile options are incompatible with ASAN + # Microsoft suppresses /INCREMENTAL, but emits a warning, so silence it. + target_link_options(${the_target} ${scope} /INCREMENTAL:NO) + + # May also need to explicitly remove /RTC flags + + elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang") # also matches AppleClang, ARMClang, etc. + # Ordinary gcc behavior. Factor this out into a subroutine when we need more than twice. + target_compile_options(${the_target} ${scope} -g -fno-omit-frame-pointer -fno-optimize-sibling-calls) + + # Clang recommends a linker flag as well as a compiler flag + target_link_options(${the_target} ${scope} -fsanitize=${SANITIZER}) + if (sanitizer MATCHES "^address$") + # There may be problems if clang tries to link the ASAN library statically + target_link_options(${the_target} ${scope} -shared-libasan) + endif() + + elseif (CMAKE_CXX_COMPILER_ID MATCHES "GNU") + # Ordinary gcc behavior + target_compile_options(${the_target} ${scope} -g -fno-omit-frame-pointer -fno-optimize-sibling-calls) + + else() + message(WARN "Compiler \"${CMAKE_CXX_COMPILER_ID}\" not explicitly supported; behaving as if GNU") + endif() +endfunction() + +# +# target_sanitizer_options +# +# There are a number of options for the various sanitizers, notably about error recovery +# and trapping on error. +# +# Limitations: Unimplemented at the present time. For future use. +# +function(target_sanitizer_options the_target sanitizer options) + message(FATAL_ERROR "target_sanitizer_options: not yet implemented") +endfunction() diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index b982e1f2e07..85080f1bd83 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -26,6 +26,7 @@ # THE SOFTWARE. # +include(Sanitizers) find_package(Catch_EP REQUIRED) # These options not exposed in bootstrap script. @@ -236,6 +237,12 @@ add_executable( ${TILEDB_TEST_SOURCES} ) +# Set sanitizer options first, before all other options standard or otherwise +if (SANITIZER) + target_sanitizer(tiledb_unit PUBLIC ${SANITIZER}) + message(STATUS "Enable sanitizer \"${SANITIZER}\" for TileDB library tests.") +endif() + # We want tests to continue as normal even as the API is changing, # so don't warn for deprecations, since they'll be escalated to errors. if (NOT MSVC) diff --git a/tiledb/CMakeLists.txt b/tiledb/CMakeLists.txt index 1a358e3c906..f6497ac46d6 100644 --- a/tiledb/CMakeLists.txt +++ b/tiledb/CMakeLists.txt @@ -26,6 +26,8 @@ # THE SOFTWARE. # +include(Sanitizers) + ############################################################ # CMake policies ############################################################ @@ -243,6 +245,12 @@ add_library(TILEDB_CORE_OBJECTS OBJECT ${TILEDB_EXTERNALS_SOURCES} ) +# Set sanitizer options first, before all other options standard or otherwise +if (SANITIZER) + target_sanitizer(TILEDB_CORE_OBJECTS PUBLIC ${SANITIZER}) + message(STATUS "Enable sanitizer \"${SANITIZER}\" for TileDB library itself.") +endif() + # Compile all core sources with PIC set_property(TARGET TILEDB_CORE_OBJECTS PROPERTY POSITION_INDEPENDENT_CODE ON) @@ -283,22 +291,6 @@ endif() # Compile options/definitions ############################################################ -if (SANITIZER) - if (NOT CMAKE_BUILD_TYPE MATCHES "Debug") - message(FATAL_ERROR "Sanitizers only enabled for Debug build") - endif() - string(TOLOWER ${SANITIZER} SANITIZER) - if (NOT SANITIZER MATCHES "^(address|memory|leak|thread|undefined)$") - message(FATAL_ERROR "Unknown clang sanitizer: ${SANITIZER})") - else() - message(STATUS "The TileDB library is compiled with sanitizer ${SANITIZER} enabled") - endif() - target_compile_options(TILEDB_CORE_OBJECTS - PRIVATE - -g -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=${SANITIZER} - ) -endif() - if (TILEDB_VERBOSE) add_definitions(-DTILEDB_VERBOSE) message(STATUS "The TileDB library is compiled with verbosity.") @@ -423,10 +415,7 @@ endif() # Sanitizer linker flags if (SANITIZER) - target_link_libraries(TILEDB_CORE_OBJECTS_ILIB - INTERFACE - -fsanitize=${SANITIZER} - ) + target_sanitizer(TILEDB_CORE_OBJECTS_ILIB INTERFACE ${SANITIZER}) endif() # Coverage linker flags From 01e4109ff43ad13ece40ee4a2bcb5e11f302bb34 Mon Sep 17 00:00:00 2001 From: Eric Hughes Date: Wed, 30 Jun 2021 16:37:28 -0600 Subject: [PATCH 2/2] Changed visibility of sanitizer for TILEDB_CORE_OBJECTS to match what it had been originally. --- tiledb/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tiledb/CMakeLists.txt b/tiledb/CMakeLists.txt index f6497ac46d6..dc98e89d604 100644 --- a/tiledb/CMakeLists.txt +++ b/tiledb/CMakeLists.txt @@ -247,7 +247,7 @@ add_library(TILEDB_CORE_OBJECTS OBJECT # Set sanitizer options first, before all other options standard or otherwise if (SANITIZER) - target_sanitizer(TILEDB_CORE_OBJECTS PUBLIC ${SANITIZER}) + target_sanitizer(TILEDB_CORE_OBJECTS PRIVATE ${SANITIZER}) message(STATUS "Enable sanitizer \"${SANITIZER}\" for TileDB library itself.") endif()