Re: [PATCH 3/5] Makefile: add helper for compiling with -fsanitize
- Date: Mon, 10 Jul 2017 11:07:38 -0700
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH 3/5] Makefile: add helper for compiling with -fsanitize
Jeff King <peff@xxxxxxxx> writes:
> On Mon, Jul 10, 2017 at 10:35:24AM -0700, Junio C Hamano wrote:
>> Jeff King <peff@xxxxxxxx> 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
> 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.