Backport ASAN build fixes from rustls-ffi #10
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously if `$CC` was `clang` the `Makefile` would configure `CFLAGS` and `LDFLAGS` with address sanitizer (ASAN) enabled. We only want this to happen for _debug_ builds that are using `clang`, since it's generally considered bad practice to ship sanitizer builds as release artifacts. This commit updates the logic to require `PROFILE=debug`, and then switches CI to use that profile when running tests. The Valgrind test continues to use `PROFILE=release` because ASAN and Valgrind will conflict, raising the error "ASan runtime does not come first in initial library list;". It may be possible to use both with more care/configuration, but for now we'll just use valgrind with a release build.
Previously the `Makefile` conditionally enabled address sanitizer (ASAN) based on whether the compiler was `clang`. Since GCC has supported address sanitizer since GCC 4.8[0] we can remove this check and always use ASAN for both supported compilers. The only snag is that I observed linker errors about undefined references to `__ubsan_handle_type_mismatch` unless I fix the `LDFLAGS` to also include `-fsanitize=undefined`. This was already included in the `CFLAGS`. [0]: https://github.com/google/sanitizers/wiki/AddressSanitizer#getting-addresssanitizer
ctz
approved these changes
Apr 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See rustls/rustls-ffi#423 & rustls/rustls-ffi#425
I think the main bit of use for this repo is that it enables ASAN for GCC builds. Since we're not using
clangin CI this should be a helpful test improvement.