Sponsored Content
Skip to content

chore: Upgrade Clang sanitizer to clang-22 and switch gcc-15 sanitizer to Release - #7079

Merged
bthomee merged 7 commits into
developfrom
bronek/add_clang-22_build
May 7, 2026
Merged

chore: Upgrade Clang sanitizer to clang-22 and switch gcc-15 sanitizer to Release#7079
bthomee merged 7 commits into
developfrom
bronek/add_clang-22_build

Conversation

@Bronek

@Bronek Bronek commented May 5, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

This PR does not add any new builds. It does replace the following builds on Debian:

  • clang-20 Debug on bookworm replaced with clang-22 on trixie (ASAN and UBSAN)
  • gcc-15 Debug replaced with gcc-15 Release (ASAN and UBSAN)

Context of Change

Clang 22 was released by end of February 2026 and is now available in Ubuntu 26.04 Resolute LTS, let's ensure compatibility with this compiler.

ASAN and UBSAN may yield useful results on optimized builds, let's switch GCC from Debug to Release while using Clang for Debug.

@Bronek
Bronek requested review from bthomee and mathbunnyru May 5, 2026 21:03

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Comment on lines 257 to 258
"gcc-15",
"clang-20",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What's the benefit of keeping GCC-15 and Clang-20 on Bookworm with ASAN and UBSAN (GCC only) and then separately running UBSAN on Clang-22 vs. just changing Bookworm to Trixie and Clang-20 to Clang-22 here?

@Bronek Bronek May 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if we do not care much about building with clang-20 or clang-21 (and I think that, as long as we build with clang-22, that's fair) then we could do that, i.e. switch debian-bookworm:clang-20 to debian-trixie:clang-22 for both ASAN and UBSAN. However the UBSAN implementation on GCC and Clang is sufficiently different to run these sanitizers with both compilers. Having said that, it would also make sense to switch gcc-15 from Debug build to Release and run these two sanitizers with GCC on Release build, so that the sanitizer works on optimized code (with instrumentation). Would that be ok with you ?

We might also enable MemorySanitizer on Clang (not available on GCC) but that's presumably a different PR. EDIT: MemorySanitizer requires libc++ on Linux and that's definitely out of scope here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That would be generally fine with me, although we are hitting the limits of the runners with the number of currently triggered runs - typically they are OOMing. We can of course reduce the number of concurrently running jobs...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, but that won't increase the number of build/test jobs. Replacing clang-20 with clang-22 and switching the existing gcc-15 from Debug to Release.

@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.1%. Comparing base (8e2aa33) to head (cadad38).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7079     +/-   ##
=========================================
- Coverage     82.1%   82.1%   -0.0%     
=========================================
  Files         1010    1010             
  Lines        76023   76023             
  Branches      7372    7366      -6     
=========================================
- Hits         62409   62403      -6     
- Misses       13614   13620      +6     

see 3 files with indirect coverage changes

Impacted file tree graph

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

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ValueError crash risk on RHEL clang entries — see inline.

Review by Claude Opus 4.6 · Prompt: V15

Comment thread .github/scripts/strategy-matrix/generate.py
@Bronek Bronek changed the title chore: Add clang-22 and enable clang-21 chore: Upgrade ASAN/UBSAN Clang builds to clang-22 and switch ASAN/UBSAN on gcc-15 to Release May 6, 2026
@Bronek Bronek changed the title chore: Upgrade ASAN/UBSAN Clang builds to clang-22 and switch ASAN/UBSAN on gcc-15 to Release chore: Upgrade ASAN/UBSAN Clang builds to clang-22 and switch ASAN/UBSAN on gcc-15 to Release May 6, 2026
@Bronek Bronek changed the title chore: Upgrade ASAN/UBSAN Clang builds to clang-22 and switch ASAN/UBSAN on gcc-15 to Release chore: Upgrade Clang sanitizer to clang-22 and switch gcc-15 sanitizer to Release May 6, 2026
@Bronek
Bronek force-pushed the bronek/add_clang-22_build branch from c96a3ff to d6eb9e5 Compare May 6, 2026 19:18

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Took a pass through this

The sanitizer condition at line 243 creates unintended extra builds — see inline.


Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V15

Comment thread .github/scripts/strategy-matrix/generate.py Outdated

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

f"{os['compiler_name']}-{os['compiler_version']}"
in ["clang-20", "clang-21"]
os["compiler_name"] == "clang"
and os["compiler_version"].isdigit()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one is probably not needed

@Bronek Bronek May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We get any for os["compiler_version"] on rhel, see https://github.com/XRPLF/rippled/actions/runs/25455105133/job/74682423667?pr=7079 . Since there is going to be clang-23 by end of 2026 , it's probably better to avoid listing these compiler versions individually.

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@bthomee
bthomee added this pull request to the merge queue May 7, 2026
Merged via the queue into develop with commit d67e061 May 7, 2026
3 checks passed
@bthomee
bthomee deleted the bronek/add_clang-22_build branch May 7, 2026 11:18
@mvadari mvadari added this to the 3.2.0 milestone May 20, 2026
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants