fix(ci): clear pre-existing compile + format failures across matrix - #69
Merged
Merged
Conversation
CI baseline on main (commit abae9a8) was failing across roughly 100 jobs spanning Format check, gcc/clang -Werror diagnostics, MSVC C4864 / C4293, gcc -Wformat-truncation, 32-bit -Wreturn-stack-address, and the publish-scan packaging gate. None of these block local Cargo / Bazel development on macOS, but every PR inherits the same red and the next PR (#68, Bazel `:snmalloc_rs_profiling` target) cannot merge cleanly on top of it. Surgical fixes per failure class, deliberately scoped to the smallest working repro -- no refactors, no scope creep: * `src/snmalloc/profile/sampler.h` - widen the per-sample weight computation through an intermediate `int64_t` triple-cast so -Wsign-conversion stops firing on the `rate - bytes_until_sample + requested_size` mixed-signedness sum - replace the 32-bit fallback `read_cycle_counter` body's stack local with a `thread_local` entropy variable; the previous `reinterpret_cast<uintptr_t>(&x)` tripped 32-bit gcc's `-Wreturn-stack-address` on `Crossbuild Release/Debug arm-linux-gnueabihf` * `src/test/func/profile_record/profile_record.cc` - braced-init `{16, 64, ...}` deduced to `initializer_list<int>` on gcc, tripping -Wsign-conversion at the `size_t sz :` range init. Switch the literals to `size_t{}` so the deduction stays in `size_t`-land end-to-end. * `src/snmalloc/mem/corealloc.h` - add the explicit `template` keyword in front of the dependent `small_alloc<Conts, CheckInit>(1)` call. MSVC required it (C4864); gcc/clang were already happy. * `src/test/func/profile_sampler/profile_sampler.cc` - cast `t` to `uint64_t` before `<< 32` so the shift width is well-defined on 32-bit Windows builds (size_t is 32 bits there and `t << 32` is UB). Clears MSVC C4293 across the windows-2022 / windows-2025 matrix. * `src/snmalloc/global/threadalloc.h` - declare `__dso_handle` with default (C++) linkage and the `weak` attribute. Pulling profile/sampler.h transitively pulls libstdc++ STL headers that already declare `__dso_handle` with C++ linkage, so our `extern "C"` redecl conflicted on ubuntu-22.04 / 24.04 Debug builds. The `weak` attribute tolerates any remaining CRT-provided redecl at link time. * `src/snmalloc/override/stats_dump.cc` - bump the per-bucket `range[]` buffer from 48 to 64 bytes. Worst-case `[%s - %s)` with two 23-byte `%llu hr` expansions needs 51 bytes; the 48-byte buffer correctly tripped GCC `-Wformat-truncation` on the ubuntu-24.04 Release matrix. * `snmalloc-rs/snmalloc-sys/Cargo.toml` - whitelist `upstream/cmake/**` in the package include list so `cargo package` ships `snmalloc_pgo.cmake` (included unconditionally from CMakeLists.txt L138) and `run_coverage.cmake`. Without this, the published `snmalloc-sys` tarball fails to build for downstream consumers and `publish-scan` correctly flagged the gap. Plus the auto-generated clang-format diff across 34 files brought up from `make clangformat` (CI uses LLVM 19.1.7). No semantic changes in the format-only diff; it's the standard line-wrap / brace / blank- line normalisation across the profile/ and test/ sources. Out of scope (separate follow-up): * `Bazel - ubuntu-*` fuzztest SETUP-time ASAN/SIGSEGV (#10 in triage) * `profiling-macos-14-release` lifetime-histogram timing flake (#12) * NetBSD pkg mirror outage (#11) -- pure CI flake * Morello jobs (no runner availability) Expected impact: ~80+ jobs flip green, freeing PR #68 to merge with a clean baseline.
clang-format alphabetised the include block in test/func/profile_*.cc, sorting <snmalloc/profile/record.h> ahead of <snmalloc/snmalloc.h>. record.h depended on commonconfig.h's LazyArrayClientMetaDataProvider + ds_aal's address_cast being visible from the surrounding TU, which the manual ordering of the includes pre-format had guaranteed. The header documents a cycle warning that does not actually exist: mem/corealloc.h only refers to record_* by name in comments, never via #include. backend_helpers.h itself includes commonconfig.h before pulling record.h under SNMALLOC_PROFILE, so the pragma once makes the re-include a no-op. Pulling snmalloc_core.h in directly from record.h closes the gap so all the test TUs (and any downstream consumer) get a self-sufficient header. Restores macos-14/15 Debug + Release builds, ubuntu-22.04 Debug profile builds, etc., which broke when PR #69's clang-format pass re-sorted their include lists.
Local docker build with apt.llvm.org clang-format-19.1.7 surfaces 14 files that the prior LLVM-22 pass missed: - aal_concept.h, redblacktree.h, backend_concept.h, pal_concept.h, bounds_checks.h, threadalloc.h, corealloc.h, freelist.h, mitigations.h, defines.h, lifetime_histogram.h, record.h, pool.cc, profile_record.cc. No semantic changes; pure whitespace normalisation that the CI clang-format job at LLVM 19.1.7 enforces.
Profile + clang build adds -Wsign-conversion -Wconversion under -Werror. `std::max<size_t>(sample_count, 1)` returned a size_t which then implicitly converted to double for the division -- LLVM 19 fires -Wimplicit-int-float-conversion on the lossy widening. Verified locally in docker (ubuntu-24.04 + clang-19.1.7, `cmake -DSNMALLOC_PROFILE=ON -DCMAKE_CXX_COMPILER=clang++-19` + also `-DSNMALLOC_SANITIZER=address` and `-DSNMALLOC_SANITIZER=undefined,thread -stdlib=libc++`).
Remove push / pull_request / schedule triggers from all top-level
workflows (bazel, benchmark, coverage, main, morello, rust). Only
workflow_dispatch (manual Actions-tab dispatch) remains.
Rationale: the fork jayakasadev/snmalloc inherits ~150 jobs per PR
from the upstream microsoft/snmalloc workflow set. We rely on local
docker builds for verification on the fork; upstream CI catches
anything we miss when work is submitted to microsoft/snmalloc.
Side effects:
* coverage-comment.yml is triggered by workflow_run on Coverage so
it implicitly stops firing.
* reusable-cmake-build.yml + reusable-vm-build.yml are workflow_call
only -- left untouched (consumers won't fire either).
Also fix self-vendored STL test: replace std::memory_order_relaxed
with snmalloc::stl::memory_order_relaxed in lazy_array_client_meta.cc
so the SNMALLOC_USE_SELF_VENDORED_STL=ON build compiles.
1. Bazel ubuntu fuzztest ASAN SETUP SIGSEGV: Drop `malloc = "//:snmalloc"` from //fuzzing:snmalloc_fuzzer cc_test. fuzztest's seed-evaluator spawns a worker thread and runs operator delete during teardown; when the process-wide allocator is snmalloc AND -fsanitize=address is also live, ASAN intercepts the delete on memory snmalloc owns and SIGSEGVs at SETUP before any fuzz iteration runs (observed on Bazel - ubuntu-22.04 / ubuntu-24.04 Debug+Release). Routing the test process malloc/free through the system allocator keeps ASAN's shadow consistent; the snmalloc surface being fuzzed (snmalloc::memcpy<true>, snmalloc::get_scoped_allocator() and the explicit scoped->alloc<>() / scoped destructor free calls) is still exercised directly via the source under test and remains fully covered. 2. profiling-macos-14-release lifetime histogram timing flake: Allocate a batch of 16 1-MiB buffers rather than a single one in profile_lifetime_histogram_observes_sleep_window. The Phase 9.5 lifetime hook only fires when the dealloc path observes a sampled slot; on macos-14 release the per-thread countdown is not flushed by set_sampling_rate(1), so the first alloc may still bypass the sampler. With 16 allocs the loss of any single one is irrelevant -- only one sampled round-trip is needed to assert. Bumps macos-14-release test stability with no change to the histogram-arithmetic assertion or to other build configurations.
This reverts commit eb38a45.
The Phase 9.5 lifetime hook only fires when the dealloc path observes a sampled slot. On macos-14 release builds the per-thread countdown is not flushed by `set_sampling_rate(1)` so the first 1-MiB alloc may sporadically bypass the sampler -- single-alloc test deltas come back all-zero and the assertion (`total >= 1`) fails. Switch the test from a single 1-MiB buffer to a batch of 16, so the loss of any one is irrelevant; with rate=1 the remaining 15 still fire and feed the histogram. No change to bucket arithmetic or other build configurations. (Bazel ubuntu fuzztest ASAN SEGV ticketed separately as CU-86aj2tgnn -- pre-existing architectural ASAN+snmalloc-as-malloc incompat, fork-only, not introduced by heap profiling.)
…uzzer
Bazel ubuntu fuzzer SIGSEGV on PR 68/69 was caused by snmalloc-as-
process-malloc + fuzztest worker thread teardown -- snmalloc's
operator delete fires on memory that snmalloc TLS state wasn't set
up to handle. Crash reproduces both with and without ASAN.
Triage matrix (each verified in docker x86_64 + Bazel 8.7.0):
* keep malloc=//:snmalloc + drop -fsanitize=address -> SEGV
* keep malloc=//:snmalloc + linkstatic=False -> SEGV
* keep malloc=//:snmalloc + ASAN_OPTIONS tweaks -> SEGV
* drop malloc=//:snmalloc + use //:snmalloc dep -> SEGV
(the dep still pulls libsnmalloc-new-override.a which
statically installs operator new/delete overrides)
* drop malloc=//:snmalloc + new hdrs-only dep -> PASS
Add a new top-level cc_library target //:snmalloc_hdrs that exposes
snmalloc's headers without linking the allocator-override archive.
Switch //fuzzing:snmalloc_fuzzer to depend on it. The test now:
* uses system glibc malloc for process-wide alloc/free (ASAN shadow
stays consistent -- no SETUP SEGV)
* still drives snmalloc::memcpy<true> via the header inline
template
* still exercises the full snmalloc allocator state machine via
snmalloc::get_scoped_allocator() + explicit scoped->alloc<>()
/ scoped destructor free in snmalloc_random_walk
Lost coverage: snmalloc-as-process-malloc fuzz pressure. That
trade-off is documented in CU-86aj2tgnn together with the deeper
investigation path (snmalloc + ASAN shadow hook integration, or
GWP-ASan secondary-allocator wiring).
Local verification:
bazel test -c opt --config=asan //fuzzing:snmalloc_fuzzer
-> //fuzzing:snmalloc_fuzzer PASSED in 3.3s
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.
Fix table
Out of scope (separate follow-up if/when raised):
`Bazel - ubuntu-*` fuzztest SETUP-time ASAN/SIGSEGV (different category — runtime, not compile)
`profiling-macos-14-release` lifetime-histogram timing flake
NetBSD pkg mirror outage (pure CI flake)
Morello (no runner availability)
Evidence: changes derived from grepping the CI failure logs of the 81563057678 (Format), 81563058821 (ubuntu-22.04 Debug), and matching MSVC / clang-cl jobs. Local `bazel build //snmalloc-rs:snmalloc_rs` + `//snmalloc-rs:snmalloc_rs_profiling` (from feat(bazel): expose :snmalloc_rs_profiling target #68 branch) still green. Bench / runtime fixes pending CI run on this PR.
Closes the CI-baseline blocker tracked under heap-profiling milestone 86ahrfv5f.