Re: [PATCH v2] ci: add address and undefined sanitizer tasks

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Oct 10 2022, Junio C Hamano wrote:

> The signal of "passed with asan, but not ubsan" (or vice versa) is
> not that useful in practice, run both santizers in a single task.
>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Range-diff against v1:
> 1:  04a9dc5439 ! 1:  cbf0d80ab1 ci: add address and undefined sanitizer tasks
>     @@ Metadata
>       ## Commit message ##
>          ci: add address and undefined sanitizer tasks
>      
>     +    The signal of "passed with asan, but not ubsan" (or vice versa) is
>     +    not that useful in practice, run both santizers in a single task.
>     +
>     +    Helped-by: Jeff King <peff@peff.net>
>          Signed-off-by: Junio C Hamano <gitster@pobox.com>
>      
>       ## .github/workflows/main.yml ##
>     @@ .github/workflows/main.yml: jobs:
>                 - jobname: linux-leaks
>                   cc: gcc
>                   pool: ubuntu-latest
>     -+          - jobname: linux-address
>     -+            cc: gcc
>     -+            pool: ubuntu-latest
>     -+          - jobname: linux-undefined
>     ++          - jobname: linux-sanitize
>      +            cc: gcc
>      +            pool: ubuntu-latest
>           env:
>     @@ ci/lib.sh: linux-leaks)
>       	export GIT_TEST_PASSING_SANITIZE_LEAK=true
>       	export GIT_TEST_SANITIZE_LEAK_LOG=true
>       	;;
>     -+linux-address | linux-undefined)
>     -+	export SANITIZE=${jobname#linux-}
>     ++linux-sanitize)
>     ++	export SANITIZE=address,undefined
>      +	;;
>       esac
>       
>
>  .github/workflows/main.yml | 3 +++
>  ci/lib.sh                  | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 831f4df56c..92d27db0b9 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -251,6 +251,9 @@ jobs:
>            - jobname: linux-leaks
>              cc: gcc
>              pool: ubuntu-latest
> +          - jobname: linux-sanitize
> +            cc: gcc
> +            pool: ubuntu-latest
>      env:
>        CC: ${{matrix.vector.cc}}
>        CC_PACKAGE: ${{matrix.vector.cc_package}}
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 1b0cc2b57d..c9c4982e21 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -278,6 +278,9 @@ linux-leaks)
>  	export GIT_TEST_PASSING_SANITIZE_LEAK=true
>  	export GIT_TEST_SANITIZE_LEAK_LOG=true
>  	;;
> +linux-sanitize)
> +	export SANITIZE=address,undefined
> +	;;
>  esac
>  
>  MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"

This LGTM, and all looks a bit familiar, even down to Jeff having the
same feedback on the v1:
https://lore.kernel.org/git/patch-1.1-e48b6853dd5-20220726T110716Z-avarab@gmail.com/
:)

FWIW I had a re-roll ready for that, but was waiting for the dust to
settle on failures. As soon as we were SANITIZE=address clean (which 2x
commits broke in quick succession) we had the scalar SANITIZE=undefined
failure, and then the release.

But this works too. It's the same as what I've had queued up locally,
except the job name is different, which was in response to other
feedback in the original thread:

https://github.com/git/git/compare/master...avar:git:avar/ci-add-SANITIZE%3Daddress-and-SANITIZE%3Dundefined-2