From 1a6be5745fcf9f90e4218b73712b71ea06361792 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 9 Mar 2022 20:55:08 +0100 Subject: [PATCH 01/17] bench: Make benchmarks compile on MSVC --- src/bench.h | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/bench.h b/src/bench.h index aa275fe91..611ba11f0 100644 --- a/src/bench.h +++ b/src/bench.h @@ -7,15 +7,31 @@ #ifndef SECP256K1_BENCH_H #define SECP256K1_BENCH_H +#include #include #include #include -#include "sys/time.h" + +#if (defined(_MSC_VER) && _MSC_VER >= 1900) +# include +#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) From cca8cbbac84624fd350efc4086af25a06dcf8090 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 9 Mar 2022 20:57:06 +0100 Subject: [PATCH 02/17] configure: Output message when checking for valgrind --- build-aux/m4/bitcoin_secp.m4 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build-aux/m4/bitcoin_secp.m4 b/build-aux/m4/bitcoin_secp.m4 index dda770e00..61ca3c23d 100644 --- a/build-aux/m4/bitcoin_secp.m4 +++ b/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) From 1cc09414149d0c0c6a4a500d83efc3bd66f3ebcd Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 9 Mar 2022 20:58:38 +0100 Subject: [PATCH 03/17] configure: Don't abort if the compiler does not define __STDC__ This removes a check for $ac_cv_prog_cc_c89 which is set by AC_PROG_CC if defined(__STDC__) in the preprocessor. (Standard compliant compilers are supposed to define __STDC__ to 1 but the value is actually not checked here.) Unfortunately, MSVC doesn't define it, so configure fails for MSVC. This check is not very useful in practice. Over 30 years after C89 has been released, there are no C compilers out there that are not sufficiently compliant with C89 for the project. The only practically relevant case was that the check rejected C++ compilers. A different method to reject C++ compilers will be introduced in a later commit. --- configure.ac | 3 --- 1 file changed, 3 deletions(-) diff --git a/configure.ac b/configure.ac index 16a492071..144cfb9ae 100644 --- a/configure.ac +++ b/configure.ac @@ -35,9 +35,6 @@ m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) PKG_PROG_PKG_CONFIG 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 From 3b4f3d0d46dd278fbe4ffa68b1b6e14e3ea3b17f Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 17 Mar 2022 22:26:19 +0100 Subject: [PATCH 04/17] build: Reject C++ compilers in the preprocessor --- .cirrus.yml | 7 +++---- src/secp256k1.c | 11 +++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 271cbe5f5..94d1811ba 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -312,10 +312,9 @@ task: name: "C++ -fpermissive" << : *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: EXPERIMENTAL: yes ECDH: yes diff --git a/src/secp256k1.c b/src/secp256k1.c index 8f34c3528..96102d365 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -4,6 +4,17 @@ * file COPYING or https://www.opensource.org/licenses/mit-license.php.* ***********************************************************************/ +/* This is a C project. It should not be compiled with a C++ compiler, + * and we error out if we detect one. + * + * We still want to be able to test the project with a C++ compiler + * because it is still good to know if this will lead to real trouble, so + * there is a possibility to override the check. But be warned that + * compiling with a C++ compiler is not supported. */ +#if defined(__cplusplus) && !defined(SECP256K1_CPLUSPLUS_TEST_OVERRIDE) +#error Trying to compile a C project with a C++ compiler. +#endif + #define SECP256K1_BUILD #include "../include/secp256k1.h" From 09f3d71c51a9621653d766e2fe7e657534e57bd6 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 9 Mar 2022 21:06:43 +0100 Subject: [PATCH 05/17] configure: Add a few CFLAGS for MSVC --- configure.ac | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/configure.ac b/configure.ac index 144cfb9ae..9f33e1690 100644 --- a/configure.ac +++ b/configure.ac @@ -86,23 +86,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) From 2f984ffc45eba89faa9e79da3d5d5bd50a6c1c3d Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Sun, 26 Dec 2021 22:06:32 +0700 Subject: [PATCH 06/17] Save negations in var-time group addition - Updated _gej_add_var, _gej_add_ge_var, _gej_add_zinv_var - 2 fewer _fe_negate in each method - Updated operation counts and standardize layout - Added internal benchmark for _gej_add_zinv_var - Update sage files (fixed by Tim Ruffing) --- sage/prove_group_implementations.sage | 77 +++++++++------------ src/bench_internal.c | 10 +++ src/group_impl.h | 99 +++++++++++++++++---------- 3 files changed, 105 insertions(+), 81 deletions(-) diff --git a/sage/prove_group_implementations.sage b/sage/prove_group_implementations.sage index 96ce33506..652bd87f1 100644 --- a/sage/prove_group_implementations.sage +++ b/sage/prove_group_implementations.sage @@ -40,29 +40,26 @@ def formula_secp256k1_gej_add_var(branch, a, b): s2 = s2 * a.Z h = -u1 h = h + u2 - i = -s1 - i = i + s2 + i = -s2 + i = i + s1 if branch == 2: r = formula_secp256k1_gej_double_var(a) return (constraints(), constraints(zero={h : 'h=0', i : 'i=0', a.Infinity : 'a_finite', b.Infinity : 'b_finite'}), r) if branch == 3: return (constraints(), constraints(zero={h : 'h=0', a.Infinity : 'a_finite', b.Infinity : 'b_finite'}, nonzero={i : 'i!=0'}), point_at_infinity()) - i2 = i^2 + t = h * b.Z + rz = a.Z * t h2 = h^2 + h2 = -h2 h3 = h2 * h - h = h * b.Z - rz = a.Z * h t = u1 * h2 - rx = t - rx = rx * 2 + rx = i^2 rx = rx + h3 - rx = -rx - rx = rx + i2 - ry = -rx - ry = ry + t - ry = ry * i + rx = rx + t + rx = rx + t + t = t + rx + ry = t * i h3 = h3 * s1 - h3 = -h3 ry = ry + h3 return (constraints(), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite'}, nonzero={h : 'h!=0'}), jacobianpoint(rx, ry, rz)) @@ -80,28 +77,25 @@ def formula_secp256k1_gej_add_ge_var(branch, a, b): s2 = s2 * a.Z h = -u1 h = h + u2 - i = -s1 - i = i + s2 + i = -s2 + i = i + s1 if (branch == 2): r = formula_secp256k1_gej_double_var(a) return (constraints(zero={b.Z - 1 : 'b.z=1'}), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite', h : 'h=0', i : 'i=0'}), r) if (branch == 3): return (constraints(zero={b.Z - 1 : 'b.z=1'}), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite', h : 'h=0'}, nonzero={i : 'i!=0'}), point_at_infinity()) - i2 = i^2 - h2 = h^2 - h3 = h * h2 rz = a.Z * h + h2 = h^2 + h2 = -h2 + h3 = h2 * h t = u1 * h2 - rx = t - rx = rx * 2 + rx = i^2 rx = rx + h3 - rx = -rx - rx = rx + i2 - ry = -rx - ry = ry + t - ry = ry * i + rx = rx + t + rx = rx + t + t = t + rx + ry = t * i h3 = h3 * s1 - h3 = -h3 ry = ry + h3 return (constraints(zero={b.Z - 1 : 'b.z=1'}), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite'}, nonzero={h : 'h!=0'}), jacobianpoint(rx, ry, rz)) @@ -109,14 +103,15 @@ def formula_secp256k1_gej_add_zinv_var(branch, a, b): """libsecp256k1's secp256k1_gej_add_zinv_var""" bzinv = b.Z^(-1) if branch == 0: - return (constraints(), constraints(nonzero={b.Infinity : 'b_infinite'}), a) - if branch == 1: + rinf = b.Infinity bzinv2 = bzinv^2 bzinv3 = bzinv2 * bzinv rx = b.X * bzinv2 ry = b.Y * bzinv3 rz = 1 - return (constraints(), constraints(zero={b.Infinity : 'b_finite'}, nonzero={a.Infinity : 'a_infinite'}), jacobianpoint(rx, ry, rz)) + return (constraints(), constraints(nonzero={a.Infinity : 'a_infinite'}), jacobianpoint(rx, ry, rz, rinf)) + if branch == 1: + return (constraints(), constraints(zero={a.Infinity : 'a_finite'}, nonzero={b.Infinity : 'b_infinite'}), a) azz = a.Z * bzinv z12 = azz^2 u1 = a.X @@ -126,29 +121,25 @@ def formula_secp256k1_gej_add_zinv_var(branch, a, b): s2 = s2 * azz h = -u1 h = h + u2 - i = -s1 - i = i + s2 + i = -s2 + i = i + s1 if branch == 2: r = formula_secp256k1_gej_double_var(a) return (constraints(), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite', h : 'h=0', i : 'i=0'}), r) if branch == 3: return (constraints(), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite', h : 'h=0'}, nonzero={i : 'i!=0'}), point_at_infinity()) - i2 = i^2 + rz = a.Z * h h2 = h^2 - h3 = h * h2 - rz = a.Z - rz = rz * h + h2 = -h2 + h3 = h2 * h t = u1 * h2 - rx = t - rx = rx * 2 + rx = i^2 rx = rx + h3 - rx = -rx - rx = rx + i2 - ry = -rx - ry = ry + t - ry = ry * i + rx = rx + t + rx = rx + t + t = t + rx + ry = t * i h3 = h3 * s1 - h3 = -h3 ry = ry + h3 return (constraints(), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite'}, nonzero={h : 'h!=0'}), jacobianpoint(rx, ry, rz)) diff --git a/src/bench_internal.c b/src/bench_internal.c index 3c145f306..7eb3af28d 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -254,6 +254,15 @@ void bench_group_add_affine_var(void* arg, int iters) { } } +void bench_group_add_zinv_var(void* arg, int iters) { + int i; + bench_inv *data = (bench_inv*)arg; + + for (i = 0; i < iters; i++) { + secp256k1_gej_add_zinv_var(&data->gej[0], &data->gej[0], &data->ge[1], &data->gej[0].y); + } +} + void bench_group_to_affine_var(void* arg, int iters) { int i; bench_inv *data = (bench_inv*)arg; @@ -376,6 +385,7 @@ int main(int argc, char **argv) { if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_var", bench_group_add_var, bench_setup, NULL, &data, 10, iters*10); if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_affine", bench_group_add_affine, bench_setup, NULL, &data, 10, iters*10); if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_affine_var", bench_group_add_affine_var, bench_setup, NULL, &data, 10, iters*10); + if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_zinv_var", bench_group_add_zinv_var, bench_setup, NULL, &data, 10, iters*10); if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "to_affine")) run_benchmark("group_to_affine_var", bench_group_to_affine_var, bench_setup, NULL, &data, 10, iters); if (d || have_flag(argc, argv, "ecmult") || have_flag(argc, argv, "wnaf")) run_benchmark("wnaf_const", bench_wnaf_const, bench_setup, NULL, &data, 10, iters); diff --git a/src/group_impl.h b/src/group_impl.h index b19b02a01..63735ab68 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -330,15 +330,14 @@ static void secp256k1_gej_double_var(secp256k1_gej *r, const secp256k1_gej *a, s } static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_gej *b, secp256k1_fe *rzr) { - /* Operations: 12 mul, 4 sqr, 2 normalize, 12 mul_int/add/negate */ - secp256k1_fe z22, z12, u1, u2, s1, s2, h, i, i2, h2, h3, t; + /* 12 mul, 4 sqr, 11 add/negate/normalizes_to_zero (ignoring special cases) */ + secp256k1_fe z22, z12, u1, u2, s1, s2, h, i, h2, h3, t; if (a->infinity) { VERIFY_CHECK(rzr == NULL); *r = *b; return; } - if (b->infinity) { if (rzr != NULL) { secp256k1_fe_set_int(rzr, 1); @@ -347,7 +346,6 @@ static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, cons return; } - r->infinity = 0; secp256k1_fe_sqr(&z22, &b->z); secp256k1_fe_sqr(&z12, &a->z); secp256k1_fe_mul(&u1, &a->x, &z22); @@ -355,7 +353,7 @@ static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, cons secp256k1_fe_mul(&s1, &a->y, &z22); secp256k1_fe_mul(&s1, &s1, &b->z); secp256k1_fe_mul(&s2, &b->y, &z12); secp256k1_fe_mul(&s2, &s2, &a->z); secp256k1_fe_negate(&h, &u1, 1); secp256k1_fe_add(&h, &u2); - secp256k1_fe_negate(&i, &s1, 1); secp256k1_fe_add(&i, &s2); + secp256k1_fe_negate(&i, &s2, 1); secp256k1_fe_add(&i, &s1); if (secp256k1_fe_normalizes_to_zero_var(&h)) { if (secp256k1_fe_normalizes_to_zero_var(&i)) { secp256k1_gej_double_var(r, a, rzr); @@ -367,24 +365,33 @@ static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, cons } return; } - secp256k1_fe_sqr(&i2, &i); - secp256k1_fe_sqr(&h2, &h); - secp256k1_fe_mul(&h3, &h, &h2); - secp256k1_fe_mul(&h, &h, &b->z); + + r->infinity = 0; + secp256k1_fe_mul(&t, &h, &b->z); if (rzr != NULL) { - *rzr = h; + *rzr = t; } - secp256k1_fe_mul(&r->z, &a->z, &h); + secp256k1_fe_mul(&r->z, &a->z, &t); + + secp256k1_fe_sqr(&h2, &h); + secp256k1_fe_negate(&h2, &h2, 1); + secp256k1_fe_mul(&h3, &h2, &h); secp256k1_fe_mul(&t, &u1, &h2); - r->x = t; secp256k1_fe_mul_int(&r->x, 2); secp256k1_fe_add(&r->x, &h3); secp256k1_fe_negate(&r->x, &r->x, 3); secp256k1_fe_add(&r->x, &i2); - secp256k1_fe_negate(&r->y, &r->x, 5); secp256k1_fe_add(&r->y, &t); secp256k1_fe_mul(&r->y, &r->y, &i); - secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_negate(&h3, &h3, 1); + + secp256k1_fe_sqr(&r->x, &i); + secp256k1_fe_add(&r->x, &h3); + secp256k1_fe_add(&r->x, &t); + secp256k1_fe_add(&r->x, &t); + + secp256k1_fe_add(&t, &r->x); + secp256k1_fe_mul(&r->y, &t, &i); + secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_add(&r->y, &h3); } static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_ge *b, secp256k1_fe *rzr) { - /* 8 mul, 3 sqr, 4 normalize, 12 mul_int/add/negate */ - secp256k1_fe z12, u1, u2, s1, s2, h, i, i2, h2, h3, t; + /* 8 mul, 3 sqr, 13 add/negate/normalize_weak/normalizes_to_zero (ignoring special cases) */ + secp256k1_fe z12, u1, u2, s1, s2, h, i, h2, h3, t; if (a->infinity) { VERIFY_CHECK(rzr == NULL); secp256k1_gej_set_ge(r, b); @@ -397,7 +404,6 @@ static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, c *r = *a; return; } - r->infinity = 0; secp256k1_fe_sqr(&z12, &a->z); u1 = a->x; secp256k1_fe_normalize_weak(&u1); @@ -405,7 +411,7 @@ static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, c s1 = a->y; secp256k1_fe_normalize_weak(&s1); secp256k1_fe_mul(&s2, &b->y, &z12); secp256k1_fe_mul(&s2, &s2, &a->z); secp256k1_fe_negate(&h, &u1, 1); secp256k1_fe_add(&h, &u2); - secp256k1_fe_negate(&i, &s1, 1); secp256k1_fe_add(&i, &s2); + secp256k1_fe_negate(&i, &s2, 1); secp256k1_fe_add(&i, &s1); if (secp256k1_fe_normalizes_to_zero_var(&h)) { if (secp256k1_fe_normalizes_to_zero_var(&i)) { secp256k1_gej_double_var(r, a, rzr); @@ -417,28 +423,33 @@ static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, c } return; } - secp256k1_fe_sqr(&i2, &i); - secp256k1_fe_sqr(&h2, &h); - secp256k1_fe_mul(&h3, &h, &h2); + + r->infinity = 0; if (rzr != NULL) { *rzr = h; } secp256k1_fe_mul(&r->z, &a->z, &h); + + secp256k1_fe_sqr(&h2, &h); + secp256k1_fe_negate(&h2, &h2, 1); + secp256k1_fe_mul(&h3, &h2, &h); secp256k1_fe_mul(&t, &u1, &h2); - r->x = t; secp256k1_fe_mul_int(&r->x, 2); secp256k1_fe_add(&r->x, &h3); secp256k1_fe_negate(&r->x, &r->x, 3); secp256k1_fe_add(&r->x, &i2); - secp256k1_fe_negate(&r->y, &r->x, 5); secp256k1_fe_add(&r->y, &t); secp256k1_fe_mul(&r->y, &r->y, &i); - secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_negate(&h3, &h3, 1); + + secp256k1_fe_sqr(&r->x, &i); + secp256k1_fe_add(&r->x, &h3); + secp256k1_fe_add(&r->x, &t); + secp256k1_fe_add(&r->x, &t); + + secp256k1_fe_add(&t, &r->x); + secp256k1_fe_mul(&r->y, &t, &i); + secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_add(&r->y, &h3); } static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_ge *b, const secp256k1_fe *bzinv) { - /* 9 mul, 3 sqr, 4 normalize, 12 mul_int/add/negate */ - secp256k1_fe az, z12, u1, u2, s1, s2, h, i, i2, h2, h3, t; + /* 9 mul, 3 sqr, 13 add/negate/normalize_weak/normalizes_to_zero (ignoring special cases) */ + secp256k1_fe az, z12, u1, u2, s1, s2, h, i, h2, h3, t; - if (b->infinity) { - *r = *a; - return; - } if (a->infinity) { secp256k1_fe bzinv2, bzinv3; r->infinity = b->infinity; @@ -449,7 +460,10 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, secp256k1_fe_set_int(&r->z, 1); return; } - r->infinity = 0; + if (b->infinity) { + *r = *a; + return; + } /** We need to calculate (rx,ry,rz) = (ax,ay,az) + (bx,by,1/bzinv). Due to * secp256k1's isomorphism we can multiply the Z coordinates on both sides @@ -467,7 +481,7 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, s1 = a->y; secp256k1_fe_normalize_weak(&s1); secp256k1_fe_mul(&s2, &b->y, &z12); secp256k1_fe_mul(&s2, &s2, &az); secp256k1_fe_negate(&h, &u1, 1); secp256k1_fe_add(&h, &u2); - secp256k1_fe_negate(&i, &s1, 1); secp256k1_fe_add(&i, &s2); + secp256k1_fe_negate(&i, &s2, 1); secp256k1_fe_add(&i, &s1); if (secp256k1_fe_normalizes_to_zero_var(&h)) { if (secp256k1_fe_normalizes_to_zero_var(&i)) { secp256k1_gej_double_var(r, a, NULL); @@ -476,14 +490,23 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, } return; } - secp256k1_fe_sqr(&i2, &i); + + r->infinity = 0; + secp256k1_fe_mul(&r->z, &a->z, &h); + secp256k1_fe_sqr(&h2, &h); - secp256k1_fe_mul(&h3, &h, &h2); - r->z = a->z; secp256k1_fe_mul(&r->z, &r->z, &h); + secp256k1_fe_negate(&h2, &h2, 1); + secp256k1_fe_mul(&h3, &h2, &h); secp256k1_fe_mul(&t, &u1, &h2); - r->x = t; secp256k1_fe_mul_int(&r->x, 2); secp256k1_fe_add(&r->x, &h3); secp256k1_fe_negate(&r->x, &r->x, 3); secp256k1_fe_add(&r->x, &i2); - secp256k1_fe_negate(&r->y, &r->x, 5); secp256k1_fe_add(&r->y, &t); secp256k1_fe_mul(&r->y, &r->y, &i); - secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_negate(&h3, &h3, 1); + + secp256k1_fe_sqr(&r->x, &i); + secp256k1_fe_add(&r->x, &h3); + secp256k1_fe_add(&r->x, &t); + secp256k1_fe_add(&r->x, &t); + + secp256k1_fe_add(&t, &r->x); + secp256k1_fe_mul(&r->y, &t, &i); + secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_add(&r->y, &h3); } From 7efc9835a977c6400f0f024f19fda47710151dc1 Mon Sep 17 00:00:00 2001 From: Sprite Date: Fri, 15 Apr 2022 16:41:43 +0800 Subject: [PATCH 07/17] Fix the false positive of `SECP_64BIT_ASM_CHECK` --- build-aux/m4/bitcoin_secp.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-aux/m4/bitcoin_secp.m4 b/build-aux/m4/bitcoin_secp.m4 index dda770e00..9cb54de09 100644 --- a/build-aux/m4/bitcoin_secp.m4 +++ b/build-aux/m4/bitcoin_secp.m4 @@ -1,7 +1,7 @@ dnl escape "$0x" below using the m4 quadrigaph @S|@, and escape it again with a \ for the shell. AC_DEFUN([SECP_64BIT_ASM_CHECK],[ AC_MSG_CHECKING(for x86_64 assembly availability) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ +AC_LINK_IFELSE([AC_LANG_PROGRAM([[ #include ]],[[ uint64_t a = 11, tmp; __asm__ __volatile__("movq \@S|@0x100000000,%1; mulq %%rsi" : "+a"(a) : "S"(tmp) : "cc", "%rdx"); From 6f6cab9989a4d3f4a28e3cdbfacc4e3e1e55c843 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 4 May 2022 20:02:52 +0000 Subject: [PATCH 08/17] abi: Don't export symbols in static Windows libraries libtool takes care of building both object versions, we just need to pick the right one to export symbols. --- include/secp256k1.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 86ab7e556..dddab346a 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -141,9 +141,13 @@ typedef int (*secp256k1_nonce_function)( # define SECP256K1_NO_BUILD #endif +/** At secp256k1 build-time DLL_EXPORT is defined when building objects destined + * for a shared library, but not for those intended for static libraries. + */ + #ifndef SECP256K1_API # if defined(_WIN32) -# ifdef SECP256K1_BUILD +# if defined(SECP256K1_BUILD) && defined(DLL_EXPORT) # define SECP256K1_API __declspec(dllexport) # else # define SECP256K1_API From bd81f4140a4228b1df3a9f631e2d207a197ae614 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 9 Mar 2022 21:08:03 +0100 Subject: [PATCH 09/17] schnorrsig bench: Suppress a stupid warning in MSVC --- src/modules/schnorrsig/bench_impl.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/modules/schnorrsig/bench_impl.h b/src/modules/schnorrsig/bench_impl.h index 41f393c84..84a172742 100644 --- a/src/modules/schnorrsig/bench_impl.h +++ b/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); } From 2be6ba0fedd0d2d62ba6f346d7ced7abde0d66e4 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Mon, 14 Mar 2022 17:15:13 +0100 Subject: [PATCH 10/17] configure: Convince autotools to work with MSVC's archiver lib.exe --- configure.ac | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 9f33e1690..a98b834f3 100644 --- a/configure.ac +++ b/configure.ac @@ -32,14 +32,19 @@ AM_INIT_AUTOMAKE([1.11.2 foreign subdir-objects]) # Make the compilation flags quiet unless V=1 is used. m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) -PKG_PROG_PKG_CONFIG - AC_PROG_CC 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 From 9efc2e5221560d19dd750e0ba32c03d4ee091227 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 10 Mar 2022 20:09:41 +0100 Subject: [PATCH 11/17] ci: Add MSVC builds This adds MSVC builds built on Linux using wine. This requires some settings of tools and flags because the autotools support for MSVC is naturally somewhat limited. The advantage of this approach is that it is compatible with our existing CI scripts, so there's no need to write a Windows CI script (in PowerShell or similar). If we want to test building and running on Windows native (e.g., as supported by Cirrus CI) we could still do this in the future. Another advantage of this approach is that contributors can simply use the docker image if they need a MSVC installation in a non-Windows environment. This commit also improves the Dockerfile by grouping RUN commands according to Docker docs: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run --- .cirrus.yml | 36 ++++++++++++++++++++++++++++++++++++ ci/cirrus.sh | 1 + ci/linux-debian.Dockerfile | 31 +++++++++++++++++++++---------- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 94d1811ba..9ff908ffe 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -263,6 +263,42 @@ task: - ./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 + << : *CAT_LOGS + # Sanitizers task: << : *LINUX_CONTAINER diff --git a/ci/cirrus.sh b/ci/cirrus.sh index b85f012d3..929c16d51 100755 --- a/ci/cirrus.sh +++ b/ci/cirrus.sh @@ -9,6 +9,7 @@ env >> test_env.log $CC -v || true valgrind --version || true +$WRAPPER_CMD --version || true ./autogen.sh diff --git a/ci/linux-debian.Dockerfile b/ci/linux-debian.Dockerfile index 5cccbb556..a83a4e36d 100644 --- a/ci/linux-debian.Dockerfile +++ b/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 From 3fb3269c22c25de3b720ad139dcf4e3cff9eda1a Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 16 Mar 2022 00:15:33 +0100 Subject: [PATCH 12/17] ci: Add 32-bit MinGW64 build This commit also raises the TEST_ITERS for wine tasks to the default. The overhead of wine is negligible, so we can certainly afford the same number of iterations as for native Linux tests. --- .cirrus.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 9ff908ffe..0cdf63791 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -246,18 +246,22 @@ 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 EXPERIMENTAL: 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 From 51f296a46c0b318b8dd572ef9ac3bb3a4140ae63 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 16 Mar 2022 14:33:37 +0100 Subject: [PATCH 13/17] ci: Run persistent wineserver to speed up wine --- ci/cirrus.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ci/cirrus.sh b/ci/cirrus.sh index 929c16d51..23f7c6d0d 100755 --- a/ci/cirrus.sh +++ b/ci/cirrus.sh @@ -5,6 +5,15 @@ 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 @@ -64,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 From 8dc4b03341c85a3be91e559d05771c51e60b0eba Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 17 Mar 2022 22:29:25 +0100 Subject: [PATCH 14/17] ci: Add a C++ job that compiles the public headers without -fpermissive --- .cirrus.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 0cdf63791..647457c8d 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -349,7 +349,7 @@ task: << : *CAT_LOGS task: - name: "C++ -fpermissive" + name: "C++ -fpermissive (entire project)" << : *LINUX_CONTAINER env: CC: g++ @@ -365,6 +365,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 From 49e2acd927ce9eb806cc10f3a1fd89a9ddd081e2 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 29 Jun 2022 11:12:00 +0200 Subject: [PATCH 15/17] configure: Improve rationale for WERROR_CFLAGS --- configure.ac | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index a98b834f3..cd5b30441 100644 --- a/configure.ac +++ b/configure.ac @@ -342,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" ### From 1827c9bf2b7d4bf6a78c2f387bdb3e885a71e292 Mon Sep 17 00:00:00 2001 From: siv2r Date: Wed, 29 Jun 2022 20:24:11 +0530 Subject: [PATCH 16/17] scratch_destroy: move VERIFY_CHECK after invalid scrach space check --- src/scratch_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scratch_impl.h b/src/scratch_impl.h index 688e18eb6..f71a20b96 100644 --- a/src/scratch_impl.h +++ b/src/scratch_impl.h @@ -25,11 +25,11 @@ static secp256k1_scratch* secp256k1_scratch_create(const secp256k1_callback* err static void secp256k1_scratch_destroy(const secp256k1_callback* error_callback, secp256k1_scratch* scratch) { if (scratch != NULL) { - VERIFY_CHECK(scratch->alloc_size == 0); /* all checkpoints should be applied */ if (secp256k1_memcmp_var(scratch->magic, "scratch", 8) != 0) { secp256k1_callback_call(error_callback, "invalid scratch space"); return; } + VERIFY_CHECK(scratch->alloc_size == 0); /* all checkpoints should be applied */ memset(scratch->magic, 0, sizeof(scratch->magic)); free(scratch); } From 069aba812542642986786f348a08af697b7e53c7 Mon Sep 17 00:00:00 2001 From: henopied <13500516+henopied@users.noreply.github.com> Date: Wed, 29 Jun 2022 20:08:47 -0500 Subject: [PATCH 17/17] Fix sepc256k1 -> secp256k1 typo in group.h --- src/group.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/group.h b/src/group.h index bb7dae1cf..585457d93 100644 --- a/src/group.h +++ b/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 */