On 7/26/2022 7:09 AM, Ævar Arnfjörð Bjarmason wrote: > Add CI targets for SANITIZE=address and SANITIZE=undefined. The former > would have caught a regression in 18bbc795fc5 (Merge branch > 'gc/bare-repo-discovery', 2022-07-22) which made its way to > "master"[1]. > > Per [2] the GitHub fork of git.git runs with these in CI, so it's > already useful to some forks of this repository. I'm a fan of adding additional sanitizer checks in our CI. Let's let computers do the work for us here instead of relying on humans. > Also per [2] we could use SANITIZE=address with some ASAN_OPTIONS > instead of our SANITIZE=leak job added in 956d2e4639b (tests: add a > test mode for SANITIZE=leak, run it in CI, 2021-09-23), but unifying > those two with these new jobs would be a lot harder, so let's leave > that for now. > - jobname: linux-leaks > cc: gcc > pool: ubuntu-latest > + - jobname: SANITIZE=address > + cc: gcc > + pool: ubuntu-latest > + - jobname: SANITIZE=undefined > + cc: gcc > + pool: ubuntu-latest > @@ -277,6 +277,12 @@ linux-leaks) > export SANITIZE=leak > export GIT_TEST_PASSING_SANITIZE_LEAK=true > ;; > +SANITIZE=address) > + export SANITIZE=address > + ;; > +SANITIZE=undefined) > + export SANITIZE=undefined > + ;; In both of these cases, we are breaking from the nearby pattern. These jobs could be renamed to linux-address and linux-undefined to match the linux-leaks job. Alternatively, we could rename linux-leaks to SANITIZE=leak, since the point is not to test the Linux platform but to use the additional runtime checks (and Linux is the fasted CI platform). Thanks, -Stolee