Merge pull request #140 from apoelstra/2021-07--resync

Upstream PRs 879, 959, 955, 944, 951, 960, 844, 963, 965

Author: apoelstra

.cirrus.yml

@@ -1,12 +1,19 @@
 env:
-  WIDEMUL: auto
+  ### compiler options
+  HOST:
+  # Specific warnings can be disabled with -Wno-error=foo.
+  # -pedantic-errors is not equivalent to -Werror=pedantic and thus not implied by -Werror according to the GCC manual.
+  WERROR_CFLAGS: -Werror -pedantic-errors
+  MAKEFLAGS: -j2
+  BUILD: check
+  ### secp256k1 config
   STATICPRECOMPUTATION: yes
   ECMULTGENPRECISION: auto
   ASM: no
-  BUILD: check
+  WIDEMUL: auto
   WITH_VALGRIND: yes
   EXTRAFLAGS:
-  HOST:
+  ### secp256k1 modules
   ECDH: no
   RECOVERY: no
   SCHNORRSIG: no
@@ -16,12 +23,11 @@ env:
   WHITELIST: no
   MUSIG: no
   ECDSAADAPTOR: no
-  EXPERIMENTAL: no
-  CTIMETEST: yes
-  BENCH: yes
+  ### test options
   TEST_ITERS:
+  BENCH: yes
   BENCH_ITERS: 2
-  MAKEFLAGS: -j2
+  CTIMETEST: yes
 
 cat_logs_snippet: &CAT_LOGS
   always:
@@ -340,3 +346,23 @@ task:
     - ./ci/cirrus.sh
   << : *CAT_LOGS
 
+task:
+  name: "C++ -fpermissive"
+  container:
+    dockerfile: ci/linux-debian.Dockerfile
+    cpu: 1
+    memory: 1G
+  env:
+    # ./configure correctly errors out when given CC=g++.
+    # We hack around this by passing CC=g++ only to make.
+    CC: gcc
+    MAKEFLAGS: -j2 CC=g++ CFLAGS=-fpermissive
+    WERROR_CFLAGS:
+    EXPERIMENTAL: yes
+    ECDH: yes
+    RECOVERY: yes
+    SCHNORRSIG: yes
+  << : *MERGE_BASE
+  test_script:
+    - ./ci/cirrus.sh
+  << : *CAT_LOGS

.gitignore

@@ -25,6 +25,7 @@ aclocal.m4
 autom4te.cache/
 config.log
 config.status
+conftest*
 *.tar.gz
 *.la
 libtool

Makefile.am

@@ -1,5 +1,9 @@
 ACLOCAL_AMFLAGS = -I build-aux/m4
 
+# AM_CFLAGS will be automatically prepended to CFLAGS by Automake when compiling some foo
+# which does not have an explicit foo_CFLAGS variable set.
+AM_CFLAGS = $(SECP_CFLAGS)
+
 lib_LTLIBRARIES = libsecp256k1.la
 include_HEADERS = include/secp256k1.h
 include_HEADERS += include/secp256k1_preallocated.h
@@ -131,10 +135,10 @@ CPPFLAGS_FOR_BUILD +=-I$(top_srcdir) -I$(builddir)/src
 gen_context_OBJECTS = gen_context.o
 gen_context_BIN = gen_context$(BUILD_EXEEXT)
 gen_%.o: src/gen_%.c src/libsecp256k1-config.h
-	$(CC_FOR_BUILD) $(DEFS) $(CPPFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@
+	$(CC_FOR_BUILD) $(DEFS) $(CPPFLAGS_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@
 
 $(gen_context_BIN): $(gen_context_OBJECTS)
-	$(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $^ -o $@
+	$(CC_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $^ -o $@
 
 $(libsecp256k1_la_OBJECTS): src/ecmult_static_context.h
 $(tests_OBJECTS): src/ecmult_static_context.h

README.md

@@ -17,6 +17,7 @@ Features:
 * Suitable for embedded systems.
 * Optional module for public key recovery.
 * Optional module for ECDH key exchange.
+* Optional module for Schnorr signatures according to [BIP-340](https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki) (experimental).
 * Optional module for ECDSA adaptor signatures (experimental).
 
 Experimental features have not received enough scrutiny to satisfy the standard of quality of this library but are made available for testing and review by the community. The APIs of these features should not be considered stable.

build-aux/m4/bitcoin_secp.m4

@@ -82,3 +82,19 @@ if test x"$has_valgrind" != x"yes"; then
   AC_CHECK_HEADER([valgrind/memcheck.h], [has_valgrind=yes; AC_DEFINE(HAVE_VALGRIND,1,[Define this symbol if valgrind is installed])])
 fi
 ])
+
+dnl SECP_TRY_APPEND_CFLAGS(flags, VAR)
+dnl Append flags to VAR if CC accepts them.
+AC_DEFUN([SECP_TRY_APPEND_CFLAGS], [
+  AC_MSG_CHECKING([if ${CC} supports $1])
+  SECP_TRY_APPEND_CFLAGS_saved_CFLAGS="$CFLAGS"
+  CFLAGS="$1 $CFLAGS"
+  AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])], [flag_works=yes], [flag_works=no])
+  AC_MSG_RESULT($flag_works)
+  CFLAGS="$SECP_TRY_APPEND_CFLAGS_saved_CFLAGS"
+  if test x"$flag_works" = x"yes"; then
+    $2="$$2 $1"
+  fi
+  unset flag_works
+  AC_SUBST($2)
+])

ci/linux-debian.Dockerfile

@@ -13,6 +13,7 @@ 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 llvm libc6-dbg \
+        g++ \
         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 \

configure.ac

@@ -8,10 +8,6 @@ AH_TOP([#define LIBSECP256K1_CONFIG_H])
 AH_BOTTOM([#endif /*LIBSECP256K1_CONFIG_H*/])
 AM_INIT_AUTOMAKE([foreign subdir-objects])
 
-# Set -g if CFLAGS are not already set, which matches the default autoconf
-# behavior (see PROG_CC in the Autoconf manual) with the exception that we don't
-# set -O2 here because we set it in any case (see further down).
-: ${CFLAGS="-g"}
 LT_INIT
 
 # Make the compilation flags quiet unless V=1 is used.
@@ -42,8 +38,8 @@ AM_PROG_AS
 case $host_os in
   *darwin*)
      if  test x$cross_compiling != xyes; then
-       AC_PATH_PROG([BREW],brew,)
-       if test x$BREW != x; then
+       AC_CHECK_PROG([BREW], brew, brew)
+       if test x$BREW = xbrew; then
          # These Homebrew packages may be keg-only, meaning that they won't be found
          # in expected paths because they may conflict with system files. Ask
          # Homebrew where each one is located, then adjust paths accordingly.
@@ -58,10 +54,10 @@ case $host_os in
            VALGRIND_CPPFLAGS="-I$valgrind_prefix/include"
          fi
        else
-         AC_PATH_PROG([PORT],port,)
+         AC_CHECK_PROG([PORT], port, port)
          # If homebrew isn't installed and macports is, add the macports default paths
          # as a last resort.
-         if test x$PORT != x; then
+         if test x$PORT = xport; then
            CPPFLAGS="$CPPFLAGS -isystem /opt/local/include"
            LDFLAGS="$LDFLAGS -L/opt/local/lib"
          fi
@@ -70,35 +66,41 @@ case $host_os in
    ;;
 esac
 
-CFLAGS="-W $CFLAGS"
-
-warn_CFLAGS="-std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef -Wno-unused-function -Wno-long-long -Wno-overlength-strings"
-saved_CFLAGS="$CFLAGS"
-CFLAGS="$warn_CFLAGS $CFLAGS"
-AC_MSG_CHECKING([if ${CC} supports ${warn_CFLAGS}])
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
-    [ AC_MSG_RESULT([yes]) ],
-    [ AC_MSG_RESULT([no])
-      CFLAGS="$saved_CFLAGS"
-    ])
-
-saved_CFLAGS="$CFLAGS"
-CFLAGS="-Wconditional-uninitialized $CFLAGS"
-AC_MSG_CHECKING([if ${CC} supports -Wconditional-uninitialized])
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
-    [ AC_MSG_RESULT([yes]) ],
-    [ AC_MSG_RESULT([no])
-      CFLAGS="$saved_CFLAGS"
-    ])
-
-saved_CFLAGS="$CFLAGS"
-CFLAGS="-fvisibility=hidden $CFLAGS"
-AC_MSG_CHECKING([if ${CC} supports -fvisibility=hidden])
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
-    [ AC_MSG_RESULT([yes]) ],
-    [ AC_MSG_RESULT([no])
-      CFLAGS="$saved_CFLAGS"
-    ])
+# Try if some desirable compiler flags are supported and append them to SECP_CFLAGS.
+#
+# These are our own flags, so we append them to our own SECP_CFLAGS variable (instead of CFLAGS) as
+# recommended in the automake manual (Section "Flag Variables Ordering"). CFLAGS belongs to the user
+# and we are not supposed to touch it. In the Makefile, we will need to ensure that SECP_CFLAGS
+# is prepended to CFLAGS when invoking the compiler so that the user always has the last word (flag).
+#
+# Another advantage of not touching CFLAGS is that the contents of CFLAGS will be picked up by
+# libtool for compiling helper executables. For example, when compiling for Windows, libtool will
+# generate entire wrapper executables (instead of simple wrapper scripts as on Unix) to ensure
+# proper operation of uninstalled programs linked by libtool against the uninstalled shared library.
+# These executables are compiled from C source file for which our flags may not be appropriate,
+# e.g., -std=c89 flag has lead to undesirable warnings in the past.
+#
+# 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"
+])
+SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS)
 
 ###
 ### Define config arguments
@@ -253,10 +255,14 @@ AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"])
 
 if test x"$enable_coverage" = x"yes"; then
     AC_DEFINE(COVERAGE, 1, [Define this symbol to compile out all VERIFY code])
-    CFLAGS="-O0 --coverage $CFLAGS"
+    SECP_CFLAGS="-O0 --coverage $SECP_CFLAGS"
     LDFLAGS="--coverage $LDFLAGS"
 else
-    CFLAGS="-O2 $CFLAGS"
+    # Most likely the CFLAGS already contain -O2 because that is autoconf's default.
+    # We still add it here because passing it twice is not an issue, and handling
+    # this case would just add unnecessary complexity (see #896).
+    SECP_CFLAGS="-O2 $SECP_CFLAGS"
+    SECP_CFLAGS_FOR_BUILD="-O2 $SECP_CFLAGS_FOR_BUILD"
 fi
 
 AC_MSG_CHECKING([for __builtin_popcount])
@@ -403,6 +409,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)
+SECP_CFLAGS="$SECP_CFLAGS $WERROR_CFLAGS"
+
 # Handle static precomputation (after everything which modifies CFLAGS and friends)
 if test x"$use_ecmult_static_precomputation" != x"no"; then
   if test x"$cross_compiling" = x"no"; then
@@ -412,8 +421,9 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then
     fi
     # If we're not cross-compiling, simply use the same compiler for building the static precompation code.
     CC_FOR_BUILD="$CC"
-    CFLAGS_FOR_BUILD="$CFLAGS"
     CPPFLAGS_FOR_BUILD="$CPPFLAGS"
+    SECP_CFLAGS_FOR_BUILD="$SECP_CFLAGS"
+    CFLAGS_FOR_BUILD="$CFLAGS"
     LDFLAGS_FOR_BUILD="$LDFLAGS"
   else
     AX_PROG_CC_FOR_BUILD
@@ -423,42 +433,32 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then
     cross_compiling=no
     SAVE_CC="$CC"
     CC="$CC_FOR_BUILD"
-    SAVE_CFLAGS="$CFLAGS"
-    CFLAGS="$CFLAGS_FOR_BUILD"
     SAVE_CPPFLAGS="$CPPFLAGS"
     CPPFLAGS="$CPPFLAGS_FOR_BUILD"
+    SAVE_CFLAGS="$CFLAGS"
+    CFLAGS="$CFLAGS_FOR_BUILD"
     SAVE_LDFLAGS="$LDFLAGS"
     LDFLAGS="$LDFLAGS_FOR_BUILD"
 
-    warn_CFLAGS_FOR_BUILD="-Wall -Wextra -Wno-unused-function"
-    saved_CFLAGS="$CFLAGS"
-    CFLAGS="$warn_CFLAGS_FOR_BUILD $CFLAGS"
-    AC_MSG_CHECKING([if native ${CC_FOR_BUILD} supports ${warn_CFLAGS_FOR_BUILD}])
-    AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
-        [ AC_MSG_RESULT([yes]) ],
-        [ AC_MSG_RESULT([no])
-          CFLAGS="$saved_CFLAGS"
-        ])
+    SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS_FOR_BUILD)
 
     AC_MSG_CHECKING([for working native compiler: ${CC_FOR_BUILD}])
     AC_RUN_IFELSE(
       [AC_LANG_PROGRAM([], [])],
       [working_native_cc=yes],
       [working_native_cc=no],[:])
 
-    CFLAGS_FOR_BUILD="$CFLAGS"
-
     # Restore the environment
     cross_compiling=$save_cross_compiling
     CC="$SAVE_CC"
-    CFLAGS="$SAVE_CFLAGS"
     CPPFLAGS="$SAVE_CPPFLAGS"
+    CFLAGS="$SAVE_CFLAGS"
     LDFLAGS="$SAVE_LDFLAGS"
 
     if test x"$working_native_cc" = x"no"; then
       AC_MSG_RESULT([no])
       set_precomp=no
-      m4_define([please_set_for_build], [Please set CC_FOR_BUILD, CFLAGS_FOR_BUILD, CPPFLAGS_FOR_BUILD, and/or LDFLAGS_FOR_BUILD.])
+      m4_define([please_set_for_build], [Please set CC_FOR_BUILD, CPPFLAGS_FOR_BUILD, CFLAGS_FOR_BUILD, and/or LDFLAGS_FOR_BUILD.])
       if test x"$use_ecmult_static_precomputation" = x"yes";  then
         AC_MSG_ERROR([native compiler ${CC_FOR_BUILD} does not produce working binaries. please_set_for_build])
       else
@@ -471,8 +471,9 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then
   fi
 
   AC_SUBST(CC_FOR_BUILD)
-  AC_SUBST(CFLAGS_FOR_BUILD)
   AC_SUBST(CPPFLAGS_FOR_BUILD)
+  AC_SUBST(SECP_CFLAGS_FOR_BUILD)
+  AC_SUBST(CFLAGS_FOR_BUILD)
   AC_SUBST(LDFLAGS_FOR_BUILD)
 else
   set_precomp=no
@@ -626,6 +627,7 @@ AC_SUBST(SECP_INCLUDES)
 AC_SUBST(SECP_LIBS)
 AC_SUBST(SECP_TEST_LIBS)
 AC_SUBST(SECP_TEST_INCLUDES)
+AC_SUBST(SECP_CFLAGS)
 AM_CONDITIONAL([ENABLE_COVERAGE], [test x"$enable_coverage" = x"yes"])
 AM_CONDITIONAL([USE_TESTS], [test x"$use_tests" != x"no"])
 AM_CONDITIONAL([USE_EXHAUSTIVE_TESTS], [test x"$use_exhaustive_tests" != x"no"])
@@ -679,13 +681,15 @@ fi
 echo
 echo "  valgrind                = $enable_valgrind"
 echo "  CC                      = $CC"
-echo "  CFLAGS                  = $CFLAGS"
 echo "  CPPFLAGS                = $CPPFLAGS"
+echo "  SECP_CFLAGS             = $SECP_CFLAGS"
+echo "  CFLAGS                  = $CFLAGS"
 echo "  LDFLAGS                 = $LDFLAGS"
 echo
 if test x"$set_precomp" = x"yes"; then
 echo "  CC_FOR_BUILD            = $CC_FOR_BUILD"
-echo "  CFLAGS_FOR_BUILD        = $CFLAGS_FOR_BUILD"
 echo "  CPPFLAGS_FOR_BUILD      = $CPPFLAGS_FOR_BUILD"
+echo "  SECP_CFLAGS_FOR_BUILD   = $SECP_CFLAGS_FOR_BUILD"
+echo "  CFLAGS_FOR_BUILD        = $CFLAGS_FOR_BUILD"
 echo "  LDFLAGS_FOR_BUILD       = $LDFLAGS_FOR_BUILD"
 fi

contrib/lax_der_parsing.c

@@ -120,7 +120,7 @@ int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_
     /* Copy R value */
     if (rlen > 32) {
         overflow = 1;
-    } else {
+    } else if (rlen) {
         memcpy(tmpsig + 32 - rlen, input + rpos, rlen);
     }
 
@@ -132,7 +132,7 @@ int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_
     /* Copy S value */
     if (slen > 32) {
         overflow = 1;
-    } else {
+    } else if (slen) {
         memcpy(tmpsig + 64 - slen, input + spos, slen);
     }
 

contrib/lax_der_privatekey_parsing.c

@@ -44,7 +44,7 @@ int ec_privkey_import_der(const secp256k1_context* ctx, unsigned char *out32, co
     if (end < privkey+2 || privkey[0] != 0x04 || privkey[1] > 0x20 || end < privkey+2+privkey[1]) {
         return 0;
     }
-    memcpy(out32 + 32 - privkey[1], privkey + 2, privkey[1]);
+    if (privkey[1]) memcpy(out32 + 32 - privkey[1], privkey + 2, privkey[1]);
     if (!secp256k1_ec_seckey_verify(ctx, out32)) {
         memset(out32, 0, 32);
         return 0;