ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-28 17:40:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

code review changes

Browse Source 
Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
This commit is contained in:
Pratik Mankawde
2026-01-07 15:18:14 +00:00
parent 8b89ae8c52
commit 9646537c62
10 changed files with 79 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
.github/scripts/strategy-matrix/generate.py vendored
View File

@@ -256,7 +256,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
"build_type": build_type,
"os": os,
"architecture": architecture,
"sanitizers": "Address,UndefinedBehavior",
"sanitizers": "address,undefinedbehavior",
}
)
# TSAN is deactivated due to seg faults with latest compilers.
@@ -271,7 +271,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
"build_type": build_type,
"os": os,
"architecture": architecture,
"sanitizers": "Thread,UndefinedBehavior",
"sanitizers": "thread,undefinedbehavior",
}
)
else:

2
.github/workflows/reusable-build-test-config.yml vendored
View File

@@ -204,7 +204,7 @@ jobs:
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 "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}

2
BUILD.md
View File

@@ -524,7 +524,7 @@ To build dependencies and xprld with sanitizer instrumentation, set the
`SANITIZERS` environment variable(only once before running conan and cmake) and use the `sanitizers` profile in conan:
```bash
export SANITIZERS=Address,UndefinedBehavior
export SANITIZERS=address,undefinedbehavior
conan install .. --output-folder . --profile:all sanitizers --build missing --settings build_type=Debug

6
cmake/CompilationEnv.cmake
View File

@@ -49,6 +49,8 @@ endif()
set(is_64bit FALSE)
if(CMAKE_SIZEOF_VOID_P EQUAL 8)
set(is_64bit TRUE)
else()
message(FATAL_ERROR "This project should only be built on 64bit architectures.")
endif()
set(pointer_size "${CMAKE_SIZEOF_VOID_P}")
@@ -65,8 +67,8 @@ 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")
# Sanitizer configuration read from environment. We then set appropriate flags in XrplSanitizers.cmake
if($ENV{SANITIZERS} AND $ENV{SANITIZERS} MATCHES "address|thread|undefinedbehavior")
set(SANITIZERS_ENABLED TRUE)
else()
set(SANITIZERS_ENABLED FALSE)

2
cmake/XrplInterface.cmake
View File

@@ -44,8 +44,6 @@ if(jemalloc)
target_link_libraries(opts INTERFACE jemalloc::jemalloc)
endif ()
# Sanitizer configuration is handled in XrplSanitizers.cmake
#[===================================================================[
xrpld transitive library deps via an interface library
#]===================================================================]

18
cmake/XrplSanitizers.cmake
View File

@@ -3,11 +3,11 @@
This module reads the following environment variables:
- SANITIZERS: The sanitizers to enable. Possible values:
- "Address"
- "Address,UndefinedBehavior"
- "Thread"
- "Thread,UndefinedBehavior"
- "UndefinedBehavior"
- "address"
- "address,undefinedbehavior"
- "thread"
- "thread,undefinedbehavior"
- "undefinedbehavior"
The compiler type and platform are detected in CompilationEnv.cmake.
The sanitizer compile options are applied to the 'common' interface library
@@ -36,15 +36,15 @@ string(REPLACE "," ";" _san_list "${_san_list}")
separate_arguments(_san_list)
foreach(_san IN LISTS _san_list)
if(_san STREQUAL "Address")
if(_san STREQUAL "address")
set(ENABLE_ASAN TRUE)
elseif(_san STREQUAL "Thread")
elseif(_san STREQUAL "thread")
set(ENABLE_TSAN TRUE)
elseif(_san STREQUAL "UndefinedBehavior")
elseif(_san STREQUAL "undefinedbehavior")
set(ENABLE_UBSAN TRUE)
else()
message(FATAL_ERROR "Unsupported sanitizer type: ${_san}"
"Supported: Address, Thread, UndefinedBehavior and their combinations.")
"Supported: address, thread, undefinedbehavior and their combinations.")
endif()
endforeach()

16
conan/profiles/sanitizers
View File

@@ -5,21 +5,21 @@ include(default)
[conf]
{% if sanitizers %}
{% if compiler == "gcc" %}
{% if "Address" in sanitizers or "Thread" in sanitizers or "UndefinedBehavior" in sanitizers %}
{% if "address" in sanitizers or "thread" in sanitizers or "undefinedbehavior" in sanitizers %}
{% set sanitizer_list = [] %}
{% set model_code = "" %}
{% set extra_cxxflags = ["-fno-omit-frame-pointer", "-O1", "-Wno-stringop-overflow"] %}
{% if "Address" in sanitizers %}
{% if "address" in sanitizers %}
{% set _ = sanitizer_list.append("address") %}
{% set model_code = "-mcmodel=large" %}
{% elif "Thread" in sanitizers %}
{% elif "thread" in sanitizers %}
{% set _ = sanitizer_list.append("thread") %}
{% set model_code = "-mcmodel=medium" %}
{% set _ = extra_cxxflags.append("-Wno-tsan") %}
{% endif %}
{% if "UndefinedBehavior" in sanitizers %}
{% if "undefinedbehavior" in sanitizers %}
{% set _ = sanitizer_list.append("undefined") %}
{% set _ = sanitizer_list.append("float-divide-by-zero") %}
{% endif %}
@@ -31,17 +31,17 @@ include(default)
tools.build:exelinkflags+=['{{sanitizer_flags}}']
{% endif %}
{% elif compiler == "apple-clang" or compiler == "clang" %}
{% if "Address" in sanitizers or "Thread" in sanitizers or "UndefinedBehavior" in sanitizers %}
{% if "address" in sanitizers or "thread" in sanitizers or "undefinedbehavior" in sanitizers %}
{% set sanitizer_list = [] %}
{% set extra_cxxflags = ["-fno-omit-frame-pointer", "-O1"] %}
{% if "Address" in sanitizers %}
{% if "address" in sanitizers %}
{% set _ = sanitizer_list.append("address") %}
{% elif "Thread" in sanitizers %}
{% elif "thread" in sanitizers %}
{% set _ = sanitizer_list.append("thread") %}
{% endif %}
{% if "UndefinedBehavior" in sanitizers %}
{% if "undefinedbehavior" in sanitizers %}
{% set _ = sanitizer_list.append("undefined") %}
{% set _ = sanitizer_list.append("float-divide-by-zero") %}
{% set _ = sanitizer_list.append("unsigned-integer-overflow") %}

79
docs/build/sanitizers.md vendored
View File

@@ -1,6 +1,6 @@
# Sanitizer Configuration for Rippled
This document explains how to properly configure and run sanitizers (AddressSanitizer, UndefinedBehaviorSanitizer, ThreadSanitizer) with the rippled project.
This document explains how to properly configure and run sanitizers (AddressSanitizer, undefinedbehaviorSanitizer, ThreadSanitizer) with the xrpld project.
Corresponding suppression files are located in the `sanitizers/suppressions` directory.
- [Sanitizer Configuration for Rippled](#sanitizer-configuration-for-rippled)
@@ -11,9 +11,10 @@ Corresponding suppression files are located in the `sanitizers/suppressions` dir
- [Call CMake](#call-cmake)
- [Build](#build)
- [Running Tests with Sanitizers](#running-tests-with-sanitizers)
- [AddressSanitizer (ASan)](#addresssanitizer-asan)
- [ThreadSanitizer (TSan) + UndefinedBehaviorSanitizer (UBSan)](#threadsanitizer-tsan--undefinedbehaviorsanitizer-ubsan)
- [AddressSanitizer (ASAN)](#addresssanitizer-asan)
- [ThreadSanitizer (TSan)](#threadsanitizer-tsan)
- [LeakSanitizer (LSan)](#leaksanitizer-lsan)
- [undefinedbehaviorSanitizer (UBSan)](#undefinedbehaviorsanitizer-ubsan)
- [Suppression Files](#suppression-files)
- [`asan.supp`](#asansupp)
- [`lsan.supp`](#lsansupp)
@@ -21,7 +22,7 @@ Corresponding suppression files are located in the `sanitizers/suppressions` dir
- [`tsan.supp`](#tsansupp)
- [`sanitizer-ignorelist.txt`](#sanitizer-ignorelisttxt)
- [Troubleshooting](#troubleshooting)
- ["ASan is ignoring requested \_\_asan_handle_no_return" warnings](#asan-is-ignoring-requested-__asan_handle_no_return-warnings)
- ["ASAN is ignoring requested \_\_asan_handle_no_return" warnings](#asan-is-ignoring-requested-__asan_handle_no_return-warnings)
- [Sanitizer Mismatch Errors](#sanitizer-mismatch-errors)
- [References](#references)
@@ -33,9 +34,9 @@ Follow the same instructions as mentioned in [BUILD.md](../../BUILD.md) but with
1. Make sure you have a clean build directory.
2. Set the `SANITIZERS` environment variable before calling conan install and cmake. Only set it once. Make sure both conan and cmake read the same values.
Example: `export SANITIZERS=Address,UndefinedBehavior`
3. Optionally use `--profile:all sanitizers` with Conan to build dependencies with sanitizer instrumentation.
4. Set `ASAN_OPTIONS`, `LSAN_OPTIONS`, `UBSAN_OPTIONS` and `TSAN_OPTIONS` environment variables to configure sanitizer behavior when running executables.
Example: `export SANITIZERS=address,undefinedbehavior`
3. Optionally use `--profile:all sanitizers` with Conan to build dependencies with sanitizer instrumentation. [!NOTE]Building with sanitizer-instrumented dependencies is slower but produces fewer false positives.
4. Set `ASAN_OPTIONS`, `LSAN_OPTIONS`, `UBSAN_OPTIONS` and `TSAN_OPTIONS` environment variables to configure sanitizer behavior when running executables. [More details below](#running-tests-with-sanitizers).
---
@@ -53,7 +54,7 @@ cd .build
The `SANITIZERS` environment variable is used by both Conan and CMake.
```bash
export SANITIZERS=Address,UndefinedBehavior
export SANITIZERS=address,undefinedbehavior
# Standard build (without instrumenting dependencies)
conan install .. --output-folder . --build missing --settings build_type=Debug
@@ -61,7 +62,8 @@ conan install .. --output-folder . --build missing --settings build_type=Debug
conan install .. --output-folder . --profile:all sanitizers --build missing --settings build_type=Debug
```
**Note:** Do not mix Address and Thread sanitizers - they are incompatible.
[!CAUTION]
Do not mix Address and Thread sanitizers - they are incompatible.
Since you already set the `SANITIZERS` environment variable when running Conan, same values will be read for the next part.
@@ -82,48 +84,65 @@ cmake --build . --parallel 4
## Running Tests with Sanitizers
### AddressSanitizer (ASan)
### AddressSanitizer (ASAN)
**IMPORTANT**: ASan with Boost produces many false positives. Use these options:
**IMPORTANT**: ASAN with Boost produces many false positives. Use these options:
```bash
export ASAN_OPTIONS="detect_container_overflow=0 suppressions=path/to/asan.supp halt_on_error=0 log_path=asan.log"
export UBSAN_OPTIONS="suppressions=path/to/ubsan.supp print_stacktrace=1 halt_on_error=0 log_path=ubsan.log"
export LSAN_OPTIONS="suppressions=path/to/lsan.supp halt_on_error=0 log_path=lsan.log"
export ASAN_OPTIONS="detect_container_overflow=0:suppressions=path/to/asan.supp:halt_on_error=0:log_path=asan.log"
export LSAN_OPTIONS="suppressions=path/to/lsan.supp:halt_on_error=0:log_path=lsan.log"
# Run tests
./rippled --unittest --unittest-jobs=5
./xrpld --unittest --unittest-jobs=5
```
**Why `detect_container_overflow=0`?**
- Boost intrusive containers (used in `aged_unordered_container`) trigger false positives
- Boost context switching (used in `Workers.cpp`) confuses ASan's stack tracking
- Since we usually don't build boost(because we don't want to instrument boost and detect issues in boost code) with asan but use boost containers in ASAN instrumented rippled code, it generates false positives.
- Boost context switching (used in `Workers.cpp`) confuses ASAN's stack tracking
- Since we usually don't build Boost(because we don't want to instrument Boost and detect issues in Boost code) with ASAN but use Boost containers in ASAN instrumented rippled code, it generates false positives. Building dependencies with ASAN instrumentation reduces false positives.
- See: https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow
- More such flags are detailed here: https://github.com/google/sanitizers/wiki/AddressSanitizerFlags
### ThreadSanitizer (TSan) + UndefinedBehaviorSanitizer (UBSan)
### ThreadSanitizer (TSan)
```bash
export TSAN_OPTIONS="suppressions=path/to/tsan.supp halt_on_error=0 log_path=tsan.log"
# Run tests
./rippled --unittest --unittest-jobs=5
./xrpld --unittest --unittest-jobs=5
```
More details: https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual
### LeakSanitizer (LSan)
LSan is automatically enabled with ASan. To disable it:
LSan is automatically enabled with ASAN. To disable it:
```bash
export ASAN_OPTIONS="detect_leaks=0"
```
More details here: https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer
### undefinedbehaviorSanitizer (UBSan)
```bash
export UBSAN_OPTIONS="suppressions=path/to/ubsan.supp:print_stacktrace=1:halt_on_error=0:log_path=ubsan.log"
# Run tests
./xrpld --unittest --unittest-jobs=5
```
More details here: https://clang.llvm.org/docs/undefinedbehaviorSanitizer.html
## Suppression Files
### `asan.supp`
[!NOTE] Attached files contain more details.
- **Purpose**: Suppress AddressSanitizer (ASan) errors only
### [`asan.supp`](../../sanitizers/suppressions/asan.supp)
- **Purpose**: Suppress AddressSanitizer (ASAN) errors only
- **Format**: `interceptor_name:<pattern>` where pattern matches file names. Supported suppression types are:
- interceptor_name
- interceptor_via_fun
@@ -132,25 +151,25 @@ export ASAN_OPTIONS="detect_leaks=0"
- **More info**: [AddressSanitizer](https://github.com/google/sanitizers/wiki/AddressSanitizer)
- **Note**: Cannot suppress stack-buffer-overflow, container-overflow, etc.
### `lsan.supp`
### [`lsan.supp`](../../sanitizers/suppressions/lsan.supp)
- **Purpose**: Suppress LeakSanitizer (LSan) errors only
- **Format**: `leak:<pattern>` where pattern matches function/file names
- **More info**: [LeakSanitizer](https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer)
### `ubsan.supp`
### [`ubsan.supp`](../../sanitizers/suppressions/ubsan.supp)
- **Purpose**: Suppress UndefinedBehaviorSanitizer errors
- **Purpose**: Suppress undefinedbehaviorSanitizer errors
- **Format**: `<error_type>:<pattern>` (e.g., `unsigned-integer-overflow:protobuf`)
- **Covers**: Intentional overflows in sanitizers/suppressions libraries (protobuf, gRPC, stdlib)
### `tsan.supp`
### [`tsan.supp`](../../sanitizers/suppressions/tsan.supp)
- **Purpose**: Suppress ThreadSanitizer data race warnings
- **Format**: `race:<pattern>` where pattern matches function/file names
- **More info**: [ThreadSanitizerSuppressions](https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions)
### `sanitizer-ignorelist.txt`
### [`sanitizer-ignorelist.txt`](../../sanitizers/suppressions/sanitizer-ignorelist.txt)
- **Purpose**: Compile-time ignorelist for all sanitizers
- **Usage**: Passed via `-fsanitize-ignorelist=absolute/path/to/sanitizer-ignorelist.txt`
@@ -158,13 +177,13 @@ export ASAN_OPTIONS="detect_leaks=0"
## Troubleshooting
### "ASan is ignoring requested \_\_asan_handle_no_return" warnings
### "ASAN is ignoring requested \_\_asan_handle_no_return" warnings
These warnings appear when using Boost context switching and are harmless. They indicate potential false positives.
### Sanitizer Mismatch Errors
If you see undefined symbols like `___tsan_atomic_load` when building with ASan:
If you see undefined symbols like `___tsan_atomic_load` when building with ASAN:
**Problem**: Dependencies were built with a different sanitizer than the main project.
@@ -182,5 +201,5 @@ Then review the log files: `asan.log.*`, `ubsan.log.*`, `tsan.log.*`
- [AddressSanitizer Wiki](https://github.com/google/sanitizers/wiki/AddressSanitizer)
- [AddressSanitizer Flags](https://github.com/google/sanitizers/wiki/AddressSanitizerFlags)
- [Container Overflow Detection](https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow)
- [UndefinedBehaviorSanitizer](https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html)
- [undefinedbehaviorSanitizer](https://clang.llvm.org/docs/undefinedbehaviorSanitizer.html)
- [ThreadSanitizer](https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual)

2
sanitizers/suppressions/asan.supp
View File

@@ -1,6 +1,6 @@
# The idea is to empty this file gradually by fixing the underlying issues and removing suppresions.
#
# ASAN_OPTIONS="detect_container_overflow=0 suppressions=sanitizers/suppressions/asan.supp halt_on_error=0"
# ASAN_OPTIONS="detect_container_overflow=0:suppressions=sanitizers/suppressions/asan.supp:halt_on_error=0"
#
# The detect_container_overflow=0 option disables false positives from:
# - Boost intrusive containers (slist_iterator.hpp, hashtable.hpp, aged_unordered_container.h)

4
sanitizers/suppressions/sanitizer-ignorelist.txt
View File

@@ -1,3 +1,7 @@
# We were seeing some false positives and some repeated errors(since these are library files) in following files.
# Clang will skip instrumenting the files added here.
# We should fix the underlying issues(if any) and remove these entries.
deadlock:libxrpl/beast/utility/beast_Journal.cpp
deadlock:libxrpl/beast/utility/beast_Journal.cpp
deadlock:libxrpl/beast/utility/beast_PropertyStream.cpp