Merge #304: ci: Backport MSan fixes for ctime_tests

b9a82b4 ci: Update macOS image (Hennadii Stepanov)
a1be8ed ci: Silent Homebrew's noisy reinstall warnings (Hennadii Stepanov)
fd259fe ci: Add job with -fsanitize-memory-param-retval (Tim Ruffing)
76b3396 configure: Move "experimental" warning to bottom (Tim Ruffing)
e639e6c 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:
  - [55e5d975db9a641bcbeffcc3fc086e54d322fe26](bitcoin-core/secp256k1@55e5d97),
  - [e1bef0961c53b23dbaf35f5fa87149a1aadfad37](bitcoin-core/secp256k1@e1bef09),
  - [ebfb82ee2f8c15ae6129932380b1dfb0942ea35a](bitcoin-core/secp256k1@ebfb82e)

  #### References:
  Upstream: bitcoin-core/secp256k1#1517

ACKs for top commit:
  apoelstra:
    ACK b9a82b4; successfully ran local tests
  jonasnick:
    ACK b9a82b4

Tree-SHA512: b6c9ae38b142c7fadf1157a1b25cdc8e85e6cd271222b950b1246f14daf85b072979d4f2ee8791c8b81b2f6a94ad1094fc8110bbd4ff19153de6a6efad2e3cc0

Author: real-or-random

.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.

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], [

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