You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Nothing verifies that a sanitized lane actually compiled with CBM_SANITIZED_BUILD defined. #1615 gave CBM_SANITIZED one spelling and a compiler-probe backstop, but the backstop only covers the three
sanitizers that announce themselves. For UBSan and trap-UBSan there is no probe and never can be, so the
build system is the sole source of truth — and a silent regression there is invisible: the suite still
passes, just against native timing budgets on an instrumented binary.
That is not hypothetical. It is exactly what happened twice:
trap-UBSan, recorded in the comment above SANITIZED_DEFINE (Makefile.cbm:88-96): the leg "once
ran NATIVE timing budgets on an instrumented binary".
Opening this per CONTRIBUTING.md ("Open an issue first — always"; build-system changes need prior
design discussion). Happy to implement once there is a decision on shape — nothing is written yet.
Why the existing mechanisms do not cover it
mechanism
covers
does not cover
SANITIZED_DEFINE (Makefile.cbm:93-96)
any lane keyed off $(SANITIZE)
a lane with its own sanitize variable — precisely the TSan gap
UBSan, trap-UBSan — no macro, no __has_feature bit exists
the test suite passing
nothing here
a native budget on an instrumented binary is looser to violate, not tighter; it mostly still passes
The third row is the important one. The failure mode is a test that passes for months and then flakes on
a loaded runner, attributed to whatever PR happened to be in flight.
It is not only about budgets any more
scripts/msan.sh:120-129 reasons from this macro while ruling out a cause:
CBM_THREAD_STACK_MB at 256 MiB and at 1024 MiB (the cap). The knob is verified compiled in
(CBM_SANITIZED_BUILD is defined for this lane) and the floor is applied in cbm_thread_create [...]
The fault address did not move by a single byte between 256 MiB and 1024 MiB — a 4x stack increase
changing nothing is what rules out "the stack is merely too small".
If the define were ever absent on that lane, cbm_thread_stack_floor would compile to return requested
(src/foundation/compat_thread.c:33-48), the 4x increase would have changed nothing because the knob was
inert, and a documented "do not repeat" conclusion would be wrong. The premise is asserted in a comment;
nothing checks it.
Proposed shape
Three small pieces, and no changes under .github/workflows/ at all — which is the point of this shape.
1. The binary reports what it was compiled with. A --build-config early-exit in tests/test_main.c, next to the existing --version (line ~796) and --list-suites (line ~861),
printing one line of key=value:
sanitized comes straight from CBM_SANITIZED, so it answers the only question that matters: what did
the compiler actually receive, not what the Makefile intended to send.
2. scripts/test.sh asserts it. This is the single choke point — every sanitized lane in _test.yml already goes through it:
test.sh already knows the answer: it tracks SANITIZE_GIVEN (:147), the --tsan mode (:98, :179-185)
and the ARM64 trap-UBSan default (:173-177). So it can compute the expectation from the flags it is about
to pass to make, run --build-config on the binary it just built, and fail loudly on mismatch, before the
suite runs:
ERROR: build config mismatch — this lane builds instrumented but the binary reports sanitized=0.
Every sanitized-budget branch compiled to its NATIVE value.
Check that this lane's flag set carries $(SANITIZED_DEFINE) or -DCBM_SANITIZED_BUILD=1.
Note the TSan mode must check $BUILD_DIR/test-runner-tsan, not test-runner.
3. scripts/msan.sh gets the same one-liner. It is the only lane that bypasses test.sh
(_test.yml:284 → Docker → msan.sh, running build/msan/test-runner). One check next to the build at msan.sh:66-68 — and it would turn the comment quoted above into something enforced.
What this would have caught, and how fast
The TSan gap would have failed test-tsan in seconds, at the build step, with a message naming the cause
— instead of surfacing as an unrelated-looking flaky spawn test across three PRs.
Limits, stated up front
It proves the define reached the compiler, not that the sanitizer is instrumenting. That was never
the failure mode, and -fsanitize= not working would fail far louder.
It adds one flag to a test-only binary. No production surface.
test-windows (_test.yml:398) runs unsanitized, so it asserts sanitized=0 — which is also worth
having: it catches the inverse mistake of leaking the define into a native lane.
Alternatives considered
#error in sanitized.h when a probe fires without the define. Rejected in fix(build): give sanitized-build detection one spelling and a backstop #1615, for reasons
that still hold: it breaks the binary instead of correcting it, and breaks an out-of-tree make CFLAGS_EXTRA=-fsanitize=address. It is also blind to UBSan, the case this issue is really about.
Env var (CBM_EXPECT_SANITIZED=1) plus an in-suite test. A lane that forgets the variable skips
the check silently — the same silence that caused the problem. Inverting the default (fail when unset)
breaks every ad-hoc local run.
Grep Makefile.cbm in CI for the define on each flag set. Verifies the intent, not the binary, and
would have missed nothing only by accident — a flag set can also be assembled in a script.
A make print-flags target compared in CI. Same weakness: it reports what make would pass, not
what the compiler received, and it needs the CI workflow edits this shape avoids.
Scope
Three files: tests/test_main.c, scripts/test.sh, scripts/msan.sh. Roughly 40-60 lines including the
error text. One issue, one PR, per CONTRIBUTING.md.
Found while doing #1615 and #1618; not blocking either.
Summary
Nothing verifies that a sanitized lane actually compiled with
CBM_SANITIZED_BUILDdefined. #1615 gaveCBM_SANITIZEDone spelling and a compiler-probe backstop, but the backstop only covers the threesanitizers that announce themselves. For UBSan and trap-UBSan there is no probe and never can be, so the
build system is the sole source of truth — and a silent regression there is invisible: the suite still
passes, just against native timing budgets on an instrumented binary.
That is not hypothetical. It is exactly what happened twice:
SANITIZED_DEFINE(Makefile.cbm:88-96): the leg "onceran NATIVE timing budgets on an instrumented binary".
0a163d4f:CFLAGS_TSANnever includedSANITIZED_DEFINE, so every sanitizedbudget compiled to its native value. It surfaced as
subprocess_run_spawn_failurefailing on a PRwhose diff could not possibly have caused it — twice on one SHA — and cost two more PRs (fix(config): self-heal an orphan managed marker on removal (#1558) #1590, fix(subprocess+tsan): widen the spawn-retry window where it was silently disabled #1595)
to chase down.
Opening this per
CONTRIBUTING.md("Open an issue first — always"; build-system changes need priordesign discussion). Happy to implement once there is a decision on shape — nothing is written yet.
Why the existing mechanisms do not cover it
SANITIZED_DEFINE(Makefile.cbm:93-96)$(SANITIZE)sanitized.h(#1615)__has_featurebit existsThe third row is the important one. The failure mode is a test that passes for months and then flakes on
a loaded runner, attributed to whatever PR happened to be in flight.
It is not only about budgets any more
scripts/msan.sh:120-129reasons from this macro while ruling out a cause:If the define were ever absent on that lane,
cbm_thread_stack_floorwould compile toreturn requested(
src/foundation/compat_thread.c:33-48), the 4x increase would have changed nothing because the knob wasinert, and a documented "do not repeat" conclusion would be wrong. The premise is asserted in a comment;
nothing checks it.
Proposed shape
Three small pieces, and no changes under
.github/workflows/at all — which is the point of this shape.1. The binary reports what it was compiled with. A
--build-configearly-exit intests/test_main.c, next to the existing--version(line ~796) and--list-suites(line ~861),printing one line of
key=value:sanitizedcomes straight fromCBM_SANITIZED, so it answers the only question that matters: what didthe compiler actually receive, not what the Makefile intended to send.
2.
scripts/test.shasserts it. This is the single choke point — every sanitized lane in_test.ymlalready goes through it:_test.ymltest-unixscripts/test.sh CC=… CXX=…(default ASan+UBSan)sanitized=1test-diagscripts/test.sh … SANITIZE="-fsanitize=address,undefined …"sanitized=1test-lsan-macosscripts/test.sh …(ASan+LSan)sanitized=1test-tsanscripts/test.sh --tsan …sanitized=1test.sh:173-177sanitized=1scripts/test.sh SANITIZE=sanitized=0test.shalready knows the answer: it tracksSANITIZE_GIVEN(:147), the--tsanmode (:98, :179-185)and the ARM64 trap-UBSan default (:173-177). So it can compute the expectation from the flags it is about
to pass to make, run
--build-configon the binary it just built, and fail loudly on mismatch, before thesuite runs:
Note the TSan mode must check
$BUILD_DIR/test-runner-tsan, nottest-runner.3.
scripts/msan.shgets the same one-liner. It is the only lane that bypassestest.sh(
_test.yml:284→ Docker →msan.sh, runningbuild/msan/test-runner). One check next to the build atmsan.sh:66-68— and it would turn the comment quoted above into something enforced.What this would have caught, and how fast
The TSan gap would have failed
test-tsanin seconds, at the build step, with a message naming the cause— instead of surfacing as an unrelated-looking flaky spawn test across three PRs.
Limits, stated up front
the failure mode, and
-fsanitize=not working would fail far louder.still self-heal ASan/TSan/MSan, so a new lane would have to be UBSan-only to slip through.
test-windows(_test.yml:398) runs unsanitized, so it assertssanitized=0— which is also worthhaving: it catches the inverse mistake of leaking the define into a native lane.
Alternatives considered
#errorinsanitized.hwhen a probe fires without the define. Rejected in fix(build): give sanitized-build detection one spelling and a backstop #1615, for reasonsthat still hold: it breaks the binary instead of correcting it, and breaks an out-of-tree
make CFLAGS_EXTRA=-fsanitize=address. It is also blind to UBSan, the case this issue is really about.CBM_EXPECT_SANITIZED=1) plus an in-suite test. A lane that forgets the variable skipsthe check silently — the same silence that caused the problem. Inverting the default (fail when unset)
breaks every ad-hoc local run.
Makefile.cbmin CI for the define on each flag set. Verifies the intent, not the binary, andwould have missed nothing only by accident — a flag set can also be assembled in a script.
make print-flagstarget compared in CI. Same weakness: it reports what make would pass, notwhat the compiler received, and it needs the CI workflow edits this shape avoids.
Scope
Three files:
tests/test_main.c,scripts/test.sh,scripts/msan.sh. Roughly 40-60 lines including theerror text. One issue, one PR, per
CONTRIBUTING.md.Found while doing #1615 and #1618; not blocking either.