Merge BlockstreamResearch/secp256k1-zkp#304: ci: Backport MSan fixes for ctime_tests

b9a82b481f ci: Update macOS image (Hennadii Stepanov) a1be8ed1b1 ci: Silent Homebrew's noisy reinstall warnings (Hennadii Stepanov) fd259fe9ad ci: Add job with -fsanitize-memory-param-retval (Tim Ruffing) 76b3396516 configure: Move "experimental" warning to bottom (Tim Ruffing) e639e6caa9 autotools: Disable eager MSan in ctime_tests (Tim Ruffing) Pull request description: Cherry-picked the upstream fixes from bitcoin-core/secp256k1#1517 to align behaviour here and silence the false 'use-of-uninitialized-value' reports in ctime_tests under Clang ≥16. #### Result: - `ctime_tests` is clean under MSan. - Other tests are unchanged. - No API or behaviour changes. #### Commits cherry-picked: - [55e5d975db](55e5d975db), - [e1bef0961c](e1bef0961c), - [ebfb82ee2f](ebfb82ee2f) #### References: Upstream: https://github.com/bitcoin-core/secp256k1/pull/1517 ACKs for top commit: apoelstra: ACK b9a82b481fc35c5db3a127185ffada77fe107d46; successfully ran local tests jonasnick: ACK b9a82b481f Tree-SHA512: b6c9ae38b142c7fadf1157a1b25cdc8e85e6cd271222b950b1246f14daf85b072979d4f2ee8791c8b81b2f6a94ad1094fc8110bbd4ff19153de6a6efad2e3cc0
2025-09-19 17:58:29 +02:00
parent f92fd5cc91 b9a82b481f
commit 42e75b613b
3 changed files with 114 additions and 17 deletions
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -556,11 +556,18 @@ jobs:
      matrix:
        configuration:
          - env_vars:
+              CTIMETESTS: 'yes'
              CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -g'
          - env_vars:
              ECMULTGENPRECISION: 2
              ECMULTWINDOW: 2
+              CTIMETESTS: 'yes'
              CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -g -O3'
+          - env_vars:
+              # -fsanitize-memory-param-retval is clang's default, but our build system disables it
+              # when ctime_tests when enabled.
+              CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g'
+              CTIMETESTS: 'no'

    env:
      ECDH: 'yes'
@@ -576,7 +583,6 @@ jobs:
      ECDSAADAPTOR: 'yes'
      BPPP: 'yes'
      SCHNORRSIG_HALFAGG: 'yes'
-      CTIMETESTS: 'yes'
      CC: 'clang'
      SECP256K1_TEST_ITERS: 32
      ASM: 'no'
@@ -674,10 +680,10 @@ jobs:
        run: env
        if: ${{ always() }}

-  macos-native:
-    name: "x86_64: macOS Monterey"
+  x86_64-macos-native:
+    name: "x86_64: macOS Ventura, Valgrind"
    # See: https://github.com/actions/runner-images#available-images.
-    runs-on: macos-12 # Use M1 once available https://github.com/github/roadmap/issues/528
+    runs-on: macos-13

    env:
      CC: 'clang'
@@ -705,7 +711,7 @@ jobs:

      - name: Install Homebrew packages
        run: |
-          brew install automake libtool gcc
+          brew install --quiet automake libtool gcc
          ln -s $(brew --prefix gcc)/bin/gcc-?? /usr/local/bin/gcc

      - name: Install and cache Valgrind
@@ -733,6 +739,62 @@ jobs:
        run: env
        if: ${{ always() }}

+  arm64-macos-native:
+    name: "ARM64: macOS Sonoma"
+    # See: https://github.com/actions/runner-images#available-images.
+    runs-on: macos-14
+
+    env:
+      CC: 'clang'
+      HOMEBREW_NO_AUTO_UPDATE: 1
+      HOMEBREW_NO_INSTALL_CLEANUP: 1
+      WITH_VALGRIND: 'no'
+      CTIMETESTS: 'no'
+
+    strategy:
+      fail-fast: false
+      matrix:
+        env_vars:
+          - { WIDEMUL: 'int64',  RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' }
+          - { WIDEMUL: 'int128_struct', ECMULTGENPRECISION: 2, ECMULTWINDOW: 4 }
+          - { WIDEMUL: 'int128',                  ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes', CC: 'gcc' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes', CPPFLAGS: '-DVERIFY' }
+          - BUILD: 'distcheck'
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Install Homebrew packages
+        run: |
+          brew install --quiet automake libtool gcc
+          ln -s $(brew --prefix gcc)/bin/gcc-?? /usr/local/bin/gcc
+
+      - name: CI script
+        env: ${{ matrix.env_vars }}
+        run: ./ci/ci.sh
+
+      - run: cat tests.log || true
+        if: ${{ always() }}
+      - run: cat noverify_tests.log || true
+        if: ${{ always() }}
+      - run: cat exhaustive_tests.log || true
+        if: ${{ always() }}
+      - run: cat ctime_tests.log || true
+        if: ${{ always() }}
+      - run: cat bench.log || true
+        if: ${{ always() }}
+      - run: cat config.log || true
+        if: ${{ always() }}
+      - run: cat test_env.log || true
+        if: ${{ always() }}
+      - name: CI env
+        run: env
+        if: ${{ always() }}
+
  win64-native:
    name: ${{ matrix.configuration.job_name }}
    # See: https://github.com/actions/runner-images#available-images.
--- a/build-aux/m4/bitcoin_secp.m4
+++ b/build-aux/m4/bitcoin_secp.m4
@@ -45,6 +45,18 @@ fi
 AC_MSG_RESULT($has_valgrind)
 ])

+AC_DEFUN([SECP_MSAN_CHECK], [
+AC_MSG_CHECKING(whether MemorySanitizer is enabled)
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
+  #if defined(__has_feature)
+  #  if __has_feature(memory_sanitizer)
+  #    error "MemorySanitizer is enabled."
+  #  endif
+  #endif
+  ]])], [msan_enabled=no], [msan_enabled=yes])
+AC_MSG_RESULT([$msan_enabled])
+])
+
 dnl SECP_TRY_APPEND_CFLAGS(flags, VAR)
 dnl Append flags to VAR if CC accepts them.
 AC_DEFUN([SECP_TRY_APPEND_CFLAGS], [
--- a/configure.ac
+++ b/configure.ac
@@ -296,6 +296,20 @@ if test x"$enable_ctime_tests" = x"auto"; then
    enable_ctime_tests=$enable_valgrind
 fi

+print_msan_notice=no
+if test x"$enable_ctime_tests" = x"yes" && test x"$GCC" = x"yes"; then
+  SECP_MSAN_CHECK
+  # MSan on Clang >=16 reports unitialized memory in function parameters and return values, even if
+  # the uninitalized variable is never actually "used". This is called "eager" checking, and it's
+  # sounds like good idea for normal use of MSan. However, it yields many false positives in the
+  # ctime_tests because many return values depend on secret (i.e., "uninitialized") values, and
+  # we're only interested in detecting branches (which count as "uses") on secret data.
+  if test x"$msan_enabled" = x"yes"; then
+    SECP_TRY_APPEND_CFLAGS([-fno-sanitize-memory-param-retval], SECP_CFLAGS)
+    print_msan_notice=yes
+  fi
+fi
+
 if test x"$enable_coverage" = x"yes"; then
    SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOVERAGE=1"
    SECP_CFLAGS="-O0 --coverage $SECP_CFLAGS"
@@ -542,12 +556,7 @@ fi
 ### Check for --enable-experimental if necessary
 ###

-if test x"$enable_experimental" = x"yes"; then
-  AC_MSG_NOTICE([******])
-  AC_MSG_NOTICE([WARNING: experimental build])
-  AC_MSG_NOTICE([Experimental features do not have stable APIs or properties, and may not be safe for production use.])
-  AC_MSG_NOTICE([******])
-else
+if test x"$enable_experimental" = x"no"; then
  # The order of the following tests matters. If the user enables a dependent
  # module (which automatically enables the module dependencies) we want to
  # print an error for the dependent module, not the module dependency. Hence,
@@ -660,9 +669,23 @@ if test x"$set_widemul" != xauto; then
 echo "  wide multiplication       = $set_widemul"
 fi
 echo
-echo "  valgrind                  = $enable_valgrind"
-echo "  CC                        = $CC"
-echo "  CPPFLAGS                  = $CPPFLAGS"
-echo "  SECP_CFLAGS               = $SECP_CFLAGS"
-echo "  CFLAGS                    = $CFLAGS"
-echo "  LDFLAGS                   = $LDFLAGS"
+echo "  valgrind                = $enable_valgrind"
+echo "  CC                      = $CC"
+echo "  CPPFLAGS                = $CPPFLAGS"
+echo "  SECP_CFLAGS             = $SECP_CFLAGS"
+echo "  CFLAGS                  = $CFLAGS"
+echo "  LDFLAGS                 = $LDFLAGS"
+
+if test x"$print_msan_notice" = x"yes"; then
+  echo
+  echo "Note:"
+  echo "  MemorySanitizer detected, tried to add -fno-sanitize-memory-param-retval to SECP_CFLAGS"
+  echo "  to avoid false positives in ctime_tests. Pass --disable-ctime-tests to avoid this."
+fi
+
+if test x"$enable_experimental" = x"yes"; then
+  echo
+  echo "WARNING: Experimental build"
+  echo "  Experimental features do not have stable APIs or properties, and may not be safe for"
+  echo "  production use."
+fi