git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Oct 11, 2022, 08:23 UTC
Message-ID
<221011.86fsfupxx0.gmgdl@evledraar.gmail.com>
In-Reply-To
<xmqqpmezxl9p.fsf@gitster.g>
On Mon, Oct 10 2022, Junio C Hamano wrote:
Show 75 quoted lines
> 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
Previous: Jeff King
Message 9 of 9 in “ci: add address and undefined sanitizer tasks”
  1. ci: add address and undefined sanitizer tasksJunio C Hamano, Oct 9, 2022
  2. Junio C HamanoOct 10, 2022
  3. Jeff KingOct 11, 2022
  4. Junio C HamanoOct 11, 2022
  5. Junio C HamanoOct 21, 2022
  6. Jeff KingOct 21, 2022
  7. ci: add address and undefined sanitizer tasksJunio C Hamano, Oct 11, 2022
  8. Jeff KingOct 11, 2022
  9. Ævar Arnfjörð BjarmasonOct 11, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.