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

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

From
Jeff King <peff@peff.net>
Date
Jul 10, 2017, 13:24 UTC
Message-ID
<20170710132418.d6bvzxwvbejretb4@sigill.intra.peff.net>

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
Next: Jeff King
Message 1 of 18 in “building git with clang/gcc address sanitizer”
  1. 0/5 building git with clang/gcc address sanitizerJeff King, Jul 10, 2017
  2. 1/5 test-lib: set ASAN_OPTIONS variable before we run gitJeff King, Jul 10, 2017
  3. 3/5 Makefile: add helper for compiling with -fsanitizeJeff King, Jul 10, 2017
  4. Junio C HamanoJul 10, 2017
  5. Jeff KingJul 10, 2017
  6. Junio C HamanoJul 10, 2017
  7. Ramsay JonesJul 10, 2017
  8. Jeff KingJul 11, 2017
  9. 2/5 test-lib: turn on ASan abort_on_error by defaultJeff King, Jul 10, 2017
  10. Junio C HamanoJul 10, 2017
  11. 5/5 Makefile: disable unaligned loads with UBSanJeff King, Jul 10, 2017
  12. René ScharfeJul 15, 2017
  13. Jeff KingJul 16, 2017
  14. René ScharfeJul 16, 2017
  15. 4/5 Makefile: turn off -fomit-frame-pointer with sanitizersJeff King, Jul 10, 2017
  16. Jeff KingJul 10, 2017
  17. Lars SchneiderJul 10, 2017
  18. Jeff KingJul 10, 2017

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.