git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Teng Long <dyroneteng@gmail.com>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 1/3] ci: use clang for ASan/UBSan checks
Date: Thu, 1 Jun 2023 14:03:51 -0400	[thread overview]
Message-ID: <20230601180351.GA4167886@coredump.intra.peff.net> (raw)
In-Reply-To: <20230601180220.GA4167745@coredump.intra.peff.net>

Both gcc and clang support the "address" and "undefined" sanitizers.
However, they may produce different results. We've seen at least two
real world cases where gcc missed a UBSan problem but clang found it:

  1. Clang's UBSan (using clang 14.0.6) found a string index that was
     subtracted to "-1", causing an out-of-bounds read (curiously this
     didn't trigger ASan, but that may be because the string was in the
     argv memory, not stack or heap). Using gcc (version 12.2.0) didn't
     find the same problem.

     Original thread:
     https://lore.kernel.org/git/20230519005447.GA2955320@coredump.intra.peff.net/

  2. Clang's UBSan (using clang 4.0.1) complained about pointer
     arithmetic with NULL, but gcc at the time did not. This was in
     2017, and modern gcc does seem to find the issue, though.

     Original thread:
     https://lore.kernel.org/git/32a8b949-638a-1784-7fba-948ae32206fc@web.de/

Since we don't otherwise have a particular preference for one compiler
over the other for this test, let's switch to the one that we think may
be more thorough.

Note that it's entirely possible that the two are simply _different_,
and we are trading off problems that gcc would find that clang wouldn't.
However, my subjective and anecdotal experience has been that clang's
sanitizer support is a bit more mature (e.g., I recall other oddities
around leak-checking where clang performed more sensibly).

Obviously running both and cross-checking the results would give us the
best coverage, but that's very expensive to run (and these are already
some of our most expensive CI jobs). So let's use clang as our best
guess, and we can re-evaluate if we get more data points.

Signed-off-by: Jeff King <peff@peff.net>
---
You can see that this catches the recent problem when merged with
'next':

  https://github.com/peff/git/actions/runs/5102807385/jobs/9172605638

 .github/workflows/main.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 30492eacdd..487ad31e66 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -274,10 +274,10 @@ jobs:
             cc: gcc
             pool: ubuntu-latest
           - jobname: linux-asan
-            cc: gcc
+            cc: clang
             pool: ubuntu-latest
           - jobname: linux-ubsan
-            cc: gcc
+            cc: clang
             pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
-- 
2.41.0.346.g8d12207a4f


  reply	other threads:[~2023-06-01 18:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01 18:02 [PATCH 0/3] ci sanitizer cleanups and performance improvements Jeff King
2023-06-01 18:03 ` Jeff King [this message]
2023-06-01 18:09 ` [PATCH 2/3] ci: run ASan/UBSan in a single job Jeff King
2023-06-01 18:10 ` [PATCH 3/3] ci: drop linux-clang job Jeff King
2023-06-03  1:35 ` [PATCH 0/3] ci sanitizer cleanups and performance improvements Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230601180351.GA4167886@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=dyroneteng@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).