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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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.