test: runtime sanitizers + static-analysis gate, and the bugs they found#37
Merged
Conversation
Adds runtime sanitizer verification, which matters more than static lint for this library - it packs bytes, swaps endianness, uses 48-bit bitfield offsets, and coordinates through shared-memory mutexes and atomics, none of which single-TU static analysis can see. - cmake/Sanitizers.cmake replaces ASAN.cmake: ENABLE_ASAN / ENABLE_UBSAN / ENABLE_TSAN (ASan+TSan mutually exclusive, UBSan composes, findings made fatal so CI can gate). - Presets unittests-asan (ASan+UBSan) and unittests-tsan, both non-LTO. - step_sanitizers.sh runs the suite under a sanitizer with halt-on-error. - CI gains a static-analysis job (clang-tidy + cppcheck, --werror) and a sanitizers matrix (asan, tsan). Container gets libubsan, libtsan, clang. Infrastructure only; findings fixed in follow-up commits. Signed-off-by: Jo5ta <jo5ta@mail.de>
The trace format packs multi-byte values at their natural byte offset with no padding, so reading or writing them through a typed pointer (*(uint64_t *)p) is an unaligned access - undefined behavior that UBSan flags and that faults on strict-alignment targets such as s390x. Writer (arguments.c) and decoder (file.hpp get<>, TracepointInternal get<>, Tracepoint span line, formatter dump size) now move the bytes with memcpy, which the compiler folds back to a single load/store where the architecture allows unaligned access. No format or behavior change. Signed-off-by: Jo5ta <jo5ta@mail.de>
unique_stack_close reset the handler but never released the lazily allocated in-memory lookup index, leaking it for any caller that closed a stack without going through tracebuffer deinit. close() now drops the index (it owns it), so the raw stack API is leak-free too. Signed-off-by: Jo5ta <jo5ta@mail.de>
The lazy fallback that resolves a tracepoint's file offset writes a static cache word; concurrent first-callers of the same tracepoint raced on it. The value is idempotent (a deterministic offset), so the reads and writes - in the tracepoint macros and in the startup batch registration - are now relaxed atomics: race-free with no ordering cost. Signed-off-by: Jo5ta <jo5ta@mail.de>
Genuine findings from clang-tidy/cppcheck: a dead store on the dump path, implicit-widening multiplications used as offsets (md5, tracepoint), else-after-return, a redundant member init, a non-const-worthy value param, %lu -> %zu for size_t, value-init of a returned struct on an error path, and a substr self-assignment. False positives (crc8 table index, dirfd after NULL-check, signed-char in a saturating cast) get scoped NOLINT with a reason. Signed-off-by: Jo5ta <jo5ta@mail.de>
- read byte-packed data with memcpy helpers instead of misaligned typed pointers (UBSan) in the file, arguments, meta, and definition tests - close the stack in the raw unique_stack tests so the index is freed (LeakSanitizer) - guard the fault-based death tests under TSan (which does not fault on OOB/UAF like ASan) via a new CLLTK_TSAN_ENABLED flag - drain the ringbuffer after joining writers in the overwhelmed stress test so the 'no pending' check is deterministic under TSan's slowdown - value-init test structs to avoid reading uninitialized members Signed-off-by: Jo5ta <jo5ta@mail.de>
- clang-tidy runs diff-only (changed lines) so the decoder's large pre-existing backlog does not block new work; cppcheck stays full-tree - silence GCC-only warning flags fed to clang-tidy, mark Boost headers SYSTEM, and scope suppressions to proven false positives / intentional patterns (stack_alloc alloca, section-symbol compare, noexcept startup inits, separate-binary ODR, test-only alloca) - drop the low-signal inconsistent-parameter-name check - TSan suppressions file for the two intentional test races - ASan: do not abort_on_error, so death tests observe the default exit(1) - CLLTK_TSAN_ENABLED test flag and the asan/tsan container packages Signed-off-by: Jo5ta <jo5ta@mail.de>
Behavior-preserving fixes surfaced by the new sanitizer legs (unaligned access, index leak, offset-cache race). Patch level - no file layout or API change. Signed-off-by: Jo5ta <jo5ta@mail.de>
Earlier formatting used a different clang-format version; reformat with the container's clang-format (21.1.8) so the format gate passes. Signed-off-by: Jo5ta <jo5ta@mail.de>
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
1 participant
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.
Stack 8/8 — based on the registration-index PR (#36).
Adds runtime sanitizers and a static-analysis gate, then fixes what they found.
Infrastructure
cmake/Sanitizers.cmake:ENABLE_ASAN/ENABLE_UBSAN/ENABLE_TSAN(ASan+TSan mutually exclusive, UBSan findings fatal). Presetsunittests-asan(ASan+UBSan) andunittests-tsan, plusstep_sanitizers.sh.sanitizersmatrix (asan, tsan) and astatic-analysisgate. clang-tidy runs diff-only (changed lines) so the decoder's large pre-existing backlog does not block new work; cppcheck runs full-tree and is clean.Real bugs the sanitizers found (behavior-preserving fixes, 1.7.1)
*(uint64_t *)pon unpadded offsets is UB and faults on strict-alignment targets like s390x. Nowmemcpy, which the compiler folds back to a single load/store where the arch allows it.unique_stack_closenever freed the lazily-allocated lookup index; it does now.Also: genuine static-analysis fixes across library/examples/tools; sanitizer-clean tests (memcpy reads, TSan guards on fault-based death tests, a deterministic drain in the overwhelmed stress test); scoped suppressions for proven false positives and intentional patterns.
Verified in the CI container: normal build 680/680, ASan+UBSan clean, TSan clean, static gate green. (Four
condition_variable::wait_fortests can't run under TSan on emulated aarch64 due to missing SVE support locally; they run on x86_64 CI.)