Run the test suite under ASan and UBSan in CI - #51
Merged
Conversation
nsparse deserializes binary index files that may be truncated or corrupt. The readers validate defensively and ~120 negative-path assertions cover that, but those assertions only prove an exception was thrown -- not that no out-of-bounds read, overflow or misaligned access happened first. For an in-process native library a memory error is much worse than an exception, and CI built Release with no sanitizers, so nothing checked. Add NSPARSE_ENABLE_SANITIZERS, which applies -fsanitize=address,undefined globally before any target exists. Global is required, not tidiness: an instrumented translation unit and an uninstrumented one disagree about libstdc++'s container annotations, which surfaces as container-overflow false positives rather than a link error. -fno-sanitize-recover=all goes with it, because UBSan otherwise prints and continues and the run still exits 0. UBSan carries as much weight as ASan here -- overflow and misalignment are what binary parsing produces -- but under GCC its null and nonnull-attribute checks make the address of a function template instantiation non-constant, which breaks abseil's constexpr hash-function dispatch in every translation unit that includes flat_hash_map. Those three are dropped on GCC; a null dereference still surfaces, as an ASan SEGV report. One per-PR job, at the generic optimization level. tests/CMakeLists.txt registers the per-ISA distance_kernel_equivalence_test binaries (and builds nsparse_avx2/nsparse_avx512 to link them) regardless of NSPARSE_OPT_LEVEL, so that single configuration already runs the vectorized kernels -- where tail-element OOB would live -- under ASan; a SIMD-configured job would only re-cover them. It runs on ubuntu-latest rather than the CI image the other Linux jobs use, because it needs the libasan/libubsan runtimes. All 639 tests pass as-is: no existing finding is being suppressed. Verified the instrumentation is live, not a silent no-op, with throwaway tests -- a heap-buffer overflow, a use-after-free and a signed overflow each aborted the run with a report. Default builds are unaffected: cmake/sanitizers.cmake returns before adding any flag when the option is OFF, which it is by default. No perf claim. A libFuzzer harness over the CSR/mmap readers is the natural follow-up. Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
chishui
requested review from
model-collapse,
yuye-aws and
zirui-song-18
as code owners
September 8, 2026 09:38
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
2 participants
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.
Resolves part 1 of #50. Adds
NSPARSE_ENABLE_SANITIZERS(defaultOFF) and one per-PR job buildingRelWithDebInfowith-fsanitize=address,undefined. All 639 tests pass as-is — nothing suppressed.Answering the open questions:
-DNSPARSE_SANITIZERS=<list>overrides the default set.tests/CMakeLists.txtregisters the per-ISAdistance_kernel_equivalence_testbinaries at everyNSPARSE_OPT_LEVEL, so the generic job already runs the vectorized kernels under ASan.-fno-sanitize-recover=all, or UBSan prints and still exits 0. UBSan'snull/nonnull checks are dropped under GCC — they break abseil'sconstexprhash dispatch; ASan still reports null derefs.Verified live, not a no-op: injected OOB/UAF/overflow each aborted with a report. Fork CI green.
I confirm my contribution is made under the terms of the Apache 2.0 license.