From 59f06cc8df21b859ba790148e0c4b3f820fff504 Mon Sep 17 00:00:00 2001 From: Aduneer <249940941+Aduneer@users.noreply.github.com> Date: Wed, 19 Aug 2026 19:17:09 +0200 Subject: [PATCH] Build under the sanitizers and with -Werror in CI Every pull request so far was checked under -fsanitize=address,undefined by hand and came back clean. That is the argument for making it a job rather than a habit: the habit lives in a scratch file outside the repo, and the next contributor does not have it. CI becomes a four-way matrix -- both compilers against an optimised and a sanitised build -- because -O2 and the sanitizers do not see the same program. The optimised build is what ships; the sanitised one is the only thing that reports a read past the end of a buffer rather than returning whatever was next to it. Three details that are the whole point: * -fno-sanitize-recover=undefined. UBSan's default is to print the diagnostic and carry on, exiting 0, so without this the entry is decorative: a real finding would be a green build, and nobody reads the log of a job that passed. Verified by injecting an out-of-bounds read into count_label(): the optimised entry sails past it, and the sanitised entry stops with exit 2 and names src/text.cpp and the test that reached it. * -Werror is here and not in the Makefile. A warning should stop a change being merged, not stop a contributor building the project at all, so `make` on a clean checkout stays warm. * CXX and CXXFLAGS are set for the job rather than passed to each make. The Makefile takes both with ?=, which honours the environment, and the test and golden targets have to link against objects built with the same flags -- passing them one step at a time is how you get a link error against a sanitizer runtime that half the objects do not have. UBSAN_OPTIONS=print_stacktrace=1 so that a failure names the source line rather than an address. All four entries pass locally on gcc 16 and clang 22. CI runs gcc 13 and clang 18, which warn about different things, so this may well surface something the newer compilers stopped complaining about -- which is the entry doing its job on its first run rather than a reason to hold it back. --- .github/workflows/ci.yml | 40 +++++++++++++++++++++++++++++++++++++--- CHANGELOG.md | 13 +++++++++++++ README.md | 2 +- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4511a45..214d319 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,15 +9,49 @@ on: jobs: build-and-test: + # Two compilers times two build modes, so four jobs. + # # Both compilers, so a warning or a standard-library difference that only # one of them notices still fails the build. + # + # Both modes, because -O2 and the sanitizers do not see the same program: + # the optimised build is what ships, and the sanitised one is the only + # thing that reports a read past the end of a buffer rather than returning + # whatever was next to it. strategy: fail-fast: false matrix: cxx: [g++, clang++] + mode: [optimised, sanitised] + include: + # -Werror lives here rather than in the Makefile on purpose: a + # warning should stop a change being merged, not stop a contributor + # building the thing at all. `make` on a clean checkout stays warm. + - mode: optimised + cxxflags: -std=c++17 -O2 -Wall -Wextra -Werror + + # -fno-sanitize-recover=undefined is what makes this entry mean + # anything. UBSan's default is to print the diagnostic and carry on, + # exiting 0 -- so without it, a real finding is a green build and + # nobody reads the log of a job that passed. + - mode: sanitised + cxxflags: >- + -std=c++17 -O1 -g -Wall -Wextra -Werror + -fsanitize=address,undefined -fno-sanitize-recover=undefined + -fno-omit-frame-pointer runs-on: ubuntu-latest + # Set for the job rather than passed to each make: the Makefile takes both + # with ?=, and the test and golden targets have to be built with the same + # flags as the objects they link against, which is easy to get wrong one + # step at a time. + env: + CXX: ${{ matrix.cxx }} + CXXFLAGS: ${{ matrix.cxxflags }} + # Names the source line rather than an address when something does fire. + UBSAN_OPTIONS: print_stacktrace=1 + steps: - uses: actions/checkout@v4 @@ -25,10 +59,10 @@ jobs: run: ${{ matrix.cxx }} --version - name: Build - run: make CXX=${{ matrix.cxx }} + run: make - name: Run tests - run: make test CXX=${{ matrix.cxx }} + run: make test - name: Run golden end-to-end tests - run: make golden CXX=${{ matrix.cxx }} + run: make golden diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f90f84..70021d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,19 @@ detail, which is where the reasoning lives. the merge and after it, and applying only the difference. A deck whose history began before the log keeps it. +### Changed + +- **CI builds under the sanitizers and with `-Werror`,** as a four-way matrix of + both compilers against an optimised and a sanitised build. Every pull request + so far was checked under `-fsanitize=address,undefined` by hand and came back + clean, which is the argument for making it a job rather than a habit. + + `-Werror` is deliberately not in the Makefile: a warning should stop a change + being merged, not stop a contributor building the project at all. The + sanitised entry also compiles with `-fno-sanitize-recover=undefined`, without + which UBSan prints its diagnostic and exits 0 — so a real finding would have + been a green build that nobody reads the log of. + ## 0.2.0 — 2026-08-17 Audio, a second test suite, and a review screen that fits in a terminal. diff --git a/README.md b/README.md index ab7902e..5ee92ad 100644 --- a/README.md +++ b/README.md @@ -544,7 +544,7 @@ cannot drive an app that insists on a tty. ## Contributing -Issues and pull requests are welcome. `make check` should pass before you open one; CI runs both suites on gcc and clang. +Issues and pull requests are welcome. `make check` should pass before you open one; CI runs both suites on gcc and clang, in an optimised build and again under `-fsanitize=address,undefined`, all four with `-Werror`. Warnings are deliberately not errors in the Makefile itself, so a warning never stops you building — it stops the change being merged. Released versions and what changed in them are in [CHANGELOG.md](CHANGELOG.md).