rippled icon indicating copy to clipboard operation
rippled copied to clipboard

Add sanitizers to CI builds

Open pratikmankawde opened this issue 2 months ago • 2 comments

High Level Overview of Change

Added support for Sanitizer build options in CI builds workflow.

Context of Change

We want to run Address and Thread sanitizers with Undefined-behaviour sanitizer while running tests on CI. Build type: asan+ubsan & tsan+ubsan I have deactivated tsan builds for now.

Type of Change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Refactor (non-breaking change that only restructures code)
  • [ ] Performance (increase or change in throughput and/or latency)
  • [ ] Tests (you added tests for code that already exists, or your new feature included in this PR)
  • [ ] Documentation update
  • [x] Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • [ ] Release

pratikmankawde avatar Nov 04 '25 16:11 pratikmankawde

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 79.1%. Comparing base (f059f0b) to head (bc3553d).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5996   +/-   ##
=======================================
  Coverage     79.1%   79.1%           
=======================================
  Files          836     836           
  Lines        71245   71243    -2     
  Branches      8324    8314   -10     
=======================================
  Hits         56360   56360           
+ Misses       14885   14883    -2     
Files with missing lines Coverage Δ
src/libxrpl/ledger/View.cpp 94.5% <100.0%> (-<0.1%) :arrow_down:
src/libxrpl/protocol/BuildInfo.cpp 98.1% <ø> (ø)

... and 5 files with indirect coverage changes

Impacted file tree graph

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 05 '25 13:11 codecov[bot]

I have thought about this a bit (yep, on vacation😁) I don't think it currently works as intended locally.

What I mean is - you set options for building rippled itself on generate script. This has zero effect on local builds, so locally you will have a binary, which has dependencies built with sanitizer, and rippled code without sanitizer.

There is a way to fix it: you need to move all flags handling from generate script to cmake.

Benefits of this solution:

  • will work locally, and for more types of builds (by work I mean compile as intended), not just for the ones we chose to run in CI
  • if someone needs to fix something for their particular configuration, there will be a place where it should be changed
  • there will be no need to complicate that much the existing generate script
  • semgrep issue will be resolved
  • this will match Clio (we also use profiles + cmake)
  • you can put the sanitizers flag handling in a separate cmake file

That's a good suggestion. But I think the local builds with sanitizers work with the current changes. The documentation I added shares how to get this. Basically, like you said, we don't use generate script locally, rather the sanitizer conan profile. The flags in the profile are not set for dependencies, rather globally. When we use that, it automatically puts the sanitizer specific flags into toolchain file. Cmake then picks those and puts them in CMakecache. Then these flags are used for rippled as well.

pratikmankawde avatar Dec 04 '25 11:12 pratikmankawde

Note that the codebase already has references to sanitizers, which may or may not need to be deleted or repurposed:

  • BuildInfo.cpp => #if defined(SANITIZER) and #ifdef SANITIZER
  • XrplCompiler.cmake => ${san}
  • XrplInterface.cmake => ${SAN_FLAG}, SANITIZER=ASAN etc.
  • XrplSettings.cmake => set(san ...), set(SAN_FLAG "-fsanitize=${san}"), set(SAN_LIB "")
  • Boost.cmake => if(san AND is_clang)

bthomee avatar Dec 17 '25 22:12 bthomee

  • BuildInfo.cpp

Cleaned up.

pratikmankawde avatar Dec 18 '25 16:12 pratikmankawde