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

Re: [PATCH 3/5] Makefile: add helper for compiling with -fsanitize

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 10, 2017, 18:07 UTC
Message-ID
<xmqq60f06u8l.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170710174454.jbkednmzwts7yr7z@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 44 quoted lines
> On Mon, Jul 10, 2017 at 10:35:24AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > You can already build and test with ASan by doing:
>> >
>> >   make CFLAGS=-fsanitize=address test
>> >
>> > but there are a few slight annoyances:
>> >
>> >   1. It's a little long to type.
>> >
>> >   2. It override your CFLAGS completely. You'd probably
>> >      still want -O2, for instance.
>> >
>> >   3. It's a good idea to also turn off "recovery", which
>> >      lets the program keep running after a problem is
>> >      detected (with the intention of finding as many bugs as
>> >      possible in a given run). Since Git's test suite should
>> >      generally run without triggering any problems, it's
>> >      better to abort immediately and fail the test when we
>> >      do find an issue.
>> 
>> Unfortunately I do not think Comparing between versions in
>> https://gcc.gnu.org/onlinedocs, it appears that -fsanitize-recover
>> is not configurable for folks still with GCC 4.x series, and this
>> patch is not very useful unless you disable the recovery for the
>> purpose of running our tests as you said X-<.
>
> I didn't actually dig into the history of gcc support at all. Back in
> the 4.x time-frame I tried using ASan and couldn't get it to work at
> all. I ended up just always building with clang (which from my
> mostly-ignorant view seems to to be the primary platform for ASan
> development).
>
> Since this is an optional build that doesn't need to be available
> everywhere, I'd actually be fine with saying "just use clang". But as
> far as I can tell, gcc seems to work fine these days. I consider this
> mostly a best-effort tool.
>
> I'm also not sure of the behavior without -fno-sanitize-recover. I think
> ASan may barf either way. The commit message for my config.mak from a
> year or two ago claims that the problem was actually with UBSan. It
> would be useful in the long run for that to work, too.

Yes. I'd agree with all of the above. While copyediting my response, I somehow ended up removing one paragraph before that "Unfortunately" by accident X-<, but the paragraph said essentially the same "this is optional so it is a strict improvement, and I do agree recovery must be disabled to be useful in our context".

Sorry for a possible confusion.
Previous: Jeff KingNext: Ramsay Jones
Message 6 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.