Skip to content

Commit

Permalink
Merge bitcoin#835: Don't use reserved identifiers memczero and benchm…
Browse files Browse the repository at this point in the history
…ark_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#829 and bitcoin#833. Fixes bitcoin#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#833. I preferred a separate PR because it makes it easier to see the results of Travis in bitcoin#833.

ACKs for top commit:
  sipa:
    utACK 1f4dd03
  jonasnick:
    ACK 1f4dd03

Tree-SHA512: c0ec92798f3c94f3ef6ac69b3f0f39a39257a32be9d9a068832cece1ebe64c89848b70e44652fc397004b8b240883ac4bc0c8f95abbe4ba4b028de120e6734bf
  • Loading branch information
sipa committed Nov 4, 2020
2 parents d0a83f7 + 1f4dd03 commit 9e5939d
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 23 deletions.
8 changes: 4 additions & 4 deletions src/bench_sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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++) {
Expand All @@ -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);

Expand Down
16 changes: 8 additions & 8 deletions src/bench_verify.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/modules/extrakeys/main_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/modules/schnorrsig/main_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/secp256k1.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -5723,7 +5723,7 @@ int main(int argc, char **argv) {
#endif

/* util tests */
run_memczero_test();
run_secp256k1_memczero_test();

run_cmov_tests();

Expand Down
10 changes: 8 additions & 2 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 */

0 comments on commit 9e5939d

Please sign in to comment.