From d0cc5796f35941d5a6ccd05146911f102bed57c4 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Jul 2017 09:24:35 -0400 Subject: test-lib: set ASAN_OPTIONS variable before we run git We turn off ASan's leak detection by default in the test suite because it's too noisy. But we don't do so until part-way through test-lib. This is before we've run any tests, but after we do our initial "./git" to see if the binary has even been built. When built with clang, this seems to work fine. However, using "gcc -fsanitize=address", the leak checker seems to complain more aggressively: $ ./git ... ==5352==ERROR: LeakSanitizer: detected memory leaks Direct leak of 2 byte(s) in 1 object(s) allocated from: #0 0x7f120e7afcf8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1cf8) #1 0x559fc2a3ce41 in do_xmalloc /home/peff/compile/git/wrapper.c:60 #2 0x559fc2a3cf1a in do_xmallocz /home/peff/compile/git/wrapper.c:100 #3 0x559fc2a3d0ad in xmallocz /home/peff/compile/git/wrapper.c:108 #4 0x559fc2a3d0ad in xmemdupz /home/peff/compile/git/wrapper.c:124 #5 0x559fc2a3d0ad in xstrndup /home/peff/compile/git/wrapper.c:130 #6 0x559fc274535a in main /home/peff/compile/git/common-main.c:39 #7 0x7f120dabd2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0) This is a leak in the sense that we never free it, but it's in a global that is meant to last the whole program. So it's not really interesting or in need of fixing. And at any rate, mentioning leaks outside of the test_expect blocks is certainly unwelcome, as it pollutes stderr. Let's bump the setting of ASAN_OPTIONS higher in test-lib.sh to catch our initial "can we even run git?" test. While we're at it, we can add a comment to make it a bit less inscrutable. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/test-lib.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 30eb743719..c42791eb91 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -36,6 +36,14 @@ then fi GIT_BUILD_DIR="$TEST_DIRECTORY"/.. +# If we were built with ASAN, it may complain about leaks +# of program-lifetime variables. Disable it by default to lower +# the noise level. This needs to happen at the start of the script, +# before we even do our "did we build git yet" check (since we don't +# want that one to complain to stderr). +: ${ASAN_OPTIONS=detect_leaks=0} +export ASAN_OPTIONS + ################################################################ # It appears that people try to run tests without building... "$GIT_BUILD_DIR/git" >/dev/null @@ -148,9 +156,6 @@ else } fi -: ${ASAN_OPTIONS=detect_leaks=0} -export ASAN_OPTIONS - # Protect ourselves from common misconfiguration to export # CDPATH into the environment unset CDPATH -- cgit v1.2.3 From bf1ce904b753b76ac3e20dde68777ce4625b60ed Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Jul 2017 09:24:39 -0400 Subject: test-lib: turn on ASan abort_on_error by default By default, ASan will exit with code 1 when it sees an error. This means we'll notice a problem when we expected git to succeed, but not in a test_must_fail block. Let's ask it to actually raise SIGABRT instead. That will give us a signal death that test_must_fail will notice. As a bonus, it may also leave a coredump, which can be handy for digging into a failure. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index c42791eb91..fc3bc1989c 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -41,7 +41,7 @@ GIT_BUILD_DIR="$TEST_DIRECTORY"/.. # the noise level. This needs to happen at the start of the script, # before we even do our "did we build git yet" check (since we don't # want that one to complain to stderr). -: ${ASAN_OPTIONS=detect_leaks=0} +: ${ASAN_OPTIONS=detect_leaks=0:abort_on_error=1} export ASAN_OPTIONS ################################################################ -- cgit v1.2.3 From 56b5db30d0dc1e5d20fa28cbb6e76a5a932d812f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Jul 2017 09:24:42 -0400 Subject: Makefile: add helper for compiling with -fsanitize 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. With this patch, all of that happens automatically when you run: make SANITIZE=address test Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index ffa6da71b7..9fb2ff3e82 100644 --- a/Makefile +++ b/Makefile @@ -991,6 +991,10 @@ ifdef DEVELOPER CFLAGS += $(DEVELOPER_CFLAGS) endif +ifdef SANITIZE +BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE) +endif + ifndef sysconfdir ifeq ($(prefix),/usr) sysconfdir = /etc -- cgit v1.2.3 From ddbc8a6d3e15acba2e145fb9c8cd93c1531cc576 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Jul 2017 09:24:47 -0400 Subject: Makefile: turn off -fomit-frame-pointer with sanitizers The ASan manual recommends disabling this optimization, as it can make the backtraces produced by the tool harder to follow (and since this is a test-debug build, we don't care about squeezing out every last drop of performance). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 9fb2ff3e82..d03880e690 100644 --- a/Makefile +++ b/Makefile @@ -993,6 +993,7 @@ endif ifdef SANITIZE BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE) +BASIC_CFLAGS += -fno-omit-frame-pointer endif ifndef sysconfdir -- cgit v1.2.3 From 566cf0b3bd6458a74e8a3df8c01d27f35e7814f2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Jul 2017 09:24:50 -0400 Subject: Makefile: disable unaligned loads with UBSan The undefined behavior sanitizer complains about unaligned loads, even if they're OK for a particular platform in practice. It's possible that they _are_ a problem, of course, but since it's a known tradeoff the UBSan errors are just noise. Let's quiet it automatically by building with NO_UNALIGNED_LOADS when SANITIZE=undefined is in use. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index d03880e690..852dedc25c 100644 --- a/Makefile +++ b/Makefile @@ -994,6 +994,9 @@ endif ifdef SANITIZE BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE) BASIC_CFLAGS += -fno-omit-frame-pointer +ifeq ($(SANITIZE),undefined) +BASIC_CFLAGS += -DNO_UNALIGNED_LOADS +endif endif ifndef sysconfdir -- cgit v1.2.3