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.

Sign up for GitHub

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

Jump to bottom

Avoid ASAN for release builds, use w/ GCC or clang in debug builds #425

Merged
merged 3 commits into from
Apr 19, 2024
Merged

Avoid ASAN for release builds, use w/ GCC or clang in debug builds #425

merged 3 commits into from
Apr 19, 2024

Commits on Apr 19, 2024

  1. Makefile: only use ASAN for debug+clang 

    Previously if `$CC` was `clang` the `Makefile` and `Makefile.pkg-config`
make files 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.
    @cpu
    cpu committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    b3c78e3 View commit details
    Browse the repository at this point in the history

  2. Makefile: enable ASAN for GCC 

    Previously `Makefile` and `Makefile.pkg-config` 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
    @cpu
    cpu committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    a097fa1 View commit details
    Browse the repository at this point in the history

  3. ci: verify ASAN presence/absence as appropriate 

    Presently only done on Linux since the test relies on `nm` being
available.
    @cpu
    cpu committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    4afce73 View commit details
    Browse the repository at this point in the history