git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 0/5] building git with clang/gcc address sanitizer
Date: Mon, 10 Jul 2017 09:24:18 -0400	[thread overview]
Message-ID: <20170710132418.d6bvzxwvbejretb4@sigill.intra.peff.net> (raw)

I mentioned recently that I sometimes run the test suite with ASan, so
here are a few tweaks to make this easier (most of which I've been
carrying in my personal config.mak for a few years).

In my experience ASan does at least as well as valgrind at finding
memory bugs, and runs way faster. With this series, running:

  make SANITIZE=address test

takes about 4.5 minutes on my machine. A normal build+test is about 1.5
minutes on the same machine. I haven't timed a full run with --valgrind
recently, but I know that it is much much slower. :)

If you want to see it in action, you can do:

  make SANITIZE=address
  ./git log -g HEAD HEAD >/dev/null

which finds a bug I recently fixed (but the fix isn't in master yet).

There are other sanitizers, too. I _thought_ I had

  make SANITIZE=undefined test

running cleanly at one point, but it's definitely not clean now. Patch 5
helps a little by disabling unaligned loads in our get_be32() macros.
Once upon a time I had to set INTERNAL_QSORT, but it isn't necessary
anymore since everything goes through sane_qsort(). Most of the
remaining bugs are of a similar form, doing something like:

  memcpy(NULL, NULL, 0);

I don't know if it's worth having a sane_memcpy() there, or simply
tweaking the code to avoid the call (almost all of them are a single
call from apply.c:2870).

It looks like we also have a case of shifting off the left side of a
signed int, which is undefined.

You can also try:

  make SANITIZE=thread test

but it's not clean. I poked at some of the errors, and I don't think
there a problem in practice (though they may be worth cleaning up in the
name of code hygiene).

There's also:

  make SANITIZE=memory test

This is clang-only. It's supposed to find uses of uninitialized memory.
But it complains about writing out anything that zlib has touched. I
seem to recall that valgrind had a similar problem. I'm not sure what
zlib does to confuse these analyzers. For valgrind we were able to add a
suppression. We could probably do the same here, but I haven't looked
into it.

  [1/5]: test-lib: set ASAN_OPTIONS variable before we run git
  [2/5]: test-lib: turn on ASan abort_on_error by default
  [3/5]: Makefile: add helper for compiling with -fsanitize
  [4/5]: Makefile: turn off -fomit-frame-pointer with sanitizers
  [5/5]: Makefile: disable unaligned loads with UBSan

 Makefile      |  8 ++++++++
 t/test-lib.sh | 11 ++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

-Peff

             reply	other threads:[~2017-07-10 13:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-10 13:24 Jeff King [this message]
2017-07-10 13:24 ` [PATCH 1/5] test-lib: set ASAN_OPTIONS variable before we run git Jeff King
2017-07-10 13:24 ` [PATCH 2/5] test-lib: turn on ASan abort_on_error by default Jeff King
2017-07-10 17:18   ` Junio C Hamano
2017-07-10 13:24 ` [PATCH 3/5] Makefile: add helper for compiling with -fsanitize Jeff King
2017-07-10 17:35   ` Junio C Hamano
2017-07-10 17:44     ` Jeff King
2017-07-10 18:07       ` Junio C Hamano
2017-07-10 20:02       ` Ramsay Jones
2017-07-11  4:44         ` Jeff King
2017-07-10 13:24 ` [PATCH 4/5] Makefile: turn off -fomit-frame-pointer with sanitizers Jeff King
2017-07-10 13:24 ` [PATCH 5/5] Makefile: disable unaligned loads with UBSan Jeff King
2017-07-15 17:18   ` René Scharfe
2017-07-16 10:17     ` Jeff King
2017-07-16 11:02       ` René Scharfe
2017-07-10 13:30 ` [PATCH 0/5] building git with clang/gcc address sanitizer Jeff King
2017-07-10 14:40 ` Lars Schneider
2017-07-10 15:58   ` Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20170710132418.d6bvzxwvbejretb4@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).