[PATCH 0/5] building git with clang/gcc address sanitizer
- Date: Mon, 10 Jul 2017 09:24:18 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: [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:
./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).
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
[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(-)