From 32f7baca44bfd0d2acec368edaf6b0fe3e6cb4fb Mon Sep 17 00:00:00 2001
From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Date: Tue, 16 Dec 2025 11:57:04 +0000
Subject: [PATCH] code review changes

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
---
 BUILD.md                           |  6 +--
 cmake/XrplSanitizers.cmake         | 31 +++++++++++---
 docs/build/sanitizers.md           | 66 +++++++++---------------------
 sanitizers/suppressions/ubsan.supp | 37 +++++++++--------
 4 files changed, 68 insertions(+), 72 deletions(-)

diff --git a/BUILD.md b/BUILD.md
index d2c5949191..d6beaeed64 100644
--- a/BUILD.md
+++ b/BUILD.md
@@ -389,13 +389,13 @@ tools.build:cxxflags=['-DBOOST_ASIO_DISABLE_CONCEPTS']
    ```
 
    **Sanitizers:** To build dependencies with sanitizer instrumentation, set the
-   `SANITIZERS` environment variable and use the `sanitizers` profile:
+   `SANITIZERS` environment variable(only once for both conan and cmake) and use the `sanitizers` profile:
 
    ```
-   SANITIZERS=Address,UndefinedBehavior conan install .. --output-folder . --profile:all sanitizers --build missing --settings build_type=Debug
+   export SANITIZERS=Address,UndefinedBehavior
+   conan install .. --output-folder . --profile:all sanitizers --build missing --settings build_type=Debug
    ```
 
-   Note: Do not mix Address and Thread sanitizers - they are incompatible.
    See [sanitizers documentation](./docs/build/sanitizers.md) for more details.
 
    To build Debug, in the next step, be sure to set `-DCMAKE_BUILD_TYPE=Debug`
diff --git a/cmake/XrplSanitizers.cmake b/cmake/XrplSanitizers.cmake
index a93728f025..89c0133131 100644
--- a/cmake/XrplSanitizers.cmake
+++ b/cmake/XrplSanitizers.cmake
@@ -28,14 +28,28 @@ set(ENABLE_ASAN FALSE)
 set(ENABLE_TSAN FALSE)
 set(ENABLE_UBSAN FALSE)
 
-if(SANITIZERS MATCHES "Address")
+# Normalize SANITIZERS into a list
+set(_san_list "${SANITIZERS}")
+string(REPLACE "," ";" _san_list "${_san_list}")
+separate_arguments(_san_list)
+
+set(REMAINING_SANITIZERS "")
+foreach(_san IN LISTS _san_list)
+  if(_san STREQUAL "Address")
     set(ENABLE_ASAN TRUE)
-endif()
-if(SANITIZERS MATCHES "Thread")
+  elseif(_san STREQUAL "Thread")
     set(ENABLE_TSAN TRUE)
-endif()
-if(SANITIZERS MATCHES "UndefinedBehavior")
+  elseif(_san STREQUAL "UndefinedBehavior")
     set(ENABLE_UBSAN TRUE)
+  else()
+    list(APPEND REMAINING_SANITIZERS "${_san}")
+  endif()
+endforeach()
+
+if(REMAINING_SANITIZERS)
+  message(FATAL_ERROR
+    "Unknown SANITIZERS value(s): ${REMAINING_SANITIZERS}. "
+    "Supported: Address, Thread, UndefinedBehavior and their combinations.")
 endif()
 
 # Detect compiler type
@@ -98,6 +112,13 @@ endif()
 
 # Compiler-specific configuration
 if(IS_GCC)
+    # Remove any -fuse-ld=... flags previously added to 'common'
+    get_target_property(_common_if_libs common INTERFACE_LINK_LIBRARIES)
+    if(_common_if_libs)
+        list(REMOVE_ITEM _common_if_libs "-fuse-ld=mold" "-fuse-ld=gold" "-fuse-ld=lld")
+        set_target_properties(common PROPERTIES INTERFACE_LINK_LIBRARIES "${_common_if_libs}")
+    endif()
+
     # Disable mold, gold and lld linkers for GCC with sanitizers
     # Use default linker (bfd/ld) which is more lenient with mixed code models
     set(use_mold OFF CACHE BOOL "Use mold linker" FORCE)
diff --git a/docs/build/sanitizers.md b/docs/build/sanitizers.md
index 3c709ff807..3f934061e0 100644
--- a/docs/build/sanitizers.md
+++ b/docs/build/sanitizers.md
@@ -8,14 +8,11 @@ Corresponding suppression files are located in the `sanitizers/suppressions` dir
     - [Summary](#summary)
     - [Build steps:](#build-steps)
       - [Install dependencies](#install-dependencies)
-      - [AddressSanitizer (ASan) + UndefinedBehaviorSanitizer (UBSan)](#addresssanitizer-asan--undefinedbehaviorsanitizer-ubsan)
-      - [ThreadSanitizer (TSan) + UndefinedBehaviorSanitizer (UBSan)](#threadsanitizer-tsan--undefinedbehaviorsanitizer-ubsan)
-      - [Just AddressSanitizer (ASan)](#just-addresssanitizer-asan)
-      - [Just UndefinedBehaviorSanitizer (UBSan)](#just-undefinedbehaviorsanitizer-ubsan)
+      - [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-1)
+    - [ThreadSanitizer (TSan) + UndefinedBehaviorSanitizer (UBSan)](#threadsanitizer-tsan--undefinedbehaviorsanitizer-ubsan)
     - [LeakSanitizer (LSan)](#leaksanitizer-lsan)
   - [Suppression Files](#suppression-files)
     - [`asan.supp`](#asansupp)
@@ -35,8 +32,9 @@ Corresponding suppression files are located in the `sanitizers/suppressions` dir
 Follow the same instructions as mentioned in [BUILD.md](../../BUILD.md) but with the following changes:
 
 1. Make sure you have a clean build directory.
-2. Set the `SANITIZERS` environment variable when running CMake to enable sanitizers.
-3. Optionally use `--profile sanitizers` with Conan to build dependencies with sanitizer instrumentation.
+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.
 
 ---
@@ -55,53 +53,27 @@ cd .build
 The `SANITIZERS` environment variable is used by both Conan and CMake.
 
 ```bash
+export SANITIZERS=Address,UndefinedBehavior
 # Standard build (without instrumenting dependencies)
-SANITIZERS=Address,UndefinedBehavior conan install .. --output-folder . --build missing --settings build_type=Debug
+conan install .. --output-folder . --build missing --settings build_type=Debug
 
 # Or with sanitizer-instrumented dependencies (takes longer but fewer false positives)
-SANITIZERS=Address,UndefinedBehavior conan install .. --output-folder . --profile:all sanitizers --build missing --settings build_type=Debug
-```
-
-#### AddressSanitizer (ASan) + UndefinedBehaviorSanitizer (UBSan)
-
-Set the `SANITIZERS` environment variable when running CMake:
-
-```bash
-SANITIZERS=Address,UndefinedBehavior cmake .. -G Ninja \
-    -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-    -DCMAKE_BUILD_TYPE=Debug \
-    -Dtests=ON -Dxrpld=ON
-```
-
-#### ThreadSanitizer (TSan) + UndefinedBehaviorSanitizer (UBSan)
-
-```bash
-SANITIZERS=Thread,UndefinedBehavior cmake .. -G Ninja \
-    -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-    -DCMAKE_BUILD_TYPE=Debug \
-    -Dtests=ON -Dxrpld=ON
-```
-
-#### Just AddressSanitizer (ASan)
-
-```bash
-SANITIZERS=Address cmake .. -G Ninja \
-    -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-    -DCMAKE_BUILD_TYPE=Debug \
-    -Dtests=ON -Dxrpld=ON
-```
-
-#### Just UndefinedBehaviorSanitizer (UBSan)
-
-```bash
-SANITIZERS=UndefinedBehavior cmake .. -G Ninja \
-    -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-    -DCMAKE_BUILD_TYPE=Debug \
-    -Dtests=ON -Dxrpld=ON
+conan install .. --output-folder . --profile:all sanitizers --build missing --settings build_type=Debug
 ```
 
 **Note:** 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.
+
+#### Call CMake
+
+```bash
+cmake .. -G Ninja \
+    -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
+    -DCMAKE_BUILD_TYPE=Debug \
+    -Dtests=ON -Dxrpld=ON
+```
+
 #### Build
 
 ```bash
diff --git a/sanitizers/suppressions/ubsan.supp b/sanitizers/suppressions/ubsan.supp
index 7a261f9924..1504ef685f 100644
--- a/sanitizers/suppressions/ubsan.supp
+++ b/sanitizers/suppressions/ubsan.supp
@@ -199,36 +199,39 @@ unsigned-integer-overflow:__charconv/traits.h
 
 # Suppress errors in RocksDB
 # RocksDB uses intentional unsigned integer overflows in hash functions and CRC calculations
-unsigned-integer-overflow:rocks*/b/src/util/xxhash.h
-unsigned-integer-overflow:rocks*/b/src/util/xxph3.h
-unsigned-integer-overflow:rocks*/b/src/util/hash.cc
-unsigned-integer-overflow:rocks*/b/src/util/crc32c.cc
-unsigned-integer-overflow:rocks*/b/src/util/crc32c.h
-unsigned-integer-overflow:rocks*/b/src/include/rocksdb/utilities/options_type.h
-unsigned-integer-overflow:rocks*/b/src/table/format.h
-unsigned-integer-overflow:rocks*/b/src/table/format.cc
-unsigned-integer-overflow:rocks*/b/src/table/block_based/block_based_table_builder.cc
-unsigned-integer-overflow:rocks*/b/src/table/block_based/reader_common.cc
-unsigned-integer-overflow:rocks*/b/src/db/version_set.cc
+unsigned-integer-overflow:rocks*/*/util/xxhash.h
+unsigned-integer-overflow:rocks*/*/util/xxph3.h
+unsigned-integer-overflow:rocks*/*/util/hash.cc
+unsigned-integer-overflow:rocks*/*/util/crc32c.cc
+unsigned-integer-overflow:rocks*/*/util/crc32c.h
+unsigned-integer-overflow:rocks*/*/include/rocksdb/utilities/options_type.h
+unsigned-integer-overflow:rocks*/*/table/format.h
+unsigned-integer-overflow:rocks*/*/table/format.cc
+unsigned-integer-overflow:rocks*/*/table/block_based/block_based_table_builder.cc
+unsigned-integer-overflow:rocks*/*/table/block_based/reader_common.cc
+unsigned-integer-overflow:rocks*/*/db/version_set.cc
 
 # RocksDB misaligned loads (intentional for performance on ARM64)
-alignment:rocks*/b/src/util/crc32c_arm64.cc
+alignment:rocks*/*/util/crc32c_arm64.cc
 
 # nudb intentional overflows in hash functions
 unsigned-integer-overflow:nudb/detail/xxhash.hpp
 alignment:nudb/detail/xxhash.hpp
 
 # Snappy compression library intentional overflows
-unsigned-integer-overflow:snapp*/b/src/snappy.cc
+unsigned-integer-overflow:snappy.cc
 
 # Abseil intentional overflows
-unsigned-integer-overflow:abse*/b/src/absl/strings/numbers.cc
-unsigned-integer-overflow:abse*/b/src/absl/strings/internal/cord_rep_flat.h
+unsigned-integer-overflow:absl/strings/numbers.cc
+unsigned-integer-overflow:absl/strings/internal/cord_rep_flat.h
+unsigned-integer-overflow:absl/base/internal/low_level_alloc.cc
+unsigned-integer-overflow:absl/hash/internal/hash.h
+unsigned-integer-overflow:absl/container/internal/raw_hash_set.h
 
 # Standard library intentional overflows in chrono duration arithmetic
 unsigned-integer-overflow:__chrono/duration.h
 
 # Suppress undefined errors in RocksDB and nudb
-undefined:rocks.*/b/src/util/crc32c_arm64.cc
-undefined:rocks.*/b/src/util/xxhash.h
+undefined:rocks.*/*/util/crc32c_arm64.cc
+undefined:rocks.*/*/util/xxhash.h
 undefined:nudb