Adds a MemorySanitizer build to CI, building on the msan-ci starting point.
What's here
USE_MSAN CMake option (CMakeLists.txt): disables the conflicting ASan/UBSan/TSan/libfuzzer targets, switches to lld, and propagates -fsanitize=memory to the shared library and fuzz_driver.
msan CI job (.gitea/workflows/ci.yml): installs clang/lld, downloads the prebuilt MSan-instrumented libc++/libc++abi/libunwind toolchain from MinIO, builds the project against it (-stdlib=libc++ + rpath to the instrumented libs), and runs the test suite under MSan with a 300s timeout.
build_msan_toolchain.sh: reproduces the instrumented toolchain tarball from LLVM runtimes (-DLLVM_USE_SANITIZER=MemoryWithOrigins).
The instrumented-stdlib environment is what makes MSan usable on this C++ codebase: with an uninstrumented libstdc++/libc++, the fuzz harness (std::ifstream, std::set, …) immediately produces false-positive use-of-uninitialized-value reports.
Fixes made on top of the starting point
Install mc in the msan job. Every other job installs the MinIO client for the test-result upload step, but the msan job didn't — so the upload step would fail once MinIO credentials are present (after merge). Added the same mc install line.
Avoid reading uninitialized end.p for point writes/reads (ConflictSet.cpp). For point writes/reads (end.len == 0), end.p is not part of the API contract and is left uninitialized by the C/C++ API smoke tests and the fuzz test driver. insertPointWritesOrSorted and check::Job::init (the count > 1 read state machine) unconditionally built a TrivialSpan from end.p, reading the uninitialized pointer even though it's never used for point operations. MSan reports this. Moved the end span construction into the range-write/range-read branches so end.p is only read when end.len > 0. Semantically identical.
Validation
cmake -DUSE_MSAN=ON configures cleanly and ninja builds all targets (verified locally with clang; the instrumented stdlib is amd64/MinIO-hosted so the full MSan run happens in CI).
The C and C++ API smoke tests run cleanly under MSan against the instrumented library after the end.p fix (the C++ harness tests can't be exercised locally because this sandbox lacks an instrumented libc++ and runs on aarch64; they run in the amd64 CI job against the instrumented toolchain).
Full non-MSan test suite passes (8548/8548 non-valgrind tests); the only local failures are valgrind tests, which fail because this aarch64 sandbox's valgrind doesn't support the CPU (environmental, not a regression).
Closes #66.
Adds a MemorySanitizer build to CI, building on the `msan-ci` starting point.
## What's here
- **`USE_MSAN` CMake option** (`CMakeLists.txt`): disables the conflicting ASan/UBSan/TSan/libfuzzer targets, switches to `lld`, and propagates `-fsanitize=memory` to the shared library and `fuzz_driver`.
- **`msan` CI job** (`.gitea/workflows/ci.yml`): installs clang/lld, downloads the prebuilt MSan-instrumented `libc++`/`libc++abi`/`libunwind` toolchain from MinIO, builds the project against it (`-stdlib=libc++` + rpath to the instrumented libs), and runs the test suite under MSan with a 300s timeout.
- **`build_msan_toolchain.sh`**: reproduces the instrumented toolchain tarball from LLVM runtimes (`-DLLVM_USE_SANITIZER=MemoryWithOrigins`).
The instrumented-stdlib environment is what makes MSan usable on this C++ codebase: with an uninstrumented `libstdc++`/`libc++`, the fuzz harness (`std::ifstream`, `std::set`, …) immediately produces false-positive use-of-uninitialized-value reports.
## Fixes made on top of the starting point
1. **Install `mc` in the msan job.** Every other job installs the MinIO client for the test-result upload step, but the msan job didn't — so the upload step would fail once MinIO credentials are present (after merge). Added the same `mc` install line.
2. **Avoid reading uninitialized `end.p` for point writes/reads** (`ConflictSet.cpp`). For point writes/reads (`end.len == 0`), `end.p` is not part of the API contract and is left uninitialized by the C/C++ API smoke tests and the fuzz test driver. `insertPointWritesOrSorted` and `check::Job::init` (the `count > 1` read state machine) unconditionally built a `TrivialSpan` from `end.p`, reading the uninitialized pointer even though it's never used for point operations. MSan reports this. Moved the `end` span construction into the range-write/range-read branches so `end.p` is only read when `end.len > 0`. Semantically identical.
## Validation
- `cmake -DUSE_MSAN=ON` configures cleanly and `ninja` builds all targets (verified locally with clang; the instrumented stdlib is amd64/MinIO-hosted so the full MSan run happens in CI).
- The C and C++ API smoke tests run cleanly under MSan against the instrumented library after the `end.p` fix (the C++ harness tests can't be exercised locally because this sandbox lacks an instrumented libc++ and runs on aarch64; they run in the amd64 CI job against the instrumented toolchain).
- Full non-MSan test suite passes (8548/8548 non-valgrind tests); the only local failures are valgrind tests, which fail because this aarch64 sandbox's valgrind doesn't support the CPU (environmental, not a regression).
Add build_msan_toolchain.sh to produce a tarball containing an
MSan-instrumented libc++/libc++abi/libunwind toolchain, and add an msan
CI job that downloads the tarball and runs the test suite under MSan.
Also add USE_MSAN CMake option that disables conflicting sanitizers,
switches to lld, and propagates -fsanitize=memory to the shared library
and fuzz_driver targets.
For point writes and point reads (end.len == 0), end.p is not part of
the API contract and callers may leave it uninitialized -- both the C
and C++ API smoke tests and the fuzz test driver do so. However,
insertPointWritesOrSorted and check::Job::init unconditionally built a
TrivialSpan from end.p, reading the uninitialized pointer even though
it is never used for point operations.
Move the end span construction into the range-write/range-read branches
so end.p is only read when end.len > 0. This is semantically identical
but removes the uninitialized reads that MemorySanitizer reports.
The msan job uploads test results to MinIO with `mc cp`, but unlike the
test/release/coverage jobs it never installed the mc binary. When
MinIO credentials are configured (i.e. after merge) the upload step
would fail. Install mc to match the other jobs.
weaselbot
requested review from andrew 2026-07-22 22:49:19 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
andrew
Closes #66.
Adds a MemorySanitizer build to CI, building on the
msan-cistarting point.What's here
USE_MSANCMake option (CMakeLists.txt): disables the conflicting ASan/UBSan/TSan/libfuzzer targets, switches tolld, and propagates-fsanitize=memoryto the shared library andfuzz_driver.msanCI job (.gitea/workflows/ci.yml): installs clang/lld, downloads the prebuilt MSan-instrumentedlibc++/libc++abi/libunwindtoolchain from MinIO, builds the project against it (-stdlib=libc+++ rpath to the instrumented libs), and runs the test suite under MSan with a 300s timeout.build_msan_toolchain.sh: reproduces the instrumented toolchain tarball from LLVM runtimes (-DLLVM_USE_SANITIZER=MemoryWithOrigins).The instrumented-stdlib environment is what makes MSan usable on this C++ codebase: with an uninstrumented
libstdc++/libc++, the fuzz harness (std::ifstream,std::set, …) immediately produces false-positive use-of-uninitialized-value reports.Fixes made on top of the starting point
Install
mcin the msan job. Every other job installs the MinIO client for the test-result upload step, but the msan job didn't — so the upload step would fail once MinIO credentials are present (after merge). Added the samemcinstall line.Avoid reading uninitialized
end.pfor point writes/reads (ConflictSet.cpp). For point writes/reads (end.len == 0),end.pis not part of the API contract and is left uninitialized by the C/C++ API smoke tests and the fuzz test driver.insertPointWritesOrSortedandcheck::Job::init(thecount > 1read state machine) unconditionally built aTrivialSpanfromend.p, reading the uninitialized pointer even though it's never used for point operations. MSan reports this. Moved theendspan construction into the range-write/range-read branches soend.pis only read whenend.len > 0. Semantically identical.Validation
cmake -DUSE_MSAN=ONconfigures cleanly andninjabuilds all targets (verified locally with clang; the instrumented stdlib is amd64/MinIO-hosted so the full MSan run happens in CI).end.pfix (the C++ harness tests can't be exercised locally because this sandbox lacks an instrumented libc++ and runs on aarch64; they run in the amd64 CI job against the instrumented toolchain).Pull request closed