Merge bitcoin-core#835: Don't use reserved identifiers memczero and b…

…enchmark_verify_t 1f4dd03 Typedef (u)int128_t only when they're not provided by the compiler (Tim Ruffing) e89278f Don't use reserved identifiers memczero and benchmark_verify_t (Tim Ruffing) Pull request description: As identified in bitcoin-core#829 and bitcoin-core#833. Fixes bitcoin-core#829. Since we touch this anyway, this commit additionally makes the identifiers in the benchmark files a little bit more consistent. This is necessary before we can merge bitcoin-core#833. I preferred a separate PR because it makes it easier to see the results of Travis in bitcoin-core#833. ACKs for top commit: sipa: utACK 1f4dd03 jonasnick: ACK 1f4dd03 Tree-SHA512: c0ec92798f3c94f3ef6ac69b3f0f39a39257a32be9d9a068832cece1ebe64c89848b70e44652fc397004b8b240883ac4bc0c8f95abbe4ba4b028de120e6734bf
real-or-random · Nov 4, 2020 · 9e5939d · 9e5939d
2 parents d0a83f7 + 1f4dd03
commit 9e5939d
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 23 deletions.
diff --git a/src/bench_sign.c b/src/bench_sign.c
@@ -12,11 +12,11 @@ typedef struct {
     secp256k1_context* ctx;
     unsigned char msg[32];
     unsigned char key[32];
-} bench_sign;
+} bench_sign_data;
 
 static void bench_sign_setup(void* arg) {
     int i;
-    bench_sign *data = (bench_sign*)arg;
+    bench_sign_data *data = (bench_sign_data*)arg;
 
     for (i = 0; i < 32; i++) {
         data->msg[i] = i + 1;
@@ -28,7 +28,7 @@ static void bench_sign_setup(void* arg) {
 
 static void bench_sign_run(void* arg, int iters) {
     int i;
-    bench_sign *data = (bench_sign*)arg;
+    bench_sign_data *data = (bench_sign_data*)arg;
 
     unsigned char sig[74];
     for (i = 0; i < iters; i++) {
@@ -45,7 +45,7 @@ static void bench_sign_run(void* arg, int iters) {
 }
 
 int main(void) {
-    bench_sign data;
+    bench_sign_data data;
 
     int iters = get_iters(20000);
 

diff --git a/src/bench_verify.c b/src/bench_verify.c
@@ -29,11 +29,11 @@ typedef struct {
 #ifdef ENABLE_OPENSSL_TESTS
     EC_GROUP* ec_group;
 #endif
-} benchmark_verify_t;
+} bench_verify_data;
 
-static void benchmark_verify(void* arg, int iters) {
+static void bench_verify(void* arg, int iters) {
     int i;
-    benchmark_verify_t* data = (benchmark_verify_t*)arg;
+    bench_verify_data* data = (bench_verify_data*)arg;
 
     for (i = 0; i < iters; i++) {
         secp256k1_pubkey pubkey;
@@ -51,9 +51,9 @@ static void benchmark_verify(void* arg, int iters) {
 }
 
 #ifdef ENABLE_OPENSSL_TESTS
-static void benchmark_verify_openssl(void* arg, int iters) {
+static void bench_verify_openssl(void* arg, int iters) {
     int i;
-    benchmark_verify_t* data = (benchmark_verify_t*)arg;
+    bench_verify_data* data = (bench_verify_data*)arg;
 
     for (i = 0; i < iters; i++) {
         data->sig[data->siglen - 1] ^= (i & 0xFF);
@@ -84,7 +84,7 @@ int main(void) {
     int i;
     secp256k1_pubkey pubkey;
     secp256k1_ecdsa_signature sig;
-    benchmark_verify_t data;
+    bench_verify_data data;
 
     int iters = get_iters(20000);
 
@@ -103,10 +103,10 @@ int main(void) {
     data.pubkeylen = 33;
     CHECK(secp256k1_ec_pubkey_serialize(data.ctx, data.pubkey, &data.pubkeylen, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
 
-    run_benchmark("ecdsa_verify", benchmark_verify, NULL, NULL, &data, 10, iters);
+    run_benchmark("ecdsa_verify", bench_verify, NULL, NULL, &data, 10, iters);
 #ifdef ENABLE_OPENSSL_TESTS
     data.ec_group = EC_GROUP_new_by_curve_name(NID_secp256k1);
-    run_benchmark("ecdsa_verify_openssl", benchmark_verify_openssl, NULL, NULL, &data, 10, iters);
+    run_benchmark("ecdsa_verify_openssl", bench_verify_openssl, NULL, NULL, &data, 10, iters);
     EC_GROUP_free(data.ec_group);
 #endif
 

diff --git a/src/modules/extrakeys/main_impl.h b/src/modules/extrakeys/main_impl.h
@@ -180,7 +180,7 @@ int secp256k1_keypair_create(const secp256k1_context* ctx, secp256k1_keypair *ke
 
     ret = secp256k1_ec_pubkey_create_helper(&ctx->ecmult_gen_ctx, &sk, &pk, seckey32);
     secp256k1_keypair_save(keypair, &sk, &pk);
-    memczero(keypair, sizeof(*keypair), !ret);
+    secp256k1_memczero(keypair, sizeof(*keypair), !ret);
 
     secp256k1_scalar_clear(&sk);
     return ret;

diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h
@@ -179,7 +179,7 @@ int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64
     secp256k1_scalar_add(&e, &e, &k);
     secp256k1_scalar_get_b32(&sig64[32], &e);
 
-    memczero(sig64, 64, !ret);
+    secp256k1_memczero(sig64, 64, !ret);
     secp256k1_scalar_clear(&k);
     secp256k1_scalar_clear(&sk);
     memset(seckey, 0, sizeof(seckey));

diff --git a/src/secp256k1.c b/src/secp256k1.c
@@ -582,7 +582,7 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p
 
     ret = secp256k1_ec_pubkey_create_helper(&ctx->ecmult_gen_ctx, &seckey_scalar, &p, seckey);
     secp256k1_pubkey_save(pubkey, &p);
-    memczero(pubkey, sizeof(*pubkey), !ret);
+    secp256k1_memczero(pubkey, sizeof(*pubkey), !ret);
 
     secp256k1_scalar_clear(&seckey_scalar);
     return ret;

diff --git a/src/tests.c b/src/tests.c
@@ -5444,18 +5444,18 @@ void run_ecdsa_openssl(void) {
 # include "modules/schnorrsig/tests_impl.h"
 #endif
 
-void run_memczero_test(void) {
+void run_secp256k1_memczero_test(void) {
     unsigned char buf1[6] = {1, 2, 3, 4, 5, 6};
     unsigned char buf2[sizeof(buf1)];
 
-    /* memczero(..., ..., 0) is a noop. */
+    /* secp256k1_memczero(..., ..., 0) is a noop. */
     memcpy(buf2, buf1, sizeof(buf1));
-    memczero(buf1, sizeof(buf1), 0);
+    secp256k1_memczero(buf1, sizeof(buf1), 0);
     CHECK(secp256k1_memcmp_var(buf1, buf2, sizeof(buf1)) == 0);
 
-    /* memczero(..., ..., 1) zeros the buffer. */
+    /* secp256k1_memczero(..., ..., 1) zeros the buffer. */
     memset(buf2, 0, sizeof(buf2));
-    memczero(buf1, sizeof(buf1) , 1);
+    secp256k1_memczero(buf1, sizeof(buf1) , 1);
     CHECK(secp256k1_memcmp_var(buf1, buf2, sizeof(buf1)) == 0);
 }
 
@@ -5723,7 +5723,7 @@ int main(int argc, char **argv) {
 #endif
 
     /* util tests */
-    run_memczero_test();
+    run_secp256k1_memczero_test();
 
     run_cmov_tests();
 

diff --git a/src/util.h b/src/util.h
@@ -202,7 +202,7 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz
 #endif
 
 /* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */
-static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) {
+static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) {
     unsigned char *p = (unsigned char *)s;
     /* Access flag with a volatile-qualified lvalue.
        This prevents clang from figuring out (after inlining) that flag can
@@ -260,14 +260,20 @@ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag)
 # define SECP256K1_WIDEMUL_INT128 1
 #elif defined(USE_FORCE_WIDEMUL_INT64)
 # define SECP256K1_WIDEMUL_INT64 1
-#elif defined(__SIZEOF_INT128__)
+#elif defined(UINT128_MAX) || defined(__SIZEOF_INT128__)
 # define SECP256K1_WIDEMUL_INT128 1
 #else
 # define SECP256K1_WIDEMUL_INT64 1
 #endif
 #if defined(SECP256K1_WIDEMUL_INT128)
+# if !defined(UINT128_MAX) && defined(__SIZEOF_INT128__)
 SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t;
 SECP256K1_GNUC_EXT typedef __int128 int128_t;
+#define UINT128_MAX ((uint128_t)(-1))
+#define INT128_MAX ((int128_t)(UINT128_MAX >> 1))
+#define INT128_MIN (-INT128_MAX - 1)
+/* No (U)INT128_C macros because compilers providing __int128 do not support 128-bit literals.  */
+# endif
 #endif
 
 #endif /* SECP256K1_UTIL_H */