Add ASan/TSan/Valgrind configs and GitHub Actions CI#21
Merged
Conversation
Add `--config=asan`, `--config=tsan` and `--config=valgrind` to .bazelrc, mirroring the configuration used by the dallison/co coroutines repository. Add a GitHub Actions workflow that builds and tests on ubuntu-latest, ubuntu-24.04-arm and macos-latest, plus dedicated sanitizer and Valgrind jobs on Linux. Bump coroutines to 3.3.1 to pick up the matching sanitizer / fiber cooperation hooks. Fix a heap-buffer-overflow in sockets_test caught by ASan: the UDP unicast and broadcast tests called std::strcmp on a Receive_buffer sized exactly TEST_DATA.size(), which has no NUL terminator. The preceding ASSERT_EQ on a string_view already validates the bytes fully, so the strcmp calls are redundant and have been removed.
- payload_buffer_test: zero backing storage with calloc so Hexdump does not read uninitialized padding under Memcheck. Tear down the resizable-buffer test with an explicit destructor plus free() after realloc, fixing ASan alloc-dealloc-mismatch from operator delete on malloc/realloc storage. - pipe_test: compare fixed-length pipe reads with std::string_view instead of ASSERT_STREQ/strcmp on non-NUL-terminated buffers. Skip the high-iteration OverFull stress tests under TSan where the sanitizer hits nested DEADLYSIGNAL on many fiber switches. - sockets_test: skip UDP broadcast and multicast loopback tests on Apple platforms where GitHub Actions macOS runners reject or drop those packets.
SmallBlockAllocSimple, SmallBlockAlloc, and SmallBlockAllocFree used placement-new into calloc storage but never destroyed the object or released the block, which Valgrind reported as definitely lost. Tear down with an explicit destructor and free().
PayloadBuffer's VectorPush, VectorReserve, VectorResize, Realloc, PrimeBitmapAllocator, and BitMapRun::Allocate all dereferenced pointers into the buffer (hdr, p) after Allocate/Realloc/AllocateBitMapRun calls that may have relocated the underlying buffer, leaving those pointers dangling. Save the offset before any allocation and re-derive the pointer afterward across all six call sites. Also drop a leftover write through the dangling hdr in VectorPush that slipped through a previous partial fix. Add VectorPushWithResize, VectorReserveWithResize, and VectorResizeWithResize regression tests that drive the resizer with a small (256-byte) resizable buffer and verify data integrity afterward. The existing tests all used fixed-size buffers and could not trigger the bug. Original fix by Damian Stulich <damjan.stulic@getcruise.com>.
dallison
added a commit
that referenced
this pull request
May 4, 2026
* Add ASan/TSan/Valgrind configs and GitHub Actions CI Add `--config=asan`, `--config=tsan` and `--config=valgrind` to .bazelrc, mirroring the configuration used by the dallison/co coroutines repository. Add a GitHub Actions workflow that builds and tests on ubuntu-latest, ubuntu-24.04-arm and macos-latest, plus dedicated sanitizer and Valgrind jobs on Linux. Bump coroutines to 3.3.1 to pick up the matching sanitizer / fiber cooperation hooks. Fix a heap-buffer-overflow in sockets_test caught by ASan: the UDP unicast and broadcast tests called std::strcmp on a Receive_buffer sized exactly TEST_DATA.size(), which has no NUL terminator. The preceding ASSERT_EQ on a string_view already validates the bytes fully, so the strcmp calls are redundant and have been removed. * Fix CI failures for ASan, TSan, Valgrind and macOS sockets - payload_buffer_test: zero backing storage with calloc so Hexdump does not read uninitialized padding under Memcheck. Tear down the resizable-buffer test with an explicit destructor plus free() after realloc, fixing ASan alloc-dealloc-mismatch from operator delete on malloc/realloc storage. - pipe_test: compare fixed-length pipe reads with std::string_view instead of ASSERT_STREQ/strcmp on non-NUL-terminated buffers. Skip the high-iteration OverFull stress tests under TSan where the sanitizer hits nested DEADLYSIGNAL on many fiber switches. - sockets_test: skip UDP broadcast and multicast loopback tests on Apple platforms where GitHub Actions macOS runners reject or drop those packets. * Free backing storage in small-block PayloadBuffer tests SmallBlockAllocSimple, SmallBlockAlloc, and SmallBlockAllocFree used placement-new into calloc storage but never destroyed the object or released the block, which Valgrind reported as definitely lost. Tear down with an explicit destructor and free(). * Fix VectorHeader/pointer invalidation after buffer relocation PayloadBuffer's VectorPush, VectorReserve, VectorResize, Realloc, PrimeBitmapAllocator, and BitMapRun::Allocate all dereferenced pointers into the buffer (hdr, p) after Allocate/Realloc/AllocateBitMapRun calls that may have relocated the underlying buffer, leaving those pointers dangling. Save the offset before any allocation and re-derive the pointer afterward across all six call sites. Also drop a leftover write through the dangling hdr in VectorPush that slipped through a previous partial fix. Add VectorPushWithResize, VectorReserveWithResize, and VectorResizeWithResize regression tests that drive the resizer with a small (256-byte) resizable buffer and verify data integrity afterward. The existing tests all used fixed-size buffers and could not trigger the bug. Original fix by Damian Stulich <damjan.stulic@getcruise.com>.
dallison
added a commit
that referenced
this pull request
May 4, 2026
* Add ASan/TSan/Valgrind configs and GitHub Actions CI Add `--config=asan`, `--config=tsan` and `--config=valgrind` to .bazelrc, mirroring the configuration used by the dallison/co coroutines repository. Add a GitHub Actions workflow that builds and tests on ubuntu-latest, ubuntu-24.04-arm and macos-latest, plus dedicated sanitizer and Valgrind jobs on Linux. Bump coroutines to 3.3.1 to pick up the matching sanitizer / fiber cooperation hooks. Fix a heap-buffer-overflow in sockets_test caught by ASan: the UDP unicast and broadcast tests called std::strcmp on a Receive_buffer sized exactly TEST_DATA.size(), which has no NUL terminator. The preceding ASSERT_EQ on a string_view already validates the bytes fully, so the strcmp calls are redundant and have been removed. * Fix CI failures for ASan, TSan, Valgrind and macOS sockets - payload_buffer_test: zero backing storage with calloc so Hexdump does not read uninitialized padding under Memcheck. Tear down the resizable-buffer test with an explicit destructor plus free() after realloc, fixing ASan alloc-dealloc-mismatch from operator delete on malloc/realloc storage. - pipe_test: compare fixed-length pipe reads with std::string_view instead of ASSERT_STREQ/strcmp on non-NUL-terminated buffers. Skip the high-iteration OverFull stress tests under TSan where the sanitizer hits nested DEADLYSIGNAL on many fiber switches. - sockets_test: skip UDP broadcast and multicast loopback tests on Apple platforms where GitHub Actions macOS runners reject or drop those packets. * Free backing storage in small-block PayloadBuffer tests SmallBlockAllocSimple, SmallBlockAlloc, and SmallBlockAllocFree used placement-new into calloc storage but never destroyed the object or released the block, which Valgrind reported as definitely lost. Tear down with an explicit destructor and free(). * Fix VectorHeader/pointer invalidation after buffer relocation PayloadBuffer's VectorPush, VectorReserve, VectorResize, Realloc, PrimeBitmapAllocator, and BitMapRun::Allocate all dereferenced pointers into the buffer (hdr, p) after Allocate/Realloc/AllocateBitMapRun calls that may have relocated the underlying buffer, leaving those pointers dangling. Save the offset before any allocation and re-derive the pointer afterward across all six call sites. Also drop a leftover write through the dangling hdr in VectorPush that slipped through a previous partial fix. Add VectorPushWithResize, VectorReserveWithResize, and VectorResizeWithResize regression tests that drive the resizer with a small (256-byte) resizable buffer and verify data integrity afterward. The existing tests all used fixed-size buffers and could not trigger the bug. Original fix by Damian Stulich <damjan.stulic@getcruise.com>.
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.
Summary
--config=asan,--config=tsan, and--config=valgrindto.bazelrc, mirroring the configuration used by the dallison/co coroutines repository.ubuntu-latest,ubuntu-24.04-arm, andmacos-latest(with--config=apple_silicon).ubuntu-latest.ubuntu-latest.bazel-testlogson failure.coroutinesto3.3.1to pick up the matching sanitizer / fiber-cooperation hooks.sockets_test: the UDP unicast and broadcast tests calledstd::strcmpon aReceive_buffersized exactlyTEST_DATA.size()(no NUL terminator). The precedingASSERT_EQon astring_viewalready validates the bytes fully, so the redundantstrcmpcalls were removed.Test plan
Locally on macOS arm64 with the new configs:
bazel test //... --config=apple_silicon— all 6 tests passbazel test //... --config=apple_silicon --config=asan— all 6 tests pass (previously caught the unicast strcmp overflow now fixed)bazel test //... --config=tsanon Linux (CI) — TSan on macOS arm64 hits a known runtime CHECK insanitizer_allocator_secondary.hwhen freeing coroutine fiber stacks; restricting TSan to Linux in CI mirrors what the coroutines repo does.bazel test //... --config=valgrindon Linux (CI) — Valgrind is not available on macOS arm64.CI on this PR will exercise the Linux paths (TSan, Valgrind, plus the plain build/test on Ubuntu x86, Ubuntu arm, and macOS).