Skip to content

Commit

Permalink
[c++] Fix sanitizer flags and move from an option to a config type (#…
Browse files Browse the repository at this point in the history
…3150)

The builds using the `SANITIZER` option were missing the linker flags necessary to build. This PR adds those back in and moves enabling sanitizers to a config option. The new build types are enabled:

* `ASAN`: address sanitizer 
* `TSAN`: thread sanitizer
* `LSAN`: leak sanitizer
* `MSAN`: memory sanitizer
* `UBSAN`: undefined-behavior sanitizer
---------

Co-authored-by: Theodore Tsirpanis <theodore.tsirpanis@tiledb.com>
  • Loading branch information
jp-dark and teo-tsirpanis authored Oct 14, 2024
1 parent 7381508 commit 2942ea2
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 65 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/python-ci-packaging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ jobs:
-D TILEDB_REMOVE_DEPRECATIONS=ON \
-D TILEDBSOMA_ENABLE_WERROR=ON \
-D FORCE_BUILD_TILEDB=OFF
cmake --build build-libtiledbsoma -j $(nproc)
cmake --build build-libtiledbsoma -j $(nproc) --verbose
cmake --build build-libtiledbsoma --target install-libtiledbsoma
ls external/lib/
echo "TILEDBSOMA_PATH=$(pwd)/external" >> $GITHUB_ENV
Expand Down
71 changes: 39 additions & 32 deletions libtiledbsoma/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ option(TILEDB_VERBOSE "If true, sets default logging to verbose for TileDB" OFF)
option(OVERRIDE_INSTALL_PREFIX "Ignores the setting of CMAKE_INSTALL_PREFIX and sets a default prefix" ON)
option(ENABLE_ARROW_EXPORT "Installs an extra header for exporting in-memory results with Apache Arrow" ON)
option(TILEDB_LOG_OUTPUT_ON_FAILURE "If true, print error logs if dependency sub-project build fails" ON)
option(TILEDB_SANITIZER "Sanitizer to use in TILEDB. ")

# Enable compiler cache to speed up recompilation
find_program(CCACHE_FOUND ccache)
Expand All @@ -88,9 +89,24 @@ set(CMAKE_CXX_EXTENSIONS OFF) # Don't use GNU extensions
# Build with fPIC
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

# Release builds by default.
if(NOT CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE RELEASE)
# Set default builds/configuration to be Release.
get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
if (is_multi_config)
set(CMAKE_CONFIGURATION_TYPES
"Release;Debug;RelWithDebInfo;ASAN;TSAN;LSAN;UBSAN;MSAN"
CACHE
STRING
"Semi-colon separate list of build types for multi-configuration generators."
)
set(CMAKE_MAP_IMPORTED_CONFIG_ASAN Debug)
set(CMAKE_MAP_IMPORTED_CONFIG_TSAN Debug)
set(CMAKE_MAP_IMPORTED_CONFIG_LSAN Debug)
set(CMAKE_MAP_IMPORTED_CONFIG_UBSAN Debug)
set(CMAKE_MAP_IMPORTED_CONFIG_MSAN Debug)
else()
set(CMAKE_BUILD_TYPE
"Release" CACHE STRING "Build type for single-configuration generators."
)
endif()

# Use @rpath on macOS for building shared libraries.
Expand Down Expand Up @@ -214,54 +230,45 @@ else()
# build generators that set the config type at build time.
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<STREQUAL:${CMAKE_BUILD_TYPE},Debug>: -DDEBUG -O0 -g3 -ggdb3 -gdwarf-3>
$<$<CONFIG:Debug>: -DDEBUG -O0 -g3 -ggdb3 -gdwarf-3>
)
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<STREQUAL:${CMAKE_BUILD_TYPE},Release>: -DNDEBUG -O3>
$<$<CONFIG:Release>: -DNDEBUG -O3>
)
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<STREQUAL:${CMAKE_BUILD_TYPE},RelWithDebInfo>: -DNDEBUG -O3 -g3 -ggdb3 -gdwarf-3>
$<$<CONFIG:RelWithDebInfo>: -DNDEBUG -O3 -g3 -ggdb3 -gdwarf-3>
)
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<STREQUAL:${CMAKE_BUILD_TYPE},>: -DDEBUG -g3 -gdwarf-3 --coverage>
$<$<CONFIG:ASAN,TSAN,LSAN,UBSAN,MSAN>: -DDEBUG -O1 -g -fno-omit-frame-pointer -fno-optimize-sibling-calls>
)

# Use -Wno-literal-suffix on Linux for C++ libtiledbsoma target.
if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
list(APPEND TILEDBSOMA_COMPILE_OPTIONS -Wno-literal-suffix)
endif()
endif()

# ###########################################################
# Compile options/definitions
# ###########################################################
if(SANITIZER)
string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER)

if(NOT CMAKE_BUILD_TYPE_LOWER MATCHES "debug")
message(FATAL_ERROR "Sanitizers only enabled for Debug build")
endif()
# TODO: There is a bug in CMake 3.22.1 but not 3.27.6 where nested generators
# are not fully evaluated in the `target_link_library` commands. The GENEX_EVAL
# commands are not multi-configuration friendly and should be removed after we
# update CMake.
# See https://www.reddit.com/r/cmake/comments/17d70h6/why_arent_generator_expressions_evaluated_for/
# for this placeholder solution.
set(TILEDBSOMA_SANITIZER_FLAG "")
list(APPEND TILEDBSOMA_SANITIZER_FLAG "\$<GENEX_EVAL:$<$<CONFIG:ASAN>:-fsanitize=address>>")
list(APPEND TILEDBSOMA_SANITIZER_FLAG "\$<GENEX_EVAL:$<$<CONFIG:LSAN>:-fsanitize=leak>>")
list(APPEND TILEDBSOMA_SANITIZER_FLAG "\$<GENEX_EVAL:$<$<CONFIG:TSAN>:-fsanitize=thread>>")
list(APPEND TILEDBSOMA_SANITIZER_FLAG "\$<GENEX_EVAL:$<$<CONFIG:UBSAN>:-fsanitize=undefined>>")
list(APPEND TILEDBSOMA_SANITIZER_FLAG "\$<GENEX_EVAL:$<$<CONFIG:MSAN>:-fsanitize=memory>>")

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-SOMA library is compiled with sanitizer ${SANITIZER} enabled")
# Compiler specific additions:
if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
# Use -Wno-literal-suffix on Linux for C++ libtiledbsoma target.
list(APPEND TILEDBSOMA_COMPILE_OPTIONS -Wno-literal-suffix)
endif()

set(TILEDBSOMA_SANITIZER_OPTIONS
-g -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=${SANITIZER}
)
else()
set(TILEDBSOMA_SANITIZER_OPTIONS "")
endif()



# Definitions for all targets
add_definitions(-D_FILE_OFFSET_BITS=64)

Expand Down
2 changes: 1 addition & 1 deletion libtiledbsoma/cmake/Superbuild.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ set(INHERITED_CMAKE_ARGS
-DTILEDB_SERIALIZATION=${TILEDB_SERIALIZATION}
-DTILEDB_WERROR=${TILEDB_WERROR}
-DTILEDB_VERBOSE=${TILEDB_VERBOSE}
-DTILEDB_SANITIZER=${TILEDB_SANITIZER}
-DTileDB_DIR=${TileDB_DIR}
-DSANITIZER=${SANITIZER}
-DENABLE_ARROW_EXPORT=${ENABLE_ARROW_EXPORT}
-DOVERRIDE_INSTALL_PREFIX=${OVERRIDE_INSTALL_PREFIX}
-DTILEDBSOMA_BUILD_STATIC=${TILEDBSOMA_BUILD_STATIC}
Expand Down
67 changes: 36 additions & 31 deletions libtiledbsoma/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@ set_source_files_properties(
add_library(TILEDBSOMA_NANOARROW_OBJECT OBJECT
${CMAKE_CURRENT_SOURCE_DIR}/external/src/nanoarrow/nanoarrow.c
)
target_link_options(TILEDBSOMA_NANOARROW_OBJECT
PRIVATE
${TILEDBSOMA_SANITIZER_FLAG}
)
target_compile_options(TILEDBSOMA_NANOARROW_OBJECT
PRIVATE
${TILEDBSOMA_COMPILE_OPTIONS}
${TILEDBSOMA_SANITIZER_OPTIONS}
${TILEDBSOMA_SANITIZER_FLAG}
)
target_include_directories(TILEDBSOMA_NANOARROW_OBJECT
PUBLIC
Expand Down Expand Up @@ -52,15 +56,20 @@ add_library(TILEDB_SOMA_OBJECTS OBJECT

target_compile_definitions(TILEDB_SOMA_OBJECTS
PRIVATE
-DTILEDB_NO_API_DEPRECATION_WARNINGS
-DTILEDB_NO_API_DEPRECATION_WARNINGS
)

target_compile_options(TILEDB_SOMA_OBJECTS
PRIVATE
${TILEDBSOMA_COMPILE_OPTIONS}
${TILEDBSOMA_WERROR_OPTION}
${TILEDBSOMA_SANITIZER_OPTIONS}
${TILEDBSOMA_COMPILE_OPTIONS}
${TILEDBSOMA_WERROR_OPTION}
${TILEDBSOMA_SANITIZER_FLAG}
)
target_link_options(TILEDB_SOMA_OBJECTS
PRIVATE
${TILEDBSOMA_SANITIZER_FLAG}
)


set_property(TARGET TILEDB_SOMA_OBJECTS PROPERTY POSITION_INDEPENDENT_CODE ON)
target_include_directories(TILEDB_SOMA_OBJECTS
Expand Down Expand Up @@ -91,6 +100,11 @@ add_library(TILEDB_SOMA_GEOMETRY_OBJECTS OBJECT
${CMAKE_CURRENT_SOURCE_DIR}/geometry/operators/io/write.cc
)

target_link_options(TILEDB_SOMA_GEOMETRY_OBJECTS
PRIVATE
${TILEDBSOMA_SANITIZER_FLAG}
)

target_compile_definitions(TILEDB_SOMA_GEOMETRY_OBJECTS
PRIVATE
-DTILEDB_NO_API_DEPRECATION_WARNINGS
Expand Down Expand Up @@ -141,12 +155,9 @@ if(TILEDBSOMA_BUILD_STATIC)
OUTPUT_NAME "tiledbsoma"
)
endif()
if(SANITIZER)
target_link_libraries(tiledbsoma_static
INTERFACE
-fsanitize=${SANITIZER}
)
endif()
target_link_libraries(tiledbsoma_static
PRIVATE
)
else()
add_library(tiledbsoma SHARED
$<TARGET_OBJECTS:TILEDB_SOMA_OBJECTS>
Expand All @@ -156,15 +167,13 @@ else()
list(APPEND TILEDBSOMA_INSTALL_TARGETS tiledbsoma)
target_link_libraries(tiledbsoma
PUBLIC
TileDB::tiledb_shared
spdlog::spdlog
TileDB::tiledb_shared
spdlog::spdlog
)
target_link_options(tiledbsoma
PRIVATE
${TILEDBSOMA_SANITIZER_FLAG}
)
if(SANITIZER)
target_link_libraries(tiledbsoma
INTERFACE
-fsanitize=${SANITIZER}
)
endif()
endif()

# Install header files
Expand Down Expand Up @@ -297,20 +306,16 @@ if(TILEDBSOMA_BUILD_CLI)

target_link_libraries(tiledbsoma-cli
PUBLIC

# CLI11::CLI11
# spdlog::spdlog
tiledbsoma
TileDB::tiledb_shared
# CLI11::CLI11
# spdlog::spdlog
tiledbsoma
TileDB::tiledb_shared
)
target_link_options(tiledbsoma-cli
PRIVATE
${TILEDBSOMA_SANITIZER_FLAG}
)

# Sanitizer linker flags
if(SANITIZER)
target_link_libraries(tiledbsoma-cli
INTERFACE
-fsanitize=${SANITIZER}
)
endif()

if(NOT APPLE AND NOT WIN32)
target_link_libraries(tiledbsoma-cli PRIVATE pthread)
Expand Down
10 changes: 10 additions & 0 deletions libtiledbsoma/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ target_link_libraries(unit_soma
Catch2::Catch2WithMain
TileDB::tiledb_shared
)
target_link_options(unit_soma
PRIVATE
${TILEDBSOMA_SANITIZER_FLAG}
)

target_include_directories(unit_soma
PRIVATE
Expand All @@ -59,6 +63,7 @@ target_compile_definitions(unit_soma
target_compile_options(unit_soma
PRIVATE
${TILEDBSOMA_COMPILE_OPTIONS}
${TILEDBSOMA_SANITIZER_FLAG}
${TILEDBSOMA_WERROR_OPTION}
)

Expand Down Expand Up @@ -105,6 +110,11 @@ target_link_libraries(unit_geometry
TileDB::tiledb_shared
)

target_link_options(unit_geometry
PRIVATE
${TILEDBSOMA_SANITIZER_FLAG}
)

target_include_directories(unit_geometry
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/../src
Expand Down

0 comments on commit 2942ea2

Please sign in to comment.