Set up CI, with an opt lane and a sanitizer lane #1

New issue

Closed

opened 2026-08-18 01:15:48 +00:00 by bcox · 2 comments

bcox commented

2026-08-18 01:15:48 +00:00

Owner

There is no CI configuration in the repository — no .github, .forgejo,
.woodpecker or .drone. Everything is verified by running bazel test //...
by hand, at the default fastbuild configuration.

That gap has already hidden a real bug for an unknown length of time.

What it cost

//dsp:lfo_test failed under -c opt while passing at fastbuild. The cause
was a lifetime bug, not a numerical one: Lfo::NoteOn keeps a pointer to the
patch rather than copying it, and twelve call sites in the test passed a
temporary, so patch_ referred to dead stack for the rest of the note.
AddressSanitizer named it exactly — stack-use-after-scope.

It survived at -O0, where the stack slot happens to still hold the values,
and failed at -O3, where it does not. Nothing caught it because the suite
only ever ran at one optimisation level.

It also meant .bazelrc's stated invariant was false the whole time:

The DSP core must stay bit-reproducible: no FMA contraction, no fast-math
reassociation. A float reference that drifts per-optimisation-level is
useless as an oracle for the fixed-point implementation.

Fixed in 80db42a. The suite now passes at fastbuild, -c opt and
--config=opt, and NoteOn(Patch&&) is deleted on Lfo, Layer and
FmPairLayer so the same mistake is a compile error rather than a
configuration-dependent miscompile.

What is wanted

Three lanes, because each catches a different class:

bazel test //... — the default fastbuild lane.
bazel test //... --config=opt — this is the one that would have caught
the above. Worth treating as non-optional given the bit-reproducibility
claim the project makes about dsp/.
bazel test //... --copt=-fsanitize=address,undefined --copt=-fno-sanitize-recover=all --copt=-fsanitize-address-use-after-return=always --linkopt=-fsanitize=address,undefined — ASan is what actually named the
bug once the opt lane exposed it. UBSan is nearly free alongside it.

Notes for whoever picks this up:

The toolchain is hermetic (toolchains_llvm), so a runner needs only Bazel
and network access to fetch it; no system compiler or dev packages.
A clean --expunge build is expensive. Caching Bazel's repository cache
between runs is worth setting up early.
//tools:patch_editor links GLFW/OpenGL but is not run by any test, so a
headless runner can still bazel build it. It should be built, since it is
the one target the test suite does not cover.
The build is warning-free as of 27e06bf/08c4b3a, so --copt=-Werror
is worth considering for our own code — note .bazelrc deliberately
suppresses warnings for external/.

ThreadSanitizer is deliberately not in this list; see the separate issue.

There is no CI configuration in the repository — no `.github`, `.forgejo`, `.woodpecker` or `.drone`. Everything is verified by running `bazel test //...` by hand, at the default `fastbuild` configuration. That gap has already hidden a real bug for an unknown length of time. ## What it cost `//dsp:lfo_test` failed under `-c opt` while passing at `fastbuild`. The cause was a lifetime bug, not a numerical one: `Lfo::NoteOn` keeps a pointer to the patch rather than copying it, and twelve call sites in the test passed a temporary, so `patch_` referred to dead stack for the rest of the note. AddressSanitizer named it exactly — `stack-use-after-scope`. It survived at `-O0`, where the stack slot happens to still hold the values, and failed at `-O3`, where it does not. Nothing caught it because the suite only ever ran at one optimisation level. It also meant `.bazelrc`'s stated invariant was false the whole time: > The DSP core must stay bit-reproducible: no FMA contraction, no fast-math > reassociation. A float reference that drifts per-optimisation-level is > useless as an oracle for the fixed-point implementation. Fixed in `80db42a`. The suite now passes at `fastbuild`, `-c opt` and `--config=opt`, and `NoteOn(Patch&&)` is deleted on `Lfo`, `Layer` and `FmPairLayer` so the same mistake is a compile error rather than a configuration-dependent miscompile. ## What is wanted Three lanes, because each catches a different class: 1. **`bazel test //...`** — the default `fastbuild` lane. 2. **`bazel test //... --config=opt`** — this is the one that would have caught the above. Worth treating as non-optional given the bit-reproducibility claim the project makes about `dsp/`. 3. **`bazel test //... --copt=-fsanitize=address,undefined --copt=-fno-sanitize-recover=all --copt=-fsanitize-address-use-after-return=always --linkopt=-fsanitize=address,undefined`** — ASan is what actually named the bug once the opt lane exposed it. UBSan is nearly free alongside it. Notes for whoever picks this up: - The toolchain is hermetic (`toolchains_llvm`), so a runner needs only Bazel and network access to fetch it; no system compiler or dev packages. - A clean `--expunge` build is expensive. Caching Bazel's repository cache between runs is worth setting up early. - `//tools:patch_editor` links GLFW/OpenGL but is not run by any test, so a headless runner can still `bazel build` it. It should be built, since it is the one target the test suite does not cover. - The build is warning-free as of `27e06bf`/`08c4b3a`, so `--copt=-Werror` is worth considering for our own code — note `.bazelrc` deliberately suppresses warnings for `external/`. ThreadSanitizer is deliberately not in this list; see the separate issue.

bcox commented

2026-08-18 01:28:39 +00:00

Author

Owner

test comment

bcox added the

infrastructure

label

2026-08-18 01:34:39 +00:00

bcox referenced this issue

2026-08-28 23:04:32 +00:00

Add a ThreadSanitizer config to enforce the main-thread macro-name copy #2

bcox commented

2026-09-10 21:30:55 +00:00

Author

Owner

Not implementing this, but the lane design above needs one input it did not have: the default build is now -O3 (07a725d), where it was unoptimized when this was written.

That changes the relationship between lanes 1 and 2 specifically. Lane 2 is described here as "the one that would have caught the above" -- the //dsp:lfo_test stack-use-after-scope that survived at -O0 because the stack slot happened to still hold the values. Lane 1 now catches that class too, because lane 1 is optimized. The two lanes differ only by -c opt, and therefore only by NDEBUG.

Still worth running both, but for a different reason than the one written above: what lane 2 uniquely covers now is the NDEBUG build -- the eight runtime assert()s this tree relies on are compiled out there and live everywhere else, so lane 2 is the only lane that exercises the code a shipped bundle actually runs. That is a real difference and a smaller one than an optimization level.

The new gap, stated so whoever picks this up decides it deliberately rather than discovering it: no lane runs unoptimized any more. For the bug in this issue's own story that is a straight improvement, since -O0 is what hid it. It cuts the other way for UBSan, which sees undefined behavior an optimizer may have deleted before instrumentation -- a computation whose result is unused is the usual shape. --cxxopt=-O0 on the command line is how to get that view for a run, and a fourth lane pinning it is worth considering if the sanitizer lanes are meant to be exhaustive rather than fast.

Context on why the default moved: the suite ran about 30x slower unoptimized, which had tests being written around the sanitizer budget rather than around what they needed to measure -- see #61, which closed because its subject went from 665 s under asan to 11 s. The whole suite is now 28 s uncached at the default.

One practical note for the caching advice above: the sanitizer lanes share bazel's fastbuild output directory, so an asan run's testlogs are overwritten by the next default-config run. Anything that needs to read per-target timings or failures out of a lane has to do it before the next lane starts.

Not implementing this, but the lane design above needs one input it did not have: **the default build is now `-O3`** (`07a725d`), where it was unoptimized when this was written. That changes the relationship between lanes 1 and 2 specifically. Lane 2 is described here as "the one that would have caught the above" -- the `//dsp:lfo_test` `stack-use-after-scope` that survived at `-O0` because the stack slot happened to still hold the values. **Lane 1 now catches that class too**, because lane 1 is optimized. The two lanes differ only by `-c opt`, and therefore only by `NDEBUG`. Still worth running both, but for a different reason than the one written above: what lane 2 uniquely covers now is the `NDEBUG` build -- the eight runtime `assert()`s this tree relies on are compiled out there and live everywhere else, so lane 2 is the only lane that exercises the code a shipped bundle actually runs. That is a real difference and a smaller one than an optimization level. The new gap, stated so whoever picks this up decides it deliberately rather than discovering it: **no lane runs unoptimized any more.** For the bug in this issue's own story that is a straight improvement, since `-O0` is what hid it. It cuts the other way for UBSan, which sees undefined behavior an optimizer may have deleted before instrumentation -- a computation whose result is unused is the usual shape. `--cxxopt=-O0` on the command line is how to get that view for a run, and a fourth lane pinning it is worth considering if the sanitizer lanes are meant to be exhaustive rather than fast. Context on why the default moved: the suite ran about 30x slower unoptimized, which had tests being written around the sanitizer budget rather than around what they needed to measure -- see #61, which closed because its subject went from 665 s under asan to 11 s. The whole suite is now 28 s uncached at the default. One practical note for the caching advice above: the sanitizer lanes share bazel's fastbuild output directory, so an asan run's testlogs are overwritten by the next default-config run. Anything that needs to read per-target timings or failures out of a lane has to do it before the next lane starts.

bcox referenced this issue from a commit

2026-10-03 01:11:24 +00:00

ci: Forgejo Actions lanes for opt and the sanitizers (#1)

bcox referenced this issue from a pull request that will close it,

2026-10-03 01:11:24 +00:00

CI: Forgejo Actions lanes for opt and the sanitizers #106

bcox closed this issue

2026-10-03 01:15:11 +00:00