WIP: ci: a sanitizer lane for the checks that offer --sanitize

dyst commented

2026-11-12 23:38:41 +00:00

First-time contributor

Why

Eleven checks build the code they test with the address and undefined-behaviour sanitizers when given --sanitize, but nothing ran them that way, so a use after free or an overflow in that code passes CI by luck.

What changed

tests: a check says what turns its sanitizers on with a fourth declaration, RUN_ALL_SANITIZE = ["--sanitize"], beside its RUN_ALL_ARGS (test_low_vram_texture_mips.py declares ["--sanitize", "address,undefined"], as its option takes the list). The eleven checks that declare it: test_dump_readback, test_image_view_retry, test_import_buffer_binding, test_low_vram_texture_mips, test_program_cache_cleanup, test_recording_admission, test_sampler_admission, test_texel_buffer_ownership, test_texture_staging_publication, test_vulkan_clear_mip, test_vulkan_copy_span.
tests: tests/run_all.py reads RUN_ALL_SANITIZE in read_declarations (in DECLARATIONS; checked like the others, a non-empty list of strings), and --sanitize runs only the checks that declare it, with those arguments added in python_check: no C++ unit tests, no byte-compile, and no --write-times. On Windows (without --no-skip) it skips them with the reason "the sanitizer lane runs on Linux": llvm-mingw's ASan is a DLL runtime, and the checks' Linux flags (-no-pie) stop clang there.
tests: tests/test_run_all_declarations.py covers the declaration's shapes (a list; True or [] stop the run) and a run of the lane with one declaring check (given exactly its arguments) and one that isn't (not run).
ci: .github/workflows/source-regressions.yml gets a Linux job, Sanitizer lane (ASan, UBSan), with the same pinned checkouts as the Python job (the source, Vulkan-Headers v1.4.345, Python 3.12, g++ and clang), running python -I -B tests/run_all.py --sanitize --strict.

Flags and settings

None: no environment switch, menu row or ini key. The new tests/run_all.py --sanitize option and the RUN_ALL_SANITIZE declaration are the test runner's, described above.

Measurements

Nothing to measure in the game: the change is a test-runner option and a CI job, and no game or renderer code changes. What was run, on the GitHub version (NotRexed/Hookshot#2908):

The lane, in a Linux container (python:3.12-slim, Debian's g++, the Builder's Vulkan headers): run_all.py --sanitize --strict: 11 passed, 0 failed.
On Windows: the 11 are skipped with the reason.
tests/test_run_all_declarations.py: the declaration's shapes and the lane's run, as above.
Every check that reads the changed files (19), through run_all.py, rebased on GitHub's main of that day, Windows: 18 pass; test_spawn_emit fails the same way on main there.
On Longshot, this branch, locally: 18 of the 19 checks that read its files pass; test_spawn_emit fails the same way on Longshot's main on the same PC.

Testing

Tested: the runs under Measurements (on the GitHub version) and the game-free checks this description names. Checked but not measured: the lane in the Linux container (11 passed).

Not tested: the CI job on Longshot: it is a GitHub Actions workflow and Forgejo has no runner yet. The lane's runs recorded are the Linux container's and Windows' (skipped there).

Needs other testers: None

Screenshots

Nothing on screen.

Risk and rollback

Only run_all.py --sanitize and the new CI job run the lane; a plain run_all.py runs the same checks with the same arguments as before. A malformed RUN_ALL_SANITIZE in a test stops the run with its file and line, as the other declarations do. On Longshot the job does not run at all until Forgejo has a runner. Reverting the commit is enough.

Reviewer notes

Carried over from NotRexed/Hookshot#2908 with the move to Longshot: the same change on Longshot's main, with its rename applied. Its review and discussion are there.

The checks above were done on the GitHub version; this branch is the same change with the rename applied (the same files and line counts). Forgejo runs no checks, and the lane is a GitHub Actions workflow that Forgejo has no runner for yet: on Longshot, tests/run_all.py --sanitize runs it locally. Only the checks that read the changed files were run, not the whole suite. The dev-loop proposal's D3 (NotRexed/Hookshot#2636).

Issues

None on Longshot. See NotRexed/Hookshot#2636 (D3).

Checklist

For everyone

One topic.
The title is a Conventional Commit (type(scope): description).
Rebased on current main.
python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
Tested in the game, or what wasn't is named under Testing.
Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
Flags table filled in; README / docs/development.md switch reference updated.
Screenshots for a change that shows on screen.
A row in docs/tested-configurations.md for each measured run.
MKWVK_API_VERSION bumped (gpu/vk/vk_api.h's MkwVkHost or MkwVkRenderer changed).
The SPIR-V headers regenerated with glslang (a shader changed).
A game-free test added for the fix, failing on main and passing here.
A check under tests/ that reads changed source updated in this PR, not weakened.
Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.
Docs updated.

🤖 Generated with Claude Code

## Why Eleven checks build the code they test with the address and undefined-behaviour sanitizers when given `--sanitize`, but nothing ran them that way, so a use after free or an overflow in that code passes CI by luck. ## What changed - tests: a check says what turns its sanitizers on with a fourth declaration, `RUN_ALL_SANITIZE = ["--sanitize"]`, beside its `RUN_ALL_ARGS` (`test_low_vram_texture_mips.py` declares `["--sanitize", "address,undefined"]`, as its option takes the list). The eleven checks that declare it: `test_dump_readback`, `test_image_view_retry`, `test_import_buffer_binding`, `test_low_vram_texture_mips`, `test_program_cache_cleanup`, `test_recording_admission`, `test_sampler_admission`, `test_texel_buffer_ownership`, `test_texture_staging_publication`, `test_vulkan_clear_mip`, `test_vulkan_copy_span`. - tests: `tests/run_all.py` reads `RUN_ALL_SANITIZE` in `read_declarations` (in `DECLARATIONS`; checked like the others, a non-empty list of strings), and `--sanitize` runs only the checks that declare it, with those arguments added in `python_check`: no C++ unit tests, no byte-compile, and no `--write-times`. On Windows (without `--no-skip`) it skips them with the reason "the sanitizer lane runs on Linux": llvm-mingw's ASan is a DLL runtime, and the checks' Linux flags (`-no-pie`) stop clang there. - tests: `tests/test_run_all_declarations.py` covers the declaration's shapes (a list; `True` or `[]` stop the run) and a run of the lane with one declaring check (given exactly its arguments) and one that isn't (not run). - ci: `.github/workflows/source-regressions.yml` gets a Linux job, *Sanitizer lane (ASan, UBSan)*, with the same pinned checkouts as the Python job (the source, Vulkan-Headers v1.4.345, Python 3.12, g++ and clang), running `python -I -B tests/run_all.py --sanitize --strict`. ## Flags and settings None: no environment switch, menu row or ini key. The new `tests/run_all.py --sanitize` option and the `RUN_ALL_SANITIZE` declaration are the test runner's, described above. ## Measurements Nothing to measure in the game: the change is a test-runner option and a CI job, and no game or renderer code changes. What was run, on the GitHub version (NotRexed/Hookshot#2908): - The lane, in a Linux container (python:3.12-slim, Debian's g++, the Builder's Vulkan headers): `run_all.py --sanitize --strict`: 11 passed, 0 failed. - On Windows: the 11 are skipped with the reason. - `tests/test_run_all_declarations.py`: the declaration's shapes and the lane's run, as above. - Every check that reads the changed files (19), through `run_all.py`, rebased on GitHub's `main` of that day, Windows: 18 pass; `test_spawn_emit` fails the same way on `main` there. - On Longshot, this branch, locally: 18 of the 19 checks that read its files pass; `test_spawn_emit` fails the same way on Longshot's `main` on the same PC. ## Testing **Tested:** the runs under Measurements (on the GitHub version) and the game-free checks this description names. Checked but not measured: the lane in the Linux container (11 passed). **Not tested:** the CI job on Longshot: it is a GitHub Actions workflow and Forgejo has no runner yet. The lane's runs recorded are the Linux container's and Windows' (skipped there). **Needs other testers:** None ## Screenshots Nothing on screen. ## Risk and rollback Only `run_all.py --sanitize` and the new CI job run the lane; a plain `run_all.py` runs the same checks with the same arguments as before. A malformed `RUN_ALL_SANITIZE` in a test stops the run with its file and line, as the other declarations do. On Longshot the job does not run at all until Forgejo has a runner. Reverting the commit is enough. ## Reviewer notes Carried over from [NotRexed/Hookshot#2908](https://github.com/NotRexed/Hookshot/pull/2908) with the move to Longshot: the same change on Longshot's `main`, with its rename applied. Its review and discussion are there. The checks above were done on the GitHub version; this branch is the same change with the rename applied (the same files and line counts). Forgejo runs no checks, and the lane is a GitHub Actions workflow that Forgejo has no runner for yet: on Longshot, `tests/run_all.py --sanitize` runs it locally. Only the checks that read the changed files were run, not the whole suite. The dev-loop proposal's D3 ([NotRexed/Hookshot#2636](https://github.com/NotRexed/Hookshot/pull/2636)). ## Issues None on Longshot. See [NotRexed/Hookshot#2636](https://github.com/NotRexed/Hookshot/pull/2636) (D3). ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [ ] Rebased on current `main`. - [ ] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [ ] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [ ] Flags table filled in; README / docs/development.md switch reference updated. - [ ] Screenshots for a change that shows on screen. - [ ] A row in `docs/tested-configurations.md` for each measured run. - [ ] `MKWVK_API_VERSION` bumped (`gpu/vk/vk_api.h`'s `MkwVkHost` or `MkwVkRenderer` changed). - [ ] The SPIR-V headers regenerated with glslang (a shader changed). - [ ] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. - [ ] Docs updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dyst added 1 commit

2026-11-12 23:38:41 +00:00

ci: a sanitizer lane for the checks that offer --sanitize 674fcdc5bd

Eleven checks build their code with the address and undefined-behaviour
sanitizers when given --sanitize, but nothing ran them that way. A check
now declares what turns them on (RUN_ALL_SANITIZE = ["--sanitize"], or
the sanitizers to use for test_low_vram_texture_mips), and
`tests/run_all.py --sanitize` runs those checks, and only those, with it.
A new Linux job in source-regressions.yml runs the lane with --strict.
On Windows the lane is skipped with the reason (llvm-mingw's ASan is a
DLL runtime, and the checks' -no-pie stops clang there).

All 11 pass with --sanitize --strict in a Linux container (python:3.12,
Debian's g++). tests/test_run_all_declarations.py: the declaration's
shapes, and a run of the lane. The dev-loop proposal's D3 (#2636).

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

mooki referenced this pull request

2026-11-12 23:57:12 +00:00

Idea: opt-in hardware tests on self-hosted Forgejo runners #43

dyst changed title from ~~ci: a sanitizer lane for the checks that offer --sanitize~~ to WIP: ci: a sanitizer lane for the checks that offer --sanitize

2026-11-13 01:13:52 +00:00

dyst commented

2026-11-13 01:13:52 +00:00

Author

First-time contributor

Back to WIP under #48's process: this was carried over before the new template. Its description will be brought to the template and measured on Longshot (main against this PR, at named commits) when it's wanted for review; ask here if you'd like it sooner.

dyst commented

2026-11-13 01:58:18 +00:00

Author

First-time contributor

Description brought to Longshot's PR template (#48). Still WIP: its numbers are from the GitHub version, and it isn't rebased on current main; it gets measured on Longshot (main against this PR, at named commits) before it is marked ready.

dyst commented

2026-11-13 02:00:33 +00:00

Author

First-time contributor

Description moved to the template's Testing section and whole checklist (#72): what was tested, what wasn't, and what needs other testers (their test requests to open while this is WIP).

This pull request is marked as a work in progress.

This branch is out-of-date with the base branch

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u 'ci/sanitize-lane:dyst-ci/sanitize-lane'

git switch 'dyst-ci/sanitize-lane'

Rows
Columns

WIP: ci: a sanitizer lane for the checks that offer --sanitize #29

Why

What changed

Flags and settings

Measurements

Testing

Screenshots

Risk and rollback

Reviewer notes

Issues

Checklist

For everyone

If applicable

Checkout