From 5df471fa5b8d4a5b1c8fe0d92992d9a9ea987558 Mon Sep 17 00:00:00 2001 From: itchyny Date: Tue, 4 Mar 2025 22:12:25 +0900 Subject: [PATCH] Disable Valgrind by default during testing (#3269) Previously, the configure script automatically enables Valgrind during testing when the valgrind command is found. However, running tests with Valgrind is quite slow, so it is better to disable it by default, and to enable it only when the user specifies the `--enable-valgrind` flag. --- .github/workflows/ci.yml | 4 ---- .github/workflows/oniguruma.yml | 2 ++ .github/workflows/scanbuild.yml | 2 +- .github/workflows/valgrind.yml | 1 + Dockerfile | 1 - Makefile.am | 23 +++++++++++------------ configure.ac | 13 +++---------- tests/setup | 2 +- 8 files changed, 19 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 92c5d859..7e188238 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,7 +88,6 @@ jobs: ./configure \ --host=${{ matrix.CC }} \ --disable-docs \ - --disable-valgrind \ --with-oniguruma=builtin \ --enable-static \ --enable-all-static \ @@ -154,7 +153,6 @@ jobs: ./configure \ --host="${{ matrix.target }}$(uname -r)" \ --disable-docs \ - --disable-valgrind \ --with-oniguruma=builtin \ --enable-static \ --enable-all-static \ @@ -230,7 +228,6 @@ jobs: autoreconf -i ./configure \ --disable-docs \ - --disable-valgrind \ --with-oniguruma=builtin \ --disable-shared \ --enable-static \ @@ -276,7 +273,6 @@ jobs: autoreconf -i ./configure \ --disable-docs \ - --disable-valgrind \ --with-oniguruma=builtin make distcheck make dist dist-zip diff --git a/.github/workflows/oniguruma.yml b/.github/workflows/oniguruma.yml index ebe471de..7c6c4e6d 100644 --- a/.github/workflows/oniguruma.yml +++ b/.github/workflows/oniguruma.yml @@ -22,6 +22,7 @@ jobs: autoreconf -i ./configure \ --disable-docs \ + --enable-valgrind \ --with-oniguruma=yes make -j"$(nproc)" file ./jq @@ -54,6 +55,7 @@ jobs: autoreconf -i ./configure \ --disable-docs \ + --enable-valgrind \ --with-oniguruma=no make -j"$(nproc)" file ./jq diff --git a/.github/workflows/scanbuild.yml b/.github/workflows/scanbuild.yml index 135842fb..56e72d7a 100644 --- a/.github/workflows/scanbuild.yml +++ b/.github/workflows/scanbuild.yml @@ -49,7 +49,7 @@ jobs: MAKEVARS: ${{ matrix.makevars }} run: | autoreconf -i - ./configure --with-oniguruma=builtin $COVERAGE + ./configure --with-oniguruma=builtin --enable-valgrind $COVERAGE scan-build --keep-going make -j4 - name: Test env: diff --git a/.github/workflows/valgrind.yml b/.github/workflows/valgrind.yml index 6d0a8557..e8e075cd 100644 --- a/.github/workflows/valgrind.yml +++ b/.github/workflows/valgrind.yml @@ -22,6 +22,7 @@ jobs: autoreconf -i ./configure \ --disable-docs \ + --enable-valgrind \ --with-oniguruma=builtin make -j"$(nproc)" file ./jq diff --git a/Dockerfile b/Dockerfile index bee3200b..17950b40 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,7 +19,6 @@ COPY . /app RUN autoreconf -i \ && ./configure \ --disable-docs \ - --disable-valgrind \ --with-oniguruma=builtin \ --enable-static \ --enable-all-static \ diff --git a/Makefile.am b/Makefile.am index c7504e41..08cb668f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -65,23 +65,18 @@ endif include_HEADERS = src/jv.h src/jq.h -if ENABLE_UBSAN -AM_CFLAGS += -fsanitize=undefined -endif - AM_CPPFLAGS = -I$(srcdir)/src -I$(srcdir)/vendor -### Running tests under Valgrind +### Address sanitizer (ASan) if ENABLE_ASAN AM_CFLAGS += -fsanitize=address -NO_VALGRIND = 1 -else -if ENABLE_VALGRIND -NO_VALGRIND = -else -NO_VALGRIND = 1 endif + +### Undefined Behavior Sanitizer + +if ENABLE_UBSAN +AM_CFLAGS += -fsanitize=undefined endif ### Code coverage with gcov @@ -127,7 +122,11 @@ TESTS = tests/mantest tests/jqtest tests/shtest tests/utf8test tests/base64test if !WIN32 TESTS += tests/optionaltest endif -AM_TESTS_ENVIRONMENT = JQ=$(abs_builddir)/jq NO_VALGRIND=$(NO_VALGRIND) + +AM_TESTS_ENVIRONMENT = JQ=$(abs_builddir)/jq +if ENABLE_VALGRIND +AM_TESTS_ENVIRONMENT += ENABLE_VALGRIND=1 +endif # This is a magic make variable that causes it to treat tests/man.test as a # DATA-type dependency for the check target. As a result, it will attempt to diff --git a/configure.ac b/configure.ac index 88dc60b3..a70a77a9 100644 --- a/configure.ac +++ b/configure.ac @@ -39,11 +39,6 @@ if test "$USE_MAINTAINER_MODE" = yes; then AC_CHECK_PROGS(LEX, flex lex) fi -dnl Check for valgrind -AC_CHECK_PROGS(valgrind_cmd, valgrind) -if test "x$valgrind_cmd" = "x" ; then - AC_MSG_WARN([valgrind is required to test jq.]) -fi AC_CHECK_FUNCS(memmem) AC_CHECK_HEADER("sys/cygwin.h", [have_cygwin=1;]) @@ -54,10 +49,9 @@ dnl Running tests with Valgrind is slow. It is faster to iterate on dnl code without Valgrind until tests pass, then enable Valgrind and dnl fix leaks. AC_ARG_ENABLE([valgrind], - AS_HELP_STRING([--disable-valgrind],[do not run tests under Valgrind])) + AS_HELP_STRING([--enable-valgrind],[enable Valgrind during testing])) -dnl Running tests with Valgrind is slow; address sanitizer (ASAN) is -dnl faster. +dnl Address sanitizer (ASan) AC_ARG_ENABLE([asan], AS_HELP_STRING([--enable-asan],[enable address sanitizer])) @@ -114,7 +108,7 @@ AS_IF([test "x$enable_decnum" != "xno"],[ AC_DEFINE([USE_DECNUM], 1, [Define to enable decnum support.]) ]) -AM_CONDITIONAL([ENABLE_VALGRIND], [test "x$enable_valgrind" != xno]) +AM_CONDITIONAL([ENABLE_VALGRIND], [test "x$enable_valgrind" = xyes]) AM_CONDITIONAL([ENABLE_ASAN], [test "x$enable_asan" = xyes]) AM_CONDITIONAL([ENABLE_UBSAN], [test "x$enable_ubsan" = xyes]) AM_CONDITIONAL([ENABLE_GCOV], [test "x$enable_gcov" = xyes]) @@ -291,7 +285,6 @@ AC_SUBST(onig_LDFLAGS) AM_CONDITIONAL([BUILD_ONIGURUMA], [test "x$build_oniguruma" = xyes]) AM_CONDITIONAL([WITH_ONIGURUMA], [test "x$with_oniguruma" != xno]) -AC_SUBST([BUNDLER], ["$bundle_cmd"]) AC_CONFIG_MACRO_DIRS([config/m4 m4]) AC_CONFIG_HEADERS([src/config.h]) diff --git a/tests/setup b/tests/setup index 935ab038..7e6dfe69 100755 --- a/tests/setup +++ b/tests/setup @@ -16,7 +16,7 @@ JQ=${JQ:-$JQBASEDIR/jq} LC_ALL=C export LC_ALL -if [ -z "${NO_VALGRIND-}" ] && which valgrind > /dev/null; then +if [ -n "${ENABLE_VALGRIND-}" ] && which valgrind > /dev/null; then VALGRIND="valgrind --error-exitcode=1 --leak-check=full \ --suppressions=$JQTESTDIR/onig.supp \ --suppressions=$JQTESTDIR/local.supp"