Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[c++] Fix sanitizer flags and move from an option to a config type #3150

Merged
merged 10 commits into from
Oct 14, 2024
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. ")
jp-dark marked this conversation as resolved.
Show resolved Hide resolved

# 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}
jp-dark marked this conversation as resolved.
Show resolved Hide resolved
-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}
jp-dark marked this conversation as resolved.
Show resolved Hide resolved
)

# 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
Loading