Web lists-archives.com

[PATCH 0/5] building git with clang/gcc address sanitizer

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(-)