Skip to content

chore: Do not duplicate sanitizer flags #7058

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.

Sign up for GitHub

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

Merged
bthomee merged 10 commits into from
May 5, 2026
Merged

chore: Do not duplicate sanitizer flags #7058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
288c224
chore: Do not duplicate sanitizer flags
mathbunnyru Jan 26, 2026
3626d0b
Remove old parts of docs
mathbunnyru Apr 30, 2026
62dec4e
Better comments
mathbunnyru Apr 30, 2026
557548f
Merge branch 'develop' into san_flags
mathbunnyru May 1, 2026
bce47d8
Even more polished version
mathbunnyru May 1, 2026
c7aca84
Merge branch 'develop' into san_flags
mathbunnyru May 4, 2026
7b2697d
Merge branch 'develop' into san_flags
mathbunnyru May 5, 2026
9f8ca5b
Add caution
mathbunnyru May 5, 2026
17743e9
Apply suggestions from code review
mathbunnyru May 5, 2026
81c032f
Fix style
mathbunnyru May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/reusable-build-test-config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ jobs:
working-directory: ${{ env.BUILD_DIR }}
env:
BUILD_TYPE: ${{ inputs.build_type }}
SANITIZERS: ${{ inputs.sanitizers }}
CMAKE_ARGS: ${{ inputs.cmake_args }}
run: |
cmake \
Expand Down
6 changes: 3 additions & 3 deletions BUILD.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,16 +530,16 @@ stored inside the build directory, as either of:
## Sanitizers

To build dependencies and xrpld with sanitizer instrumentation, set the
`SANITIZERS` environment variable (only once before running conan and cmake) and use the `sanitizers` profile in conan:
`SANITIZERS` environment variable when running `conan install` and use the `sanitizers` profile:
Copy link
Copy Markdown
Contributor

@pratikmankawde pratikmankawde May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMake files still read SANITIZERS env. variable. So if a dev. sets this flag to one value before running conan install then sets it again before running cmake, there will be a conflict.

Copy link
Copy Markdown
Contributor Author

@mathbunnyru mathbunnyru May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do CMake files still read it?

Copy link
Copy Markdown
Contributor

@pratikmankawde pratikmankawde May 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used in cmake/XrplSanitizers.cmake, but where is SANITIZERS set?

Copy link
Copy Markdown
Contributor Author

@mathbunnyru mathbunnyru May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no reading of env variable SANITIZERS in this CMake file (or I haven't found it).
cmake variable SANITIZERS comes from using tools.cmake.cmaketoolchain:extra_variables conan option

So, unless someone does -DSANITIZERS (and we never had this way of passing sanitizers), so it's highly unlikely to happen, this variable will be the same as conan got.


```bash
export SANITIZERS=address,undefinedbehavior

conan install .. --output-folder . --profile:all sanitizers --build missing --settings build_type=Debug

cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Debug -Dxrpld=ON -Dtests=ON ..
```

You can then build and test as usual, with the generated `xrpld` binary containing the sanitizer instrumentation. When you run it, it will report any sanitizer errors it detects in the console output.

See [Sanitizers docs](./docs/build/sanitizers.md) for more details.

## Options
Expand Down
235 changes: 54 additions & 181 deletions cmake/XrplSanitizers.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,223 +1,96 @@
#[===================================================================[
Configure sanitizers based on environment variables.
Apply sanitizer flags built by the Conan profile.

This module reads the following environment variables:
- SANITIZERS: The sanitizers to enable. Possible values:
- "address"
- "address,undefinedbehavior"
- "thread"
- "thread,undefinedbehavior"
- "undefinedbehavior"
Parsing, validation, and flag construction are performed in conan/profiles/sanitizers.
This module reads the following CMake variables injected by the Conan toolchain via extra_variables:

The compiler type and platform are detected in CompilationEnv.cmake.
The sanitizer compile options are applied to the 'common' interface library
which is linked to all targets in the project.
- SANITIZERS: The active sanitizers (e.g. "address,undefinedbehavior").
- SANITIZERS_COMPILER_FLAGS: Space-separated compiler flags.
- SANITIZERS_LINKER_FLAGS: Space-separated linker flags.

Internal flag variables set by this module:

- SANITIZER_TYPES: List of sanitizer types to enable (e.g., "address",
"thread", "undefined"). And two more flags for undefined behavior sanitizer (e.g., "float-divide-by-zero", "unsigned-integer-overflow").
This list is joined with commas and passed to -fsanitize=<list>.

- SANITIZERS_COMPILE_FLAGS: Compiler flags for sanitizer instrumentation.
Includes:
* -fno-omit-frame-pointer: Preserves frame pointers for stack traces
* -O1: Minimum optimization for reasonable performance
* -fsanitize=<types>: Enables sanitizer instrumentation
* -fsanitize-ignorelist=<path>: (Clang only) Compile-time ignorelist
* -mcmodel=large/medium: (GCC only) Code model for large binaries
* -Wno-stringop-overflow: (GCC only) Suppresses false positive warnings
* -Wno-tsan: (For GCC TSAN combination only) Suppresses atomic_thread_fence warnings

- SANITIZERS_LINK_FLAGS: Linker flags for sanitizer runtime libraries.
Includes:
* -fsanitize=<types>: Links sanitizer runtime libraries
* -mcmodel=large/medium: (GCC only) Matches compile-time code model

- SANITIZERS_RELOCATION_FLAGS: (GCC only, x86_64 only) Code model flags for linking.
Used to handle large instrumented binaries on x86_64:
* -mcmodel=large: For AddressSanitizer (prevents relocation errors)
* -mcmodel=medium: For ThreadSanitizer (large model is incompatible)
On ARM64, these flags are omitted since GCC does not support
-mcmodel=large with -fPIC, and -mcmodel=medium does not exist.
The flags are applied to the 'common' interface library which is linked to all targets in the project.
#]===================================================================]

include_guard(GLOBAL)
include(CompilationEnv)

# Read environment variable
set(SANITIZERS "")
if(DEFINED ENV{SANITIZERS})
set(SANITIZERS "$ENV{SANITIZERS}")
endif()

# Set SANITIZERS_ENABLED flag for use in other modules
if(SANITIZERS MATCHES "address|thread|undefinedbehavior")
set(SANITIZERS_ENABLED TRUE)
else()
if(NOT DEFINED SANITIZERS)
set(SANITIZERS_ENABLED FALSE)
return()
endif()
set(SANITIZERS_ENABLED TRUE)
Comment thread
mathbunnyru marked this conversation as resolved.
Comment thread
bthomee marked this conversation as resolved.

# Sanitizers are not supported on Windows/MSVC
if(is_msvc)
message(
FATAL_ERROR
"Sanitizers are not supported on Windows/MSVC. "
"Please unset the SANITIZERS environment variable."
)
endif()

message(STATUS "Configuring sanitizers: ${SANITIZERS}")

# Parse SANITIZERS value to determine which sanitizers to enable
set(enable_asan FALSE)
set(enable_tsan FALSE)
set(enable_ubsan FALSE)

# Normalize SANITIZERS into a list
set(san_list "${SANITIZERS}")
string(REPLACE "," ";" san_list "${san_list}")
separate_arguments(san_list)

foreach(san IN LISTS san_list)
if(san STREQUAL "address")
set(enable_asan TRUE)
elseif(san STREQUAL "thread")
set(enable_tsan TRUE)
elseif(san STREQUAL "undefinedbehavior")
set(enable_ubsan TRUE)
else()
message(
FATAL_ERROR
"Unsupported sanitizer type: ${san}"
"Supported: address, thread, undefinedbehavior and their combinations."
)
endif()
endforeach()

# Validate sanitizer compatibility
if(enable_asan AND enable_tsan)
message(
FATAL_ERROR
"AddressSanitizer and ThreadSanitizer are incompatible and cannot be enabled simultaneously. "
"Use 'address' or 'thread', optionally with 'undefinedbehavior'."
)
endif()

# Frame pointer is required for meaningful stack traces. Sanitizers recommend minimum of -O1 for reasonable performance
set(SANITIZERS_COMPILE_FLAGS "-fno-omit-frame-pointer" "-O1")

# Build the sanitizer flags list
set(SANITIZER_TYPES)

if(enable_asan)
list(APPEND SANITIZER_TYPES "address")
elseif(enable_tsan)
list(APPEND SANITIZER_TYPES "thread")
endif()
message(STATUS "=== Configuring Sanitizers ===")
message(STATUS " SANITIZERS: ${SANITIZERS}")
message(STATUS " Compile flags: ${SANITIZERS_COMPILER_FLAGS}")
message(STATUS " Link flags: ${SANITIZERS_LINKER_FLAGS}")
Comment thread
mathbunnyru marked this conversation as resolved.
Comment thread
mathbunnyru marked this conversation as resolved.
Comment thread
mathbunnyru marked this conversation as resolved.
Comment thread
mathbunnyru marked this conversation as resolved.

if(enable_ubsan)
# UB sanitizer flags
list(APPEND SANITIZER_TYPES "undefined" "float-divide-by-zero")
if(is_clang)
# Clang supports additional UB checks. More info here
# https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
list(APPEND SANITIZER_TYPES "unsigned-integer-overflow")
endif()
endif()

# Configure code model for GCC on amd64 Use large code model for ASAN to avoid relocation errors Use medium code model
# for TSAN (large is not compatible with TSAN)
set(SANITIZERS_RELOCATION_FLAGS)

# Compiler-specific configuration
# GCC with sanitizers is incompatible with mold, gold, and lld linkers.
Comment thread
mathbunnyru marked this conversation as resolved.
# Namely, the instrumented binary exceeds size limits imposed by these linkers.
if(is_gcc)
# Disable mold, gold and lld linkers for GCC with sanitizers Use default linker (bfd/ld) which is more lenient with
# mixed code models This is needed since the size of instrumented binary exceeds the limits set by mold, lld and
# gold linkers
set(use_mold OFF CACHE BOOL "Use mold linker" FORCE)
set(use_gold OFF CACHE BOOL "Use gold linker" FORCE)
set(use_lld OFF CACHE BOOL "Use lld linker" FORCE)
message(
STATUS
" Disabled mold, gold, and lld linkers for GCC with sanitizers"
)
endif()

# Suppress false positive warnings in GCC with stringop-overflow
list(APPEND SANITIZERS_COMPILE_FLAGS "-Wno-stringop-overflow")

if(is_amd64 AND enable_asan)
message(STATUS " Using large code model (-mcmodel=large)")
list(APPEND SANITIZERS_COMPILE_FLAGS "-mcmodel=large")
list(APPEND SANITIZERS_RELOCATION_FLAGS "-mcmodel=large")
elseif(enable_tsan)
# GCC doesn't support atomic_thread_fence with tsan. Suppress warnings.
list(APPEND SANITIZERS_COMPILE_FLAGS "-Wno-tsan")
if(is_amd64)
message(STATUS " Using medium code model (-mcmodel=medium)")
list(APPEND SANITIZERS_COMPILE_FLAGS "-mcmodel=medium")
list(APPEND SANITIZERS_RELOCATION_FLAGS "-mcmodel=medium")
endif()
endif()
# Flags arrive as space-separated strings; split into CMake lists before use
separate_arguments(
sanitizers_compiler_flags
UNIX_COMMAND
"${SANITIZERS_COMPILER_FLAGS}"
Copy link
Copy Markdown
Contributor

@pratikmankawde pratikmankawde May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to validate SANITIZERS_* flags here? In case user runs conan with one set of flags and cmake (in a later session) with another, but cache is still holding old flags?

Copy link
Copy Markdown
Contributor Author

@mathbunnyru mathbunnyru May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it locally, using the same build dir:

  1. Ran conan with UB sanitizer
  2. Ran cmake
  3. Ran conan with Address sanitizer
  4. Ran cmake

The CMake output was proper both times.

)
separate_arguments(
sanitizers_linker_flags
UNIX_COMMAND
"${SANITIZERS_LINKER_FLAGS}"
)

# Join sanitizer flags with commas for -fsanitize option
list(JOIN SANITIZER_TYPES "," SANITIZER_TYPES_STR)
target_compile_options(
common
INTERFACE
$<$<COMPILE_LANGUAGE:CXX>:${sanitizers_compiler_flags}>
$<$<COMPILE_LANGUAGE:C>:${sanitizers_compiler_flags}>
)
target_link_options(common INTERFACE ${sanitizers_linker_flags})

Comment thread
mathbunnyru marked this conversation as resolved.
# Add sanitizer to compile and link flags
list(APPEND SANITIZERS_COMPILE_FLAGS "-fsanitize=${SANITIZER_TYPES_STR}")
set(SANITIZERS_LINK_FLAGS
"${SANITIZERS_RELOCATION_FLAGS}"
"-fsanitize=${SANITIZER_TYPES_STR}"
)
elseif(is_clang)
# Add ignorelist for Clang (GCC doesn't support this) Use CMAKE_SOURCE_DIR to get the path to the ignorelist
set(IGNORELIST_PATH
# This module appends -fsanitize-ignorelist=<path> for Clang builds.
# The ignorelist path contains CMAKE_SOURCE_DIR, so it must be set here, rather than in the Conan profile.
# GCC does not support -fsanitize-ignorelist.
if(is_clang)
set(ignorelist_path
"${CMAKE_SOURCE_DIR}/sanitizers/suppressions/sanitizer-ignorelist.txt"
)
if(NOT EXISTS "${IGNORELIST_PATH}")
if(NOT EXISTS "${ignorelist_path}")
message(
FATAL_ERROR
"Sanitizer ignorelist not found: ${IGNORELIST_PATH}"
"Sanitizer ignorelist not found: ${ignorelist_path}"
)
endif()

list(
APPEND SANITIZERS_COMPILE_FLAGS
"-fsanitize-ignorelist=${IGNORELIST_PATH}"
target_compile_options(
common
INTERFACE
$<$<COMPILE_LANGUAGE:CXX>:-fsanitize-ignorelist=${ignorelist_path}>
$<$<COMPILE_LANGUAGE:C>:-fsanitize-ignorelist=${ignorelist_path}>
)
message(STATUS " Using sanitizer ignorelist: ${IGNORELIST_PATH}")

# Join sanitizer flags with commas for -fsanitize option
list(JOIN SANITIZER_TYPES "," SANITIZER_TYPES_STR)

# Add sanitizer to compile and link flags
list(APPEND SANITIZERS_COMPILE_FLAGS "-fsanitize=${SANITIZER_TYPES_STR}")
set(SANITIZERS_LINK_FLAGS "-fsanitize=${SANITIZER_TYPES_STR}")
message(STATUS " Ignorelist: ${ignorelist_path}")
endif()

message(STATUS " Compile flags: ${SANITIZERS_COMPILE_FLAGS}")
message(STATUS " Link flags: ${SANITIZERS_LINK_FLAGS}")

# Apply the sanitizer flags to the 'common' interface library This is the same library used by XrplCompiler.cmake
target_compile_options(
common
INTERFACE
$<$<COMPILE_LANGUAGE:CXX>:${SANITIZERS_COMPILE_FLAGS}>
$<$<COMPILE_LANGUAGE:C>:${SANITIZERS_COMPILE_FLAGS}>
)

# Apply linker flags
target_link_options(common INTERFACE ${SANITIZERS_LINK_FLAGS})

# Define SANITIZERS macro for BuildInfo.cpp
set(sanitizers_list)
if(enable_asan)
if(SANITIZERS MATCHES "address")
set(enable_asan ON)
list(APPEND sanitizers_list "ASAN")
endif()
if(enable_tsan)
if(SANITIZERS MATCHES "thread")
set(enable_tsan ON)
list(APPEND sanitizers_list "TSAN")
endif()
if(enable_ubsan)
if(SANITIZERS MATCHES "undefinedbehavior")
set(enable_ubsan ON)
list(APPEND sanitizers_list "UBSAN")
endif()
Comment thread
mathbunnyru marked this conversation as resolved.

Expand Down
2 changes: 1 addition & 1 deletion conan/profiles/ci
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
include(sanitizers)
include(sanitizers)
Loading
Loading