Merge commits '44916ae9 86e3b38 ddf2b29 6138d73 e40fd27 ' into temp-merge-1156

Author: jonasnick

.cirrus.yml

@@ -74,6 +74,7 @@ task:
     - env: {WIDEMUL:  int64,  RECOVERY: yes}
     - env: {WIDEMUL:  int64,                 ECDH: yes, SCHNORRSIG: yes, EXPERIMENTAL: yes, ECDSA_S2C: yes,  RANGEPROOF: yes, WHITELIST: yes, GENERATOR: yes, MUSIG: yes, ECDSAADAPTOR: yes, BPPP: yes}
     - env: {WIDEMUL: int128}
+    - env: {WIDEMUL: int128_struct}
     - env: {WIDEMUL: int128,  RECOVERY: yes,            SCHNORRSIG: yes}
     - env: {WIDEMUL: int128,                 ECDH: yes, SCHNORRSIG: yes, EXPERIMENTAL: yes, ECDSA_S2C: yes, RANGEPROOF: yes, WHITELIST: yes, GENERATOR: yes, MUSIG: yes, ECDSAADAPTOR: yes, BPPP: yes}
     - env: {WIDEMUL: int128,  ASM: x86_64}
@@ -268,20 +269,26 @@ task:
     ECDSAADAPTOR: yes
     BPPP: yes
     CTIMETEST: no
+    # 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.
+    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
     # 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)"
+    - name: "x86_64 (MSVC): Windows (Debian stable, Wine, int128_struct)"
       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
+        WIDEMUL: int128_struct
+    - name: "x86_64 (MSVC): Windows (Debian stable, Wine, int128_struct with __(u)mulh)"
+      env:
+        WIDEMUL: int128_struct
+        CPPFLAGS: -DSECP256K1_MSVC_MULH_TEST_OVERRIDE
     - name: "i686 (MSVC): Windows (Debian stable, Wine)"
       env:
         HOST: i686-w64-mingw32
@@ -346,6 +353,40 @@ task:
     - ./ci/cirrus.sh
   << : *CAT_LOGS
 
+# Memory sanitizers
+task:
+  << : *LINUX_CONTAINER
+  name: "MSan"
+  env:
+    ECDH: yes
+    RECOVERY: yes
+    SCHNORRSIG: yes
+    EXPERIMENTAL: yes
+    ECDSA_S2C: yes
+    GENERATOR: yes
+    RANGEPROOF: yes
+    WHITELIST: yes
+    MUSIG: yes
+    ECDSAADAPTOR: yes
+    BPPP: yes
+    CTIMETEST: no
+    CC: clang
+    SECP256K1_TEST_ITERS: 32
+    ASM: no
+  container:
+    memory: 2G
+  matrix:
+    - env:
+        CFLAGS: "-fsanitize=memory -g"
+    - env:
+        ECMULTGENPRECISION: 2
+        ECMULTWINDOW: 2
+        CFLAGS: "-fsanitize=memory -g -O3"
+  << : *MERGE_BASE
+  test_script:
+    - ./ci/cirrus.sh
+  << : *CAT_LOGS
+
 task:
   name: "C++ -fpermissive (entire project)"
   << : *LINUX_CONTAINER

Makefile.am

@@ -50,6 +50,12 @@ noinst_HEADERS += src/precomputed_ecmult.h
 noinst_HEADERS += src/precomputed_ecmult_gen.h
 noinst_HEADERS += src/assumptions.h
 noinst_HEADERS += src/util.h
+noinst_HEADERS += src/int128.h
+noinst_HEADERS += src/int128_impl.h
+noinst_HEADERS += src/int128_native.h
+noinst_HEADERS += src/int128_native_impl.h
+noinst_HEADERS += src/int128_struct.h
+noinst_HEADERS += src/int128_struct_impl.h
 noinst_HEADERS += src/scratch.h
 noinst_HEADERS += src/scratch_impl.h
 noinst_HEADERS += src/selftest.h

ci/cirrus.sh

@@ -5,6 +5,27 @@ set -x
 
 export LC_ALL=C
 
+# Print relevant CI environment to allow reproducing the job outside of CI.
+print_environment() {
+    # Turn off -x because it messes up the output
+    set +x
+    # There are many ways to print variable names and their content. This one
+    # does not rely on bash.
+    for i in WERROR_CFLAGS MAKEFLAGS BUILD \
+            ECMULTWINDOW ECMULTGENPRECISION ASM WIDEMUL WITH_VALGRIND EXTRAFLAGS \
+            EXPERIMENTAL ECDH RECOVERY SCHNORRSIG \
+            ECDSA_S2C GENERATOR RANGEPROOF WHITELIST MUSIG ECDSAADAPTOR BPPP \
+            SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETEST\
+            EXAMPLES \
+            WRAPPER_CMD CC AR NM HOST
+    do
+        eval 'printf "%s %s " "$i=\"${'"$i"'}\""'
+    done
+    echo "$0"
+    set -x
+}
+print_environment
+
 # Start persistent wineserver if necessary.
 # This speeds up jobs with many invocations of wine (e.g., ./configure with MSVC) tremendously.
 case "$WRAPPER_CMD" in

configure.ac

@@ -220,7 +220,11 @@ AC_ARG_ENABLE(reduced_surjection_proof_size,
     [SECP_SET_DEFAULT([use_reduced_surjection_proof_size], [no], [no])])
 
 # Test-only override of the (autodetected by the C code) "widemul" setting.
-# Legal values are int64 (for [u]int64_t), int128 (for [unsigned] __int128), and auto (the default).
+# Legal values are:
+#  * int64 (for [u]int64_t),
+#  * int128 (for [unsigned] __int128),
+#  * int128_struct (for int128 implemented as a structure),
+#  *  and auto (the default).
 AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_widemul=auto])
 
 AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm|no|auto],
@@ -342,6 +346,9 @@ fi
 
 # Select wide multiplication implementation
 case $set_widemul in
+int128_struct)
+  AC_DEFINE(USE_FORCE_WIDEMUL_INT128_STRUCT, 1, [Define this symbol to force the use of the structure for simulating (unsigned) int128 based wide multiplication])
+  ;;
 int128)
   AC_DEFINE(USE_FORCE_WIDEMUL_INT128, 1, [Define this symbol to force the use of the (unsigned) __int128 based wide multiplication implementation])
   ;;

src/assumptions.h

@@ -10,6 +10,9 @@
 #include <limits.h>
 
 #include "util.h"
+#if defined(SECP256K1_INT128_NATIVE)
+#include "int128_native.h"
+#endif
 
 /* This library, like most software, relies on a number of compiler implementation defined (but not undefined)
    behaviours. Although the behaviours we require are essentially universal we test them specifically here to
@@ -55,7 +58,7 @@ struct secp256k1_assumption_checker {
 
         /* To int64_t. */
         ((int64_t)(uint64_t)0xB123C456D789E012ULL == (int64_t)-(int64_t)0x4EDC3BA928761FEEULL) &&
-#if defined(SECP256K1_WIDEMUL_INT128)
+#if defined(SECP256K1_INT128_NATIVE)
         ((int64_t)(((uint128_t)0xA1234567B8901234ULL << 64) + 0xC5678901D2345678ULL) == (int64_t)-(int64_t)0x3A9876FE2DCBA988ULL) &&
         (((int64_t)(int128_t)(((uint128_t)0xB1C2D3E4F5A6B7C8ULL << 64) + 0xD9E0F1A2B3C4D5E6ULL)) == (int64_t)(uint64_t)0xD9E0F1A2B3C4D5E6ULL) &&
         (((int64_t)(int128_t)(((uint128_t)0xABCDEF0123456789ULL << 64) + 0x0123456789ABCDEFULL)) == (int64_t)(uint64_t)0x0123456789ABCDEFULL) &&
@@ -71,7 +74,7 @@ struct secp256k1_assumption_checker {
         ((((int16_t)0xE9AC) >> 4) == (int16_t)(uint16_t)0xFE9A) &&
         ((((int32_t)0x937C918A) >> 9) == (int32_t)(uint32_t)0xFFC9BE48) &&
         ((((int64_t)0xA8B72231DF9CF4B9ULL) >> 19) == (int64_t)(uint64_t)0xFFFFF516E4463BF3ULL) &&
-#if defined(SECP256K1_WIDEMUL_INT128)
+#if defined(SECP256K1_INT128_NATIVE)
         ((((int128_t)(((uint128_t)0xCD833A65684A0DBCULL << 64) + 0xB349312F71EA7637ULL)) >> 39) == (int128_t)(((uint128_t)0xFFFFFFFFFF9B0674ULL << 64) + 0xCAD0941B79669262ULL)) &&
 #endif
     1) * 2 - 1];

src/bench_whitelist.c

@@ -11,6 +11,7 @@
 #include "util.h"
 #include "bench.h"
 #include "hash_impl.h"
+#include "int128_impl.h"
 #include "scalar_impl.h"
 #include "testrand_impl.h"
 

src/ecmult_impl.h

@@ -200,9 +200,15 @@ static int secp256k1_ecmult_wnaf(int *wnaf, int len, const secp256k1_scalar *a,
         bit += now;
     }
 #ifdef VERIFY
-    CHECK(carry == 0);
-    while (bit < 256) {
-        CHECK(secp256k1_scalar_get_bits(&s, bit++, 1) == 0);
+    {
+        int verify_bit = bit;
+
+        VERIFY_CHECK(carry == 0);
+
+        while (verify_bit < 256) {
+            VERIFY_CHECK(secp256k1_scalar_get_bits(&s, verify_bit, 1) == 0);
+            verify_bit++;
+        }
     }
 #endif
     return last_set_bit + 1;