From 877054f39c9eefd0416c4fe6bb4ccfd6b600d678 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 10 Feb 2020 12:35:13 +0100 Subject: [PATCH] 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. --- CMakeLists.txt | 1 - azure-pipelines.yml | 8 ++++---- src/CMakeLists.txt | 9 ++++----- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8765a97b5..7f001d3d1 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 2b593dd2c..2575f475f 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 9e4a41a61..bcc6a0139 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