Merge bitcoin#846: ci: Run ASan/LSan and reorganize sanitizer and Valgrind jobs

02dcea1 ci: Make test iterations configurable and tweak for sanitizer builds (Tim Ruffing)
489ff5c tests: Treat empty SECP2561_TEST_ITERS as if it was unset (Tim Ruffing)
fcfcb97 ci: Simplify to use generic wrapper for QEMU, Valgrind, etc (Tim Ruffing)
de4157f ci: Run ASan/LSan and reorganize sanitizer and Valgrind jobs (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    utACK 02dcea1
  jonasnick:
    ACK 02dcea1 spot-checked ci output, checked that when `valgrind ./tests` crashes then `LOG_COMPILER=valgrind make check` also crashes.

Tree-SHA512: 5f4a2fe186eca0b4ca29190eb18e20d0804934df614cdc8eb8cf0145ff36ded43194325572bb77eaaeba85c369f6effe69b7bdf7df97ba418d72cf36c9749a8c

Author: jonasnick

.cirrus.yml

@@ -5,7 +5,6 @@ env:
   ASM: no
   BUILD: check
   WITH_VALGRIND: yes
-  RUN_VALGRIND: no
   EXTRAFLAGS:
   HOST:
   ECDH: no
@@ -14,7 +13,8 @@ env:
   EXPERIMENTAL: no
   CTIMETEST: yes
   BENCH: yes
-  ITERS: 2
+  TEST_ITERS:
+  BENCH_ITERS: 2
   MAKEFLAGS: -j2
 
 cat_logs_snippet: &CAT_LOGS
@@ -63,27 +63,8 @@ task:
     - env: {BUILD: distcheck, WITH_VALGRIND: no, CTIMETEST: no, BENCH: no}
     - env: {CPPFLAGS: -DDETERMINISTIC}
     - env: {CFLAGS: -O0, CTIMETEST: no}
-    - env:
-        CFLAGS:  "-fsanitize=undefined -fno-omit-frame-pointer"
-        LDFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer"
-        UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
-        ASM: x86_64
-        ECDH: yes
-        RECOVERY: yes
-        EXPERIMENTAL: yes
-        SCHNORRSIG: yes
-        CTIMETEST: no
     - env: { ECMULTGENPRECISION: 2 }
     - env: { ECMULTGENPRECISION: 8 }
-    - env:
-        RUN_VALGRIND: yes
-        ASM: x86_64
-        ECDH: yes
-        RECOVERY: yes
-        EXPERIMENTAL: yes
-        SCHNORRSIG: yes
-        EXTRAFLAGS: "--disable-openssl-tests"
-        BUILD:
   matrix:
     - env:
         CC: gcc
@@ -181,9 +162,9 @@ task:
     cpu: 1
     memory: 1G
   env:
-    QEMU_CMD: qemu-s390x
+    WRAPPER_CMD: qemu-s390x
+    TEST_ITERS: 16
     HOST: s390x-linux-gnu
-    BUILD:
     WITH_VALGRIND: no
     ECDH: yes
     RECOVERY: yes
@@ -204,9 +185,9 @@ task:
     cpu: 1
     memory: 1G
   env:
-    QEMU_CMD: qemu-arm
+    WRAPPER_CMD: qemu-arm
+    TEST_ITERS: 16
     HOST: arm-linux-gnueabihf
-    BUILD:
     WITH_VALGRIND: no
     ECDH: yes
     RECOVERY: yes
@@ -228,9 +209,9 @@ task:
     cpu: 1
     memory: 1G
   env:
-    QEMU_CMD: qemu-aarch64
+    WRAPPER_CMD: qemu-aarch64
+    TEST_ITERS: 16
     HOST: aarch64-linux-gnu
-    BUILD:
     WITH_VALGRIND: no
     ECDH: yes
     RECOVERY: yes
@@ -249,9 +230,9 @@ task:
     cpu: 1
     memory: 1G
   env:
-    WINE_CMD: wine64-stable
+    WRAPPER_CMD: wine64-stable
+    TEST_ITERS: 16
     HOST: x86_64-w64-mingw32
-    BUILD:
     WITH_VALGRIND: no
     ECDH: yes
     RECOVERY: yes
@@ -262,3 +243,51 @@ task:
   test_script:
     - ./ci/cirrus.sh
   << : *CAT_LOGS
+
+# Sanitizers
+task:
+  container:
+    dockerfile: ci/linux-debian.Dockerfile
+    cpu: 1
+    memory: 1G
+  env:
+    ECDH: yes
+    RECOVERY: yes
+    EXPERIMENTAL: yes
+    SCHNORRSIG: yes
+    CTIMETEST: no
+    EXTRAFLAGS: "--disable-openssl-tests"
+  matrix:
+    - name: "Valgrind (memcheck)"
+      env:
+        # The `--error-exitcode` is required to make the test fail if valgrind found errors, otherwise it'll return 0 (https://www.valgrind.org/docs/manual/manual-core.html)
+        WRAPPER_CMD: "valgrind --error-exitcode=42"
+        TEST_ITERS: 16
+    - name: "UBSan, ASan, LSan"
+      env:
+        CFLAGS: "-fsanitize=undefined,address"
+        CFLAGS_FOR_BUILD: "-fsanitize=undefined,address"
+        UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
+        ASAN_OPTIONS: "strict_string_checks=1:detect_stack_use_after_return=1:detect_leaks=1"
+        LSAN_OPTIONS: "use_unaligned=1"
+        TEST_ITERS: 32
+  # Try to cover many configurations with just a tiny matrix.
+  matrix:
+    - env:
+        ASM: auto
+        STATICPRECOMPUTATION: yes
+    - env:
+        ASM: no
+        STATICPRECOMPUTATION: no
+        ECMULTGENPRECISION: 2
+  matrix:
+    - env:
+        CC: clang
+    - env:
+        HOST: i686-linux-gnu
+        CC: i686-linux-gnu-gcc
+  << : *MERGE_BASE
+  test_script:
+    - ./ci/cirrus.sh
+  << : *CAT_LOGS
+

ci/cirrus.sh

@@ -29,48 +29,23 @@ file *tests* || true
 file bench_* || true
 file .libs/* || true
 
-if [ -n "$BUILD" ]
-then
-    make "$BUILD"
-fi
+# This tells `make check` to wrap test invocations.
+export LOG_COMPILER="$WRAPPER_CMD"
 
-if [ "$RUN_VALGRIND" = "yes" ]
-then
-    # the `--error-exitcode` is required to make the test fail if valgrind found errors, otherwise it'll return 0 (https://www.valgrind.org/docs/manual/manual-core.html)
-    valgrind --error-exitcode=42 ./tests 16
-    valgrind --error-exitcode=42 ./exhaustive_tests
-fi
+# This limits the iterations in the tests and benchmarks.
+export SECP256K1_TEST_ITERS="$TEST_ITERS"
+export SECP256K1_BENCH_ITERS="$BENCH_ITERS"
 
-if [ -n "$QEMU_CMD" ]
-then
-    $QEMU_CMD ./tests 16
-    $QEMU_CMD ./exhaustive_tests
-fi
-
-if [ -n "$WINE_CMD" ]
-then
-    $WINE_CMD ./tests 16
-    $WINE_CMD ./exhaustive_tests
-fi
+make "$BUILD"
 
 if [ "$BENCH" = "yes" ]
 then
     # Using the local `libtool` because on macOS the system's libtool has nothing to do with GNU libtool
     EXEC='./libtool --mode=execute'
-    if [ -n "$QEMU_CMD" ]
-    then
-       EXEC="$EXEC $QEMU_CMD"
-    fi
-    if [ "$RUN_VALGRIND" = "yes" ]
-    then
-        EXEC="$EXEC valgrind --error-exitcode=42"
-    fi
-    if [ -n "$WINE_CMD" ]
+    if [ -n "$WRAPPER_CMD" ]
     then
-        EXEC="$WINE_CMD"
+        EXEC="$EXEC $WRAPPER_CMD"
     fi
-    # This limits the iterations in the benchmarks below to ITER iterations.
-    export SECP256K1_BENCH_ITERS="$ITERS"
     {
         $EXEC ./bench_ecmult
         $EXEC ./bench_internal

ci/linux-debian.Dockerfile

@@ -7,11 +7,12 @@ RUN dpkg --add-architecture arm64
 RUN apt-get update
 
 # dkpg-dev: to make pkg-config work in cross-builds
+# llvm: for llvm-symbolizer, which is used by clang's UBSan for symbolized stack traces
 RUN apt-get install --no-install-recommends --no-upgrade -y \
         git ca-certificates \
         make automake libtool pkg-config dpkg-dev valgrind qemu-user \
-        gcc clang libc6-dbg \
-        gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 \
+        gcc clang llvm libc6-dbg \
+        gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libubsan1:i386 libasan5:i386 \
         gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \
         gcc-arm-linux-gnueabihf libc6-dev-armhf-cross libc6-dbg:armhf \
         gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 \

src/tests.c

@@ -6471,7 +6471,7 @@ int main(int argc, char **argv) {
         count = strtol(argv[1], NULL, 0);
     } else {
         const char* env = getenv("SECP256K1_TEST_ITERS");
-        if (env) {
+        if (env && strlen(env) > 0) {
             count = strtol(env, NULL, 0);
         }
     }