Repository navigation
ci: add MemorySanitizer test leg for the Botan backend - #2549
Merged
Merged
Conversation
The sanitizer builds currently cover ASan/UBSan/LSan; the uninitialized-memory error class is not detected anywhere in CI. Add a weekly MSan job that builds a fully instrumented toolchain (libc++, Botan, zlib, bzip2) and runs the test suite against it, since MSan requires every linked C++ component to be instrumented to avoid false positives. Runs on schedule and on dispatch; PRs touching the workflow file also trigger it, so workflow changes are validated before they land.
libc++abi requires libunwind in LLVM_ENABLE_RUNTIMES; build it instrumented and static like the other runtimes.
bzip2's Makefile hardcodes CC=gcc, overriding the CC environment variable; pass it on the command line.
Botan's configure.py takes --cxxflags, not --cxx-flags.
Botan's CLI objects are compiled against the instrumented libc++ headers, so its executables must link with -stdlib=libc++ to resolve the libc++ ABI symbols.
The runtimes build does not statically include libc++abi in libc++.a by default, leaving __cxa/vtable ABI symbols unresolved in linked executables; link the ABI statically and add -lc++abi to the downstream link flags as well.
The first full instrumented run surfaced one third-party finding: Botan 3.13.0 reads the word-padding of the 521-bit (word-unaligned) field representation while constructing the secp521r1 precomputed base-mul table during static initialization, aborting any test process that first touches P-521. Run the suite with halt_on_error=0 so this known report does not mask the rest, scan the verbose log, and fail the step only on new findings. Also build the example binaries the CLI tests execute, exclude the two tests that run clean but take 30-50 minutes each under origin tracking, and raise the job timeout to 300 minutes.
Without -fsanitize-recover=memory, clang emits non-recoverable __msan_warning_noreturn calls and every report aborts the test process regardless of MSAN_OPTIONS; the full-suite run then fails ~20 tests, all on the known secp521r1 report. Compile every instrumented component with recoverable checks so reporting continues, and let the post-run scan remain the pass/fail gate even when ctest itself exits non-zero.
The secp521r1 MSan report reproduces in the rnp suite on x86-64 but not in a minimal Botan-only driver on arm64 (both 3.13.0 and master, built with the own --enable-sanitizers=memory mode, are clean). Run the same minimal driver natively on x86-64 to determine whether the report belongs to Botan or to the embedding build. Remove this job once the question is settled.
The known secp521r1 report (randombit/botan#6086) sits inside the BEEA inversion loop, so under recoverable checks it prints continuously and blew the job budget with hundreds of MB of log. Return to aborting on reports; ctest exits non-zero when the ~20 P-521-touching tests abort on the known finding, and the post-run scan stays the pass/fail gate, failing the step only on new reports. Also remove the temporary botan-repro job: the x86-64 reproduction was confirmed and filed upstream as randombit/botan#6086.
Contributor
Author
|
Two follow-ups for anyone watching this workflow's first scheduled run:
|
This file contains hidden or 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
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.
CI currently covers ASan/UBSan/LSan (sanitizer build modes, fuzzing smoke tests) — but no workflow detects the uninitialized-memory error class. This adds a MemorySanitizer leg for the Botan backend.
Why fully instrumented
MSan reports false positives when uninstrumented code writes memory that instrumented code later reads, so the job builds an instrumented stack first:
LLVM_USE_SANITIZER=MemoryWithOrigins, static onlyThe rnp build uses
-fsanitize=memory -fsanitize-memory-track-origins=2,-nostdinc++against the instrumented libc++ headers, and static linking throughout so no uninstrumented.sois loaded at test time.Triggers
Deliberately quiet: weekly schedule, workflow_dispatch, and pull requests touching this file (so workflow changes validate themselves before landing). No per-PR load otherwise.
Scope
rnp_testssuite minus the*_slowtimeout-regression drivers;cli_testsare excluded since they shell out to the system GnuPG binariesEstimated runtime ~40-60 min including dependency builds. The first PR-triggered run below is the validation run for the workflow itself.