On 10/07/17 18:44, Jeff King wrote: > 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 > 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. Just FYI, I had a quick look at this tonight. I applied your patches to master, the tried 'make SANITIZE=address test', which worked fine. I then tried 'make SANITIZE=undefined test' and I had to control+C it after nearly two hours on one test! ;-) (somewhere in the t4xxx - unfortunately I overwrote the output file without thinking). [BTW I am on Linux Mint 18.2 x86_64, gcc version 5.4.0] After a quick look at the ./t0000-basic.sh test, I managed to get the test to complete (with 15 tests failing), with the following patch applied: -- >8 -- diff --git a/Makefile b/Makefile index 3c341b2a6..8e6433738 100644 --- a/Makefile +++ b/Makefile @@ -1016,7 +1016,7 @@ ifdef SANITIZE BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE) BASIC_CFLAGS += -fno-omit-frame-pointer ifeq ($(SANITIZE),undefined) -BASIC_CFLAGS += -DNO_UNALIGNED_LOADS +BASIC_CFLAGS += -DNO_UNALIGNED_LOADS -DSHA1DC_FORCE_ALIGNED_ACCESS endif endif diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c index 25eded139..3baddc636 100644 --- a/sha1dc/sha1.c +++ b/sha1dc/sha1.c @@ -118,6 +118,10 @@ #define SHA1DC_ALLOW_UNALIGNED_ACCESS #endif /*UNALIGNMENT DETECTION*/ +#if defined(SHA1DC_ALLOW_UNALIGNED_ACCESS) && defined(SHA1DC_FORCE_ALIGNED_ACCESS) +#undef SHA1DC_ALLOW_UNALIGNED_ACCESS +#endif + #define rotate_right(x,n) (((x)>>(n))|((x)<<(32-(n)))) #define rotate_left(x,n) (((x)<<(n))|((x)>>(32-(n)))) -------- Hmm, hopefully that is not whitespace damaged. ATB, Ramsay Jones
