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

New issue

Open

opened 2026-08-18 01:15:48 +00:00 by bcox · 1 comment

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