From 3552ee46f6dc8e4e39d005d205da375c5e6d6277 Mon Sep 17 00:00:00 2001 From: monkey-w1n5t0n Date: Tue, 21 Jul 2026 12:08:52 +0200 Subject: [PATCH] fix(core): order RingBuffer::buf_ before the atomics to unblock the CI compiler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First real signal from the restored pipeline: with checkout fixed, the cpp-tests job got far enough to fail at `-Werror=stringop-overflow` in ring_buffer.hpp on GitHub's GCC 13, a failure that had been invisible behind the broken submodule checkout for a month. Reproduced locally against gcc 13.4.0 (local default is gcc 14, which does not fire). The warning is a false positive: GCC anchors the destination object to the member at offset 0 and reports `buf_[head & kMask]` as writing past `head_._M_i` (size 8) at offset [16, 268] — offsets that are precisely buf_[0..63] of a 64-entry, 4-byte ControlEvent array. Adding an explicit `__builtin_unreachable()` bound hint does not help, because the index range was never what GCC got wrong. Declaring buf_ first anchors the analysis correctly. Not a suppression, and behaviour-preserving: RingBuffer has exactly one production user (ModeBase::events_) and is never serialized, copied, or sent over a wire, so member order is unobservable. Reasoning recorded at the declaration so nobody "tidies" the order back. Verified: full ctest suite green under gcc 13.4.0 (the CI compiler) as well as gcc 14.2, and scripts/run-all-tests.sh ALL GREEN (parity 1273 floats within 1e-5, lint, 33 Playwright specs). --- nisps/core/ring_buffer.hpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/nisps/core/ring_buffer.hpp b/nisps/core/ring_buffer.hpp index 7853d37..59f04d9 100644 --- a/nisps/core/ring_buffer.hpp +++ b/nisps/core/ring_buffer.hpp @@ -71,10 +71,20 @@ class RingBuffer { private: static constexpr std::size_t kMask = N - 1u; + // `buf_` MUST stay ahead of the atomics. `buf_[head & kMask]` is provably + // in bounds, but GCC 12/13 -Wstringop-overflow anchors the destination + // object to whichever member sits at offset 0; with the atomics first it + // reports the write as "1 byte into a region of size 0" against + // `head_._M_i` (size 8) at offset [16, 268] — offsets that are in fact + // exactly buf_[0..63]. A false positive, but -Werror makes it fatal, and + // it only fires on the CI toolchain (GCC 14 locally does not). Ordering + // buf_ first anchors the analysis correctly. Nothing depends on this + // layout: RingBuffer is never serialized, copied, or sent over a wire. + // // Head/tail use std::size_t and rely on natural unsigned wrap. + T buf_[N]{}; std::atomic head_; std::atomic tail_; - T buf_[N]{}; }; } // namespace nisps