code review changes

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
This commit is contained in:
Pratik Mankawde
2025-12-19 17:31:14 +00:00
parent 6c1ad00e28
commit 738aa9482e
8 changed files with 50 additions and 43 deletions

View File

@@ -212,7 +212,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
# Enable code coverage for Debian Bookworm using GCC 14 in Debug and no
# Unity on linux/amd64
if (
f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-14"
f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-15"
and build_type == "Debug"
and "-Dunity=OFF" in cmake_args
and architecture["platform"] == "linux/amd64"
@@ -239,6 +239,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
# so that they are easier to identify in the GitHub Actions UI, as long
# names get truncated.
# Add Address and Thread (both coupled with UB) sanitizers for specific bookworm distros.
# GCC-Asan rippled-embedded tests are failing because of https://github.com/google/sanitizers/issues/856
if (
os["distro_version"] == "bookworm"
and f"{os['compiler_name']}-{os['compiler_version']}" == "clang-20"

View File

@@ -166,6 +166,14 @@ jobs:
run: |
./xrpld --version | grep libvoidstar
- name: Set sanitizer options
if: ${{ !inputs.build_only && env.SANITIZERS_ENABLED == 'true' }}
run: |
echo "ASAN_OPTIONS=detect_container_overflow=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" >> ${GITHUB_ENV}
echo "TSAN_OPTIONS=second_deadlock_stack=1 halt_on_error=0 suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV}
echo "UBSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV}
echo "LSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV}
- name: Run the separate tests
if: ${{ !inputs.build_only }}
working-directory: ${{ inputs.build_dir }}
@@ -174,10 +182,6 @@ jobs:
BUILD_TYPE: ${{ inputs.build_type }}
PARALLELISM: ${{ runner.os == 'Windows' && '1' || steps.nproc.outputs.nproc }}
run: |
export ASAN_OPTIONS="detect_container_overflow=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp"
export TSAN_OPTIONS="second_deadlock_stack=1 halt_on_error=0 suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp"
export UBSAN_OPTIONS="suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp"
export LSAN_OPTIONS="suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp"
ctest \
--output-on-failure \
-C "${BUILD_TYPE}" \
@@ -189,10 +193,6 @@ jobs:
env:
BUILD_NPROC: ${{ steps.nproc.outputs.nproc }}
run: |
export ASAN_OPTIONS="detect_container_overflow=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp"
export TSAN_OPTIONS="second_deadlock_stack=1 halt_on_error=0 suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp"
export UBSAN_OPTIONS="suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp"
export LSAN_OPTIONS="suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp"
./xrpld --unittest --unittest-jobs "${BUILD_NPROC}"
- name: Debug failure (Linux)

View File

@@ -23,11 +23,10 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
set(is_gcc TRUE)
elseif(MSVC)
set(is_msvc TRUE)
else()
message(FATAL_ERROR "Unsupported C++ compiler: ${CMAKE_CXX_COMPILER_ID}")
endif()
set(xrpl_cxx_compiler_id "${CMAKE_CXX_COMPILER_ID}")
set(xrpl_cxx_compiler_version "${CMAKE_CXX_COMPILER_VERSION}")
# Backwards-compat aliases used in some modules
set(IS_CLANG ${is_clang})
set(IS_GCC ${is_gcc})
@@ -68,3 +67,10 @@ endif()
# Keep legacy uppercase alias for existing code
set(IS_AMD64 ${is_amd64})
# Sanitizer configuration from environment (flags set in XrplSanitizers.cmake)
if($ENV{SANITIZERS} AND $ENV{SANITIZERS} MATCHES "Address|Thread|UndefinedBehavior")
set(SANITIZERS_ENABLED TRUE)
else()
set(SANITIZERS_ENABLED FALSE)
endif()

View File

@@ -122,7 +122,7 @@ else ()
# * static option set and
# * NOT APPLE (AppleClang does not support static libc/c++) and
# * NOT SANITIZER (sanitizers typically don't work with static libc/c++)
$<$<AND:$<BOOL:${static}>,$<NOT:$<BOOL:${APPLE}>>,$<NOT:$<BOOL:${SANITIZER}>>>:
$<$<AND:$<BOOL:${static}>,$<NOT:$<BOOL:${APPLE}>>,$<NOT:$<BOOL:${SANITIZERS_ENABLED}>>>:
-static-libstdc++
-static-libgcc
>)

View File

@@ -36,19 +36,18 @@ 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()
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()
# 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")
@@ -95,7 +94,7 @@ if(is_gcc)
elseif(ENABLE_TSAN)
# GCC doesn't support atomic_thread_fence with tsan. Suppress warnings.
list(APPEND SANITIZERS_COMPILE_FLAGS "-Wno-tsan")
message(STATUS " Using medium code model (-mcmodel=medium)")
message(STATUS " Using medium code model (-mcmodel=medium)")
list(APPEND SANITIZERS_COMPILE_FLAGS "-mcmodel=medium")
list(APPEND SANITIZERS_RELOCATION_FLAGS "-mcmodel=medium")
endif()
@@ -139,11 +138,19 @@ target_compile_options(common INTERFACE
# Apply linker flags
target_link_options(common INTERFACE ${SANITIZERS_LINK_FLAGS})
# Define SANITIZER macro for BuildInfo.cpp
# Define SANITIZERS macro for BuildInfo.cpp
set(SANITIZERS_LIST)
if(ENABLE_ASAN)
target_compile_definitions(common INTERFACE SANITIZER=ASAN)
elseif(ENABLE_TSAN)
target_compile_definitions(common INTERFACE SANITIZER=TSAN)
elseif(ENABLE_UBSAN)
target_compile_definitions(common INTERFACE SANITIZER=UBSAN)
list(APPEND SANITIZERS_LIST "ASAN")
endif()
if(ENABLE_TSAN)
list(APPEND SANITIZERS_LIST "TSAN")
endif()
if(ENABLE_UBSAN)
list(APPEND SANITIZERS_LIST "UBSAN")
endif()
if(SANITIZERS_LIST)
list(JOIN SANITIZERS_LIST "_" SANITIZERS_STR)
target_compile_definitions(common INTERFACE SANITIZERS=${SANITIZERS_STR})
endif()

View File

@@ -58,13 +58,6 @@ else()
set(wextra OFF CACHE BOOL "gcc/clang only" FORCE)
endif()
# Sanitizer configuration from environment (flags set in XrplSanitizers.cmake)
if($ENV{SANITIZERS})
set(SANITIZER TRUE)
else()
set(SANITIZER FALSE)
endif()
if(is_linux AND NOT SANITIZER)
option(BUILD_SHARED_LIBS "build shared xrpl libraries" OFF)
option(static "link protobuf, openssl, libc++, and boost statically" ON)

View File

@@ -32,7 +32,7 @@ target_link_libraries(xrpl_boost
if(Boost_COMPILER)
target_link_libraries(xrpl_boost INTERFACE Boost::disable_autolinking)
endif()
if(SANITIZER AND is_clang)
if(SANITIZERS_ENABLED AND is_clang)
# TODO: gcc does not support -fsanitize-blacklist...can we do something else
# for gcc ?
if(NOT Boost_INCLUDE_DIRS AND TARGET Boost::headers)

View File

@@ -20,7 +20,7 @@ namespace BuildInfo {
char const* const versionString = "3.2.0-b0"
// clang-format on
#if defined(DEBUG) || defined(SANITIZER)
#if defined(DEBUG) || defined(SANITIZERS)
"+"
#ifdef GIT_COMMIT_HASH
GIT_COMMIT_HASH
@@ -28,13 +28,13 @@ char const* const versionString = "3.2.0-b0"
#endif
#ifdef DEBUG
"DEBUG"
#ifdef SANITIZER
#ifdef SANITIZERS
"."
#endif
#endif
#ifdef SANITIZER
BOOST_PP_STRINGIZE(SANITIZER)
#ifdef SANITIZERS
BOOST_PP_STRINGIZE(SANITIZERS)
#endif
#endif