Squashed 'src/secp256k1/' changes from 44c2452fd3..af65d30cc8

af65d30cc8 Merge bitcoin-core/secp256k1#1116: build: Fix #include "..." paths to get rid of further -I arguments
40a3473a9d build: Fix #include "..." paths to get rid of further -I arguments
43756da819 Merge bitcoin-core/secp256k1#1115: Fix sepc256k1 -> secp256k1 typo in group.h
069aba8125 Fix sepc256k1 -> secp256k1 typo in group.h
accadc94df Merge bitcoin-core/secp256k1#1114: `_scratch_destroy`: move `VERIFY_CHECK` after invalid scrach space check
cd47033335 Merge bitcoin-core/secp256k1#1084: ci: Add MSVC builds
1827c9bf2b scratch_destroy: move VERIFY_CHECK after invalid scrach space check
49e2acd927 configure: Improve rationale for WERROR_CFLAGS
8dc4b03341 ci: Add a C++ job that compiles the public headers without -fpermissive
51f296a46c ci: Run persistent wineserver to speed up wine
3fb3269c22 ci: Add 32-bit MinGW64 build
9efc2e5221 ci: Add MSVC builds
2be6ba0fed configure: Convince autotools to work with MSVC's archiver lib.exe
bd81f4140a schnorrsig bench: Suppress a stupid warning in MSVC
09f3d71c51 configure: Add a few CFLAGS for MSVC
3b4f3d0d46 build: Reject C++ compilers in the preprocessor
1cc0941414 configure: Don't abort if the compiler does not define __STDC__
cca8cbbac8 configure: Output message when checking for valgrind
1a6be5745f bench: Make benchmarks compile on MSVC

git-subtree-dir: src/secp256k1
git-subtree-split: af65d30cc84e04adcc15a3c69ce2f79aec69dbd8

Author: dhruv

.cirrus.yml

@@ -241,17 +241,57 @@ task:
   << : *CAT_LOGS
 
 task:
-  name: "x86_64 (mingw32-w64): Windows (Debian stable, Wine)"
   << : *LINUX_CONTAINER
   env:
-    WRAPPER_CMD: wine64-stable
-    SECP256K1_TEST_ITERS: 16
-    HOST: x86_64-w64-mingw32
+    WRAPPER_CMD: wine
+    WITH_VALGRIND: no
+    ECDH: yes
+    RECOVERY: yes
+    SCHNORRSIG: yes
+    CTIMETEST: no
+  matrix:
+    - name: "x86_64 (mingw32-w64): Windows (Debian stable, Wine)"
+      env:
+        HOST: x86_64-w64-mingw32
+    - name: "i686 (mingw32-w64): Windows (Debian stable, Wine)"
+      env:
+        HOST: i686-w64-mingw32
+  << : *MERGE_BASE
+  test_script:
+    - ./ci/cirrus.sh
+  << : *CAT_LOGS
+
+task:
+  << : *LINUX_CONTAINER
+  env:
+    WRAPPER_CMD: wine
+    WERROR_CFLAGS: -WX
     WITH_VALGRIND: no
     ECDH: yes
     RECOVERY: yes
+    EXPERIMENTAL: yes
     SCHNORRSIG: yes
     CTIMETEST: no
+    # Set non-essential options that affect the CLI messages here.
+    # (They depend on the user's taste, so we don't want to set them automatically in configure.ac.)
+    CFLAGS: -nologo -diagnostics:caret
+    LDFLAGS: -XCClinker -nologo -XCClinker -diagnostics:caret
+  # Use a MinGW-w64 host to tell ./configure we're building for Windows.
+  # This will detect some MinGW-w64 tools but then make will need only
+  # the MSVC tools CC, AR and NM as specified below.
+  matrix:
+    - name: "x86_64 (MSVC): Windows (Debian stable, Wine)"
+      env:
+        HOST: x86_64-w64-mingw32
+        CC: /opt/msvc/bin/x64/cl
+        AR: /opt/msvc/bin/x64/lib
+        NM: /opt/msvc/bin/x64/dumpbin -symbols -headers
+    - name: "i686 (MSVC): Windows (Debian stable, Wine)"
+      env:
+        HOST: i686-w64-mingw32
+        CC: /opt/msvc/bin/x86/cl
+        AR: /opt/msvc/bin/x86/lib
+        NM: /opt/msvc/bin/x86/dumpbin -symbols -headers
   << : *MERGE_BASE
   test_script:
     - ./ci/cirrus.sh
@@ -302,13 +342,12 @@ task:
   << : *CAT_LOGS
 
 task:
-  name: "C++ -fpermissive"
+  name: "C++ -fpermissive (entire project)"
   << : *LINUX_CONTAINER
   env:
-    # ./configure correctly errors out when given CC=g++.
-    # We hack around this by passing CC=g++ only to make.
-    CC: gcc
-    MAKEFLAGS: -j4 CC=g++ CFLAGS=-fpermissive\ -g
+    CC: g++
+    CFLAGS: -fpermissive -g
+    CPPFLAGS: -DSECP256K1_CPLUSPLUS_TEST_OVERRIDE
     WERROR_CFLAGS:
     ECDH: yes
     RECOVERY: yes
@@ -318,6 +357,14 @@ task:
     - ./ci/cirrus.sh
   << : *CAT_LOGS
 
+task:
+  name: "C++ (public headers)"
+  << : *LINUX_CONTAINER
+  test_script:
+    - g++ -Werror include/*.h
+    - clang -Werror -x c++-header include/*.h
+    - /opt/msvc/bin/x64/cl.exe -c -WX -TP include/*.h
+
 task:
   name: "sage prover"
   << : *LINUX_CONTAINER

Makefile.am

@@ -87,7 +87,7 @@ endif
 endif
 
 libsecp256k1_la_SOURCES = src/secp256k1.c
-libsecp256k1_la_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/src $(SECP_INCLUDES)
+libsecp256k1_la_CPPFLAGS = $(SECP_INCLUDES)
 libsecp256k1_la_LIBADD = $(SECP_LIBS) $(COMMON_LIB) $(PRECOMPUTED_LIB)
 libsecp256k1_la_LDFLAGS = -no-undefined -version-info $(LIB_VERSION_CURRENT):$(LIB_VERSION_REVISION):$(LIB_VERSION_AGE)
 
@@ -112,7 +112,7 @@ TESTS =
 if USE_TESTS
 noinst_PROGRAMS += tests
 tests_SOURCES = src/tests.c
-tests_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/include $(SECP_INCLUDES) $(SECP_TEST_INCLUDES)
+tests_CPPFLAGS = $(SECP_INCLUDES) $(SECP_TEST_INCLUDES)
 if VALGRIND_ENABLED
 tests_CPPFLAGS += -DVALGRIND
 noinst_PROGRAMS += valgrind_ctime_test

build-aux/m4/bitcoin_secp.m4

@@ -10,6 +10,7 @@ AC_MSG_RESULT([$has_64bit_asm])
 ])
 
 AC_DEFUN([SECP_VALGRIND_CHECK],[
+AC_MSG_CHECKING([for valgrind support])
 if test x"$has_valgrind" != x"yes"; then
   CPPFLAGS_TEMP="$CPPFLAGS"
   CPPFLAGS="$VALGRIND_CPPFLAGS $CPPFLAGS"
@@ -21,6 +22,7 @@ if test x"$has_valgrind" != x"yes"; then
     #endif
   ]])], [has_valgrind=yes; AC_DEFINE(HAVE_VALGRIND,1,[Define this symbol if valgrind is installed, and it supports the host platform])])
 fi
+AC_MSG_RESULT($has_valgrind)
 ])
 
 dnl SECP_TRY_APPEND_CFLAGS(flags, VAR)

ci/cirrus.sh

@@ -5,10 +5,20 @@ set -x
 
 export LC_ALL=C
 
+# Start persistent wineserver if necessary.
+# This speeds up jobs with many invocations of wine (e.g., ./configure with MSVC) tremendously.
+case "$WRAPPER_CMD" in
+    *wine*)
+        # This is apparently only reliable when we run a dummy command such as "hh.exe" afterwards.
+        wineserver -p && wine hh.exe
+        ;;
+esac
+
 env >> test_env.log
 
 $CC -v || true
 valgrind --version || true
+$WRAPPER_CMD --version || true
 
 ./autogen.sh
 
@@ -63,6 +73,9 @@ then
     make precomp
 fi
 
+# Shutdown wineserver again
+wineserver -k || true
+
 # Check that no repo files have been modified by the build.
 # (This fails for example if the precomp files need to be updated in the repo.)
 git diff --exit-code

ci/linux-debian.Dockerfile

@@ -1,15 +1,14 @@
 FROM debian:stable
 
-RUN dpkg --add-architecture i386
-RUN dpkg --add-architecture s390x
-RUN dpkg --add-architecture armhf
-RUN dpkg --add-architecture arm64
-RUN dpkg --add-architecture ppc64el
-RUN apt-get update
+RUN dpkg --add-architecture i386 && \
+    dpkg --add-architecture s390x && \
+    dpkg --add-architecture armhf && \
+    dpkg --add-architecture arm64 && \
+    dpkg --add-architecture ppc64el
 
 # 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 \
+RUN apt-get update && apt-get install --no-install-recommends -y \
         git ca-certificates \
         make automake libtool pkg-config dpkg-dev valgrind qemu-user \
         gcc clang llvm libc6-dbg \
@@ -19,8 +18,20 @@ RUN apt-get install --no-install-recommends --no-upgrade -y \
         gcc-arm-linux-gnueabihf libc6-dev-armhf-cross libc6-dbg:armhf \
         gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 \
         gcc-powerpc64le-linux-gnu libc6-dev-ppc64el-cross libc6-dbg:ppc64el \
-        wine gcc-mingw-w64-x86-64 \
+        gcc-mingw-w64-x86-64-win32 wine64 wine \
+        gcc-mingw-w64-i686-win32 wine32 \
         sagemath
 
-# Run a dummy command in wine to make it set up configuration
-RUN wine64-stable xcopy || true
+WORKDIR /root
+# The "wine" package provides a convience wrapper that we need
+RUN apt-get update && apt-get install --no-install-recommends -y \
+        git ca-certificates wine64 wine python3-simplejson python3-six msitools winbind procps && \
+    git clone https://github.com/mstorsjo/msvc-wine && \
+    mkdir /opt/msvc && \
+    python3 msvc-wine/vsdownload.py --accept-license --dest /opt/msvc Microsoft.VisualStudio.Workload.VCTools && \
+    msvc-wine/install.sh /opt/msvc
+
+# Initialize the wine environment. Wait until the wineserver process has
+# exited before closing the session, to avoid corrupting the wine prefix.
+RUN wine64 wineboot --init && \
+    while (ps -A | grep wineserver) > /dev/null; do sleep 1; done

configure.ac

@@ -33,14 +33,18 @@ AM_INIT_AUTOMAKE([1.11.2 foreign subdir-objects])
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
 AC_PROG_CC
-if test x"$ac_cv_prog_cc_c89" = x"no"; then
-  AC_MSG_ERROR([c89 compiler support required])
-fi
 AM_PROG_AS
 AM_PROG_AR
 
+# Clear some cache variables as a workaround for a bug that appears due to a bad
+# interaction between AM_PROG_AR and LT_INIT when combining MSVC's archiver lib.exe.
+# https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54421
+AS_UNSET(ac_cv_prog_AR)
+AS_UNSET(ac_cv_prog_ac_ct_AR)
 LT_INIT([win32-dll])
 
+PKG_PROG_PKG_CONFIG
+
 build_windows=no
 
 case $host_os in
@@ -87,23 +91,35 @@ esac
 #
 # TODO We should analogously not touch CPPFLAGS and LDFLAGS but currently there are no issues.
 AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [
-    # Try to append -Werror=unknown-warning-option to CFLAGS temporarily. Otherwise clang will
-    # not error out if it gets unknown warning flags and the checks here will always succeed
-    # no matter if clang knows the flag or not.
-    SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS="$CFLAGS"
-    SECP_TRY_APPEND_CFLAGS([-Werror=unknown-warning-option], CFLAGS)
-
-    SECP_TRY_APPEND_CFLAGS([-std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef], $1) # GCC >= 3.0, -Wlong-long is implied by -pedantic.
-    SECP_TRY_APPEND_CFLAGS([-Wno-overlength-strings], $1) # GCC >= 4.2, -Woverlength-strings is implied by -pedantic.
-    SECP_TRY_APPEND_CFLAGS([-Wall], $1) # GCC >= 2.95 and probably many other compilers
-    SECP_TRY_APPEND_CFLAGS([-Wno-unused-function], $1) # GCC >= 3.0, -Wunused-function is implied by -Wall.
-    SECP_TRY_APPEND_CFLAGS([-Wextra], $1) # GCC >= 3.4, this is the newer name of -W, which we don't use because older GCCs will warn about unused functions.
-    SECP_TRY_APPEND_CFLAGS([-Wcast-align], $1) # GCC >= 2.95
-    SECP_TRY_APPEND_CFLAGS([-Wcast-align=strict], $1) # GCC >= 8.0
-    SECP_TRY_APPEND_CFLAGS([-Wconditional-uninitialized], $1) # Clang >= 3.0 only
-    SECP_TRY_APPEND_CFLAGS([-fvisibility=hidden], $1) # GCC >= 4.0
-
-    CFLAGS="$SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS"
+    # GCC and compatible (incl. clang)
+    if test "x$GCC" = "xyes"; then
+      # Try to append -Werror=unknown-warning-option to CFLAGS temporarily. Otherwise clang will
+      # not error out if it gets unknown warning flags and the checks here will always succeed
+      # no matter if clang knows the flag or not.
+      SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS="$CFLAGS"
+      SECP_TRY_APPEND_CFLAGS([-Werror=unknown-warning-option], CFLAGS)
+
+      SECP_TRY_APPEND_CFLAGS([-std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef], $1) # GCC >= 3.0, -Wlong-long is implied by -pedantic.
+      SECP_TRY_APPEND_CFLAGS([-Wno-overlength-strings], $1) # GCC >= 4.2, -Woverlength-strings is implied by -pedantic.
+      SECP_TRY_APPEND_CFLAGS([-Wall], $1) # GCC >= 2.95 and probably many other compilers
+      SECP_TRY_APPEND_CFLAGS([-Wno-unused-function], $1) # GCC >= 3.0, -Wunused-function is implied by -Wall.
+      SECP_TRY_APPEND_CFLAGS([-Wextra], $1) # GCC >= 3.4, this is the newer name of -W, which we don't use because older GCCs will warn about unused functions.
+      SECP_TRY_APPEND_CFLAGS([-Wcast-align], $1) # GCC >= 2.95
+      SECP_TRY_APPEND_CFLAGS([-Wcast-align=strict], $1) # GCC >= 8.0
+      SECP_TRY_APPEND_CFLAGS([-Wconditional-uninitialized], $1) # Clang >= 3.0 only
+      SECP_TRY_APPEND_CFLAGS([-fvisibility=hidden], $1) # GCC >= 4.0
+
+      CFLAGS="$SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS"
+    fi
+
+    # MSVC
+    # Assume MSVC if we're building for Windows but not with GCC or compatible;
+    # libtool makes the same assumption internally.
+    # Note that "/opt" and "-opt" are equivalent for MSVC; we use "-opt" because "/opt" looks like a path.
+    if test x"$GCC" != x"yes" && test x"$build_windows" = x"yes"; then
+      SECP_TRY_APPEND_CFLAGS([-W2 -wd4146], $1) # Moderate warning level, disable warning C4146 "unary minus operator applied to unsigned type, result still unsigned"
+      SECP_TRY_APPEND_CFLAGS([-external:anglebrackets -external:W0], $1) # Suppress warnings from #include <...> files
+    fi
 ])
 SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS)
 
@@ -326,7 +342,9 @@ if test x"$enable_valgrind" = x"yes"; then
   SECP_INCLUDES="$SECP_INCLUDES $VALGRIND_CPPFLAGS"
 fi
 
-# Add -Werror and similar flags passed from the outside (for testing, e.g., in CI)
+# Add -Werror and similar flags passed from the outside (for testing, e.g., in CI).
+# We don't want to set the user variable CFLAGS in CI because this would disable
+# autoconf's logic for setting default CFLAGS, which we would like to test in CI.
 SECP_CFLAGS="$SECP_CFLAGS $WERROR_CFLAGS"
 
 ###

src/bench.h

@@ -7,15 +7,31 @@
 #ifndef SECP256K1_BENCH_H
 #define SECP256K1_BENCH_H
 
+#include <stdlib.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
-#include "sys/time.h"
+
+#if (defined(_MSC_VER) && _MSC_VER >= 1900)
+#  include <time.h>
+#else
+#  include "sys/time.h"
+#endif
 
 static int64_t gettime_i64(void) {
+#if (defined(_MSC_VER) && _MSC_VER >= 1900)
+    /* C11 way to get wallclock time */
+    struct timespec tv;
+    if (!timespec_get(&tv, TIME_UTC)) {
+        fputs("timespec_get failed!", stderr);
+        exit(1);
+    }
+    return (int64_t)tv.tv_nsec / 1000 + (int64_t)tv.tv_sec * 1000000LL;
+#else
     struct timeval tv;
     gettimeofday(&tv, NULL);
     return (int64_t)tv.tv_usec + (int64_t)tv.tv_sec * 1000000LL;
+#endif
 }
 
 #define FP_EXP (6)

src/group.h

@@ -23,7 +23,7 @@ typedef struct {
 #define SECP256K1_GE_CONST_INFINITY {SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0, 0), SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0, 0), 1}
 
 /** A group element of the secp256k1 curve, in jacobian coordinates.
- *  Note: For exhastive test mode, sepc256k1 is replaced by a small subgroup of a different curve.
+ *  Note: For exhastive test mode, secp256k1 is replaced by a small subgroup of a different curve.
  */
 typedef struct {
     secp256k1_fe x; /* actual X: x/z^2 */

src/modules/ecdh/bench_impl.h

@@ -7,7 +7,7 @@
 #ifndef SECP256K1_MODULE_ECDH_BENCH_H
 #define SECP256K1_MODULE_ECDH_BENCH_H
 
-#include "../include/secp256k1_ecdh.h"
+#include "../../../include/secp256k1_ecdh.h"
 
 typedef struct {
     secp256k1_context *ctx;

src/modules/extrakeys/tests_exhaustive_impl.h

@@ -7,8 +7,8 @@
 #ifndef SECP256K1_MODULE_EXTRAKEYS_TESTS_EXHAUSTIVE_H
 #define SECP256K1_MODULE_EXTRAKEYS_TESTS_EXHAUSTIVE_H
 
-#include "src/modules/extrakeys/main_impl.h"
 #include "../../../include/secp256k1_extrakeys.h"
+#include "main_impl.h"
 
 static void test_exhaustive_extrakeys(const secp256k1_context *ctx, const secp256k1_ge* group) {
     secp256k1_keypair keypair[EXHAUSTIVE_TEST_ORDER - 1];

src/modules/recovery/bench_impl.h

@@ -7,7 +7,7 @@
 #ifndef SECP256K1_MODULE_RECOVERY_BENCH_H
 #define SECP256K1_MODULE_RECOVERY_BENCH_H
 
-#include "../include/secp256k1_recovery.h"
+#include "../../../include/secp256k1_recovery.h"
 
 typedef struct {
     secp256k1_context *ctx;

src/modules/recovery/tests_exhaustive_impl.h

@@ -7,7 +7,7 @@
 #ifndef SECP256K1_MODULE_RECOVERY_EXHAUSTIVE_TESTS_H
 #define SECP256K1_MODULE_RECOVERY_EXHAUSTIVE_TESTS_H
 
-#include "src/modules/recovery/main_impl.h"
+#include "main_impl.h"
 #include "../../../include/secp256k1_recovery.h"
 
 void test_exhaustive_recovery_sign(const secp256k1_context *ctx, const secp256k1_ge *group) {

src/modules/schnorrsig/bench_impl.h

@@ -91,10 +91,12 @@ void run_schnorrsig_bench(int iters, int argc, char** argv) {
         free((void *)data.msgs[i]);
         free((void *)data.sigs[i]);
     }
-    free(data.keypairs);
-    free(data.pk);
-    free(data.msgs);
-    free(data.sigs);
+
+    /* Casting to (void *) avoids a stupid warning in MSVC. */
+    free((void *)data.keypairs);
+    free((void *)data.pk);
+    free((void *)data.msgs);
+    free((void *)data.sigs);
 
     secp256k1_context_destroy(data.ctx);
 }

src/modules/schnorrsig/tests_exhaustive_impl.h

@@ -8,7 +8,7 @@
 #define SECP256K1_MODULE_SCHNORRSIG_TESTS_EXHAUSTIVE_H
 
 #include "../../../include/secp256k1_schnorrsig.h"
-#include "src/modules/schnorrsig/main_impl.h"
+#include "main_impl.h"
 
 static const unsigned char invalid_pubkey_bytes[][32] = {
     /* 0 */