Commit 877054f39c9eefd0416c4fe6bb4ccfd6b600d678

Patrick Steinhardt 2020-02-10T12:35:13

cmake: consolidate Valgrind option OpenSSL doesn't initialize bytes on purpose in order to generate additional entropy. Valgrind isn't too happy about that though, causing it to generate warninings about various issues regarding use of uninitialized bytes. We traditionally had some infrastructure to silence these errors in our OpenSSL stream implementation, where we invoke the Valgrind macro `VALGRIND_MAKE_MEMDEFINED` in various callbacks that we provide to OpenSSL. Naturally, we only include these instructions if a preprocessor define "VALGRIND" is set, and that in turn is only set if passing "-DVALGRIND" to CMake. We do that in our usual Azure pipelines, but we in fact forgot to do this in our nightly build. As a result, we get a slew of warnings for these nightly builds, but not for our normal builds. To fix this, we could just add "-DVALGRIND" to our nightly builds. But starting with commit d827b11b6 (tests: execute leak checker via CTest directly, 2019-06-28), we do have a secondary variable that directs whether we want to use memory sanitizers for our builds. As such, every user wishing to use Valgrind for our tests needs to pass both options "VALGRIND" and "USE_LEAK_CHECKER", which is cumbersome and error prone, as can be seen by our own builds. Instead, let's consolidate this into a single option, removing the old "-DVALGRIND" one. Instead, let's just add the preprocessor directive if USE_LEAK_CHECKER equals "valgrind" and remove "-DVALGRIND" from our own pipelines.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8765a97..7f001d3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -58,7 +58,6 @@ OPTION(USE_SHA1				"Enable SHA1. Can be set to CollisionDetection(ON)/HTTPS/Gene
 OPTION(USE_GSSAPI			"Link with libgssapi for SPNEGO auth"			OFF)
 OPTION(USE_STANDALONE_FUZZERS		"Enable standalone fuzzers (compatible with gcc)"	OFF)
 OPTION(USE_LEAK_CHECKER			"Run tests with leak checker"				OFF)
-OPTION(VALGRIND				"Configure build for valgrind"				OFF)
 OPTION(DEBUG_POOL			"Enable debug pool allocator"				OFF)
 OPTION(ENABLE_WERROR			"Enable compilation with -Werror"			OFF)
 OPTION(USE_BUNDLED_ZLIB    		"Use the bundled version of zlib"			OFF)
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 2b593dd..2575f47 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -19,7 +19,7 @@ jobs:
       environmentVariables: |
        CC=gcc
        CMAKE_GENERATOR=Ninja
-       CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DVALGRIND=on -DUSE_GSSAPI=ON
+       CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DUSE_GSSAPI=ON
        GITTEST_NEGOTIATE_PASSWORD=${{ variables.GITTEST_NEGOTIATE_PASSWORD }}
 
 - job: linux_amd64_xenial_gcc_mbedtls
@@ -35,7 +35,7 @@ jobs:
       environmentVariables: |
        CC=gcc
        CMAKE_GENERATOR=Ninja
-       CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DVALGRIND=on -DUSE_GSSAPI=ON
+       CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DUSE_GSSAPI=ON
        GITTEST_NEGOTIATE_PASSWORD=${{ variables.GITTEST_NEGOTIATE_PASSWORD }}
 
 - job: linux_amd64_xenial_clang_openssl
@@ -51,7 +51,7 @@ jobs:
       environmentVariables: |
        CC=clang
        CMAKE_GENERATOR=Ninja
-       CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DVALGRIND=on -DUSE_GSSAPI=ON
+       CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DUSE_GSSAPI=ON
        GITTEST_NEGOTIATE_PASSWORD=${{ variables.GITTEST_NEGOTIATE_PASSWORD }}
 
 - job: linux_amd64_xenial_clang_mbedtls
@@ -67,7 +67,7 @@ jobs:
       environmentVariables: |
        CC=clang
        CMAKE_GENERATOR=Ninja
-       CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DREGEX_BACKEND=pcre -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DVALGRIND=on -DUSE_GSSAPI=ON
+       CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DREGEX_BACKEND=pcre -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DUSE_GSSAPI=ON
        GITTEST_NEGOTIATE_PASSWORD=${{ variables.GITTEST_NEGOTIATE_PASSWORD }}
 
 - job: macos
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 9e4a41a..bcc6a01 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -288,14 +288,13 @@ IF (WIN32 AND NOT CYGWIN)
 ELSEIF (AMIGA)
 	ADD_DEFINITIONS(-DNO_ADDRINFO -DNO_READDIR_R -DNO_MMAP)
 ELSE()
-	ADD_FEATURE_INFO(valgrind VALGRIND "valgrind hints")
-	IF (VALGRIND)
-		ADD_DEFINITIONS(-DVALGRIND)
-	ENDIF()
-
 	FILE(GLOB SRC_OS unix/*.c unix/*.h)
 ENDIF()
 
+IF (USE_LEAK_CHECKER STREQUAL "valgrind")
+	ADD_DEFINITIONS(-DVALGRIND)
+ENDIF()
+
 FILE(GLOB SRC_GIT2 *.c *.h
 	allocators/*.c allocators/*.h
 	streams/*.c streams/*.h