Skip to content

Conversation

@cnaples79
Copy link

@cnaples79 cnaples79 commented Oct 7, 2025

Summary

  • add a dedicated GitHub Actions workflow that runs address-, memory-, and thread-sanitized builds
  • configure clang/clang++ with the appropriate sanitizer compile/link flags and reuse the project’s cmake flow
  • reuse the existing googletest fetch so sanitizer runs execute the ctest target

Rationale

Issue #457 covers the lack of sanitizer coverage. Running ASan/MSan/TSan in CI helps catch undefined behavior early without altering the main build pipeline.

Fixes #457

- name: Configure sanitizer environment
run: |
if command -v llvm-symbolizer >/dev/null 2>&1; then
echo LLVM_SYMBOLIZER_PATH=$(command -v llvm-symbolizer) >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

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

What's this for and what uses it?


- uses: zacharyburnett/setup-abseil-cpp@713a4383f10b05948a6d9d4906056063c8da1168 # Not a release, but has #423 fix.
env:
CC: clang
Copy link
Member

Choose a reason for hiding this comment

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

Do these need to be set again? I'm not very familiar with github actions.

include:
- name: asan
sanitizer: address
compile_flags: "-fsanitize=address -fno-omit-frame-pointer"
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT about -fsanitize=address,undefined here? Merge the UBSAN and ASAN together? UBSAN is the one that catches a lot of errors during testing for me.

Copy link
Member

Choose a reason for hiding this comment

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

I think -fsanitize=address,undefined,local-bounds,nullability looks good.

https://github.com/google/s2geometry/actions/runs/18301785002/job/53309264384?pr=461#step:3:143

include:
- name: asan
sanitizer: address
compile_flags: "-fsanitize=address -fno-omit-frame-pointer"
Copy link
Member

Choose a reason for hiding this comment

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

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.

Add sanitizer workflows

3 participants