Subject: Re: [PATCH] CI: add SANITIZE=[address|undefined] jobs

From: Derrick Stolee <[email-masked]>
Date: Tue, 26 Jul 2022 09:30:07 -0400

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