Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++][CMake] Consider adding cmake presets for sanitizer builds #45206

Closed
zanmato1984 opened this issue Jan 9, 2025 · 3 comments
Closed

[C++][CMake] Consider adding cmake presets for sanitizer builds #45206

zanmato1984 opened this issue Jan 9, 2025 · 3 comments

Comments

@zanmato1984
Copy link
Contributor

zanmato1984 commented Jan 9, 2025

Describe the enhancement requested

It's a bit inconvenient when building/debugging C++ code with sanitizers, esp. working in an IDE like vscode. I either manually modify CMakePresets.json file to add a temporary preset with necessary sanitizer variables (and carefully avoid including this changes along with other intended changes), or change vscode setting by passing the sanitizer variables into cmake configuration process (but this will rebuild most of the source files because compiler flags have changed). And specifying sanitizer variables always involves at least three variables along: ARROW_USE_XXSAN=ON ARROW_JEMALLOC=OFF and ARROW_MIMALLOC=OFF (sanitizers generally don't work with 3rd-party allocators).

We should consider adding cmake presets for such sanitizer builds.

Component(s)

C++

@assignUser
Copy link
Member

manually modify CMakePresets.json file to add a temporary preset with necessary sanitizer variables (and carefully avoid including this changes along with other intended changes),

That's what CMakeUserPresets.json is for! It's already in the gitignore and should also be picked up by vsc.

But regardless a preset for sanitizers makes sense :)

@zanmato1984
Copy link
Contributor Author

That's what CMakeUserPresets.json is for! It's already in the gitignore and should also be picked up by vsc.

But regardless a preset for sanitizers makes sense :)

Oops, I think CMakeUserPresets.json might just work for me, thank you!

As for the PR already filed, let's just have it as long as people think it would make sense - though the reasoning in this issue doesn't fully justify :)

zanmato1984 added a commit that referenced this issue Jan 9, 2025
### Rationale for this change

See #45206

### What changes are included in this PR?

Add base presets for ASAN/TSAN/UBSAN. And cross product such base presets by `ninja-debug` which is, IMO, the config just enough necessary and likely to need sanitizers' aid. 

### Are these changes tested?

No need.

### Are there any user-facing changes?

None.

* GitHub Issue: #45206

Authored-by: Rossi Sun <[email protected]>
Signed-off-by: Rossi Sun <[email protected]>
@zanmato1984 zanmato1984 added this to the 20.0.0 milestone Jan 9, 2025
@zanmato1984
Copy link
Contributor Author

Issue resolved by pull request 45207
#45207

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants