Skip to content

Commit 543f9c5

Browse files
real-or-randomFabcien
authored andcommitted
[SECP256K1] Don't use reserved identifiers memczero and benchmark_verify_t
Summary: ``` As identified in #829 and #833. Fixes #829. Since we touch this anyway, this commit additionally makes the identifiers in the benchmark files a little bit more consistent. Typedef (u)int128_t only when they're not provided by the compiler ``` Backport of [[bitcoin-core/secp256k1#835 | secp256k1#835]] Test Plan: ninja check-secp256k1 bench-secp256k1 Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D9373
1 parent fc71d00 commit 543f9c5

File tree

7 files changed

+33
-27
lines changed

7 files changed

+33
-27
lines changed

src/secp256k1/src/bench_sign.c

+5-5
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ typedef struct {
1717
secp256k1_context* ctx;
1818
unsigned char msg[32];
1919
unsigned char key[32];
20-
} bench_sign;
20+
} bench_sign_data;
2121

2222
static void bench_sign_setup(void* arg) {
2323
int i;
24-
bench_sign *data = (bench_sign*)arg;
24+
bench_sign_data *data = (bench_sign_data*)arg;
2525

2626
for (i = 0; i < 32; i++) {
2727
data->msg[i] = i + 1;
@@ -33,7 +33,7 @@ static void bench_sign_setup(void* arg) {
3333

3434
static void bench_sign_run(void* arg, int iters) {
3535
int i;
36-
bench_sign *data = (bench_sign*)arg;
36+
bench_sign_data *data = (bench_sign_data*)arg;
3737

3838
unsigned char sig[74];
3939
for (i = 0; i < iters; i++) {
@@ -52,7 +52,7 @@ static void bench_sign_run(void* arg, int iters) {
5252
#ifdef ENABLE_MODULE_SCHNORR
5353
static void bench_schnorr_sign_run(void* arg, int iters) {
5454
int i,j;
55-
bench_sign *data = (bench_sign*)arg;
55+
bench_sign_data *data = (bench_sign_data*)arg;
5656

5757
unsigned char sig[64];
5858
for (i = 0; i < iters; i++) {
@@ -66,7 +66,7 @@ static void bench_schnorr_sign_run(void* arg, int iters) {
6666
#endif
6767

6868
int main(void) {
69-
bench_sign data;
69+
bench_sign_data data;
7070

7171
int iters = get_iters(20000);
7272

src/secp256k1/src/bench_verify.c

+11-11
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ typedef struct {
3333
#ifdef ENABLE_OPENSSL_TESTS
3434
EC_GROUP* ec_group;
3535
#endif
36-
} benchmark_verify_t;
36+
} bench_verify_data;
3737

38-
static void benchmark_verify(void* arg, int iters) {
38+
static void bench_verify(void* arg, int iters) {
3939
int i;
40-
benchmark_verify_t* data = (benchmark_verify_t*)arg;
40+
bench_verify_data* data = (bench_verify_data*)arg;
4141

4242
for (i = 0; i < iters; i++) {
4343
secp256k1_pubkey pubkey;
@@ -55,9 +55,9 @@ static void benchmark_verify(void* arg, int iters) {
5555
}
5656

5757
#ifdef ENABLE_OPENSSL_TESTS
58-
static void benchmark_verify_openssl(void* arg, int iters) {
58+
static void bench_verify_openssl(void* arg, int iters) {
5959
int i;
60-
benchmark_verify_t* data = (benchmark_verify_t*)arg;
60+
bench_verify_data* data = (bench_verify_data*)arg;
6161

6262
for (i = 0; i < iters; i++) {
6363
data->sig[data->siglen - 1] ^= (i & 0xFF);
@@ -85,9 +85,9 @@ static void benchmark_verify_openssl(void* arg, int iters) {
8585
#endif
8686

8787
#ifdef ENABLE_MODULE_SCHNORR
88-
static void benchmark_schnorr_verify(void* arg, int iters) {
88+
static void bench_schnorr_verify(void* arg, int iters) {
8989
int i;
90-
benchmark_verify_t* data = (benchmark_verify_t*)arg;
90+
bench_verify_data* data = (bench_verify_data*)arg;
9191

9292
for (i = 0; i < iters; i++) {
9393
secp256k1_pubkey pubkey;
@@ -107,7 +107,7 @@ int main(void) {
107107
int i;
108108
secp256k1_pubkey pubkey;
109109
secp256k1_ecdsa_signature sig;
110-
benchmark_verify_t data;
110+
bench_verify_data data;
111111

112112
int iters = get_iters(20000);
113113

@@ -126,16 +126,16 @@ int main(void) {
126126
data.pubkeylen = 33;
127127
CHECK(secp256k1_ec_pubkey_serialize(data.ctx, data.pubkey, &data.pubkeylen, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
128128

129-
run_benchmark("ecdsa_verify", benchmark_verify, NULL, NULL, &data, 10, iters);
129+
run_benchmark("ecdsa_verify", bench_verify, NULL, NULL, &data, 10, iters);
130130
#ifdef ENABLE_OPENSSL_TESTS
131131
data.ec_group = EC_GROUP_new_by_curve_name(NID_secp256k1);
132-
run_benchmark("ecdsa_verify_openssl", benchmark_verify_openssl, NULL, NULL, &data, 10, iters);
132+
run_benchmark("ecdsa_verify_openssl", bench_verify_openssl, NULL, NULL, &data, 10, iters);
133133
EC_GROUP_free(data.ec_group);
134134
#endif
135135
#ifdef ENABLE_MODULE_SCHNORR
136136
CHECK(secp256k1_schnorr_sign(data.ctx, data.sig, data.msg, data.key, NULL, NULL));
137137
data.siglen = 64;
138-
run_benchmark("schnorr_verify", benchmark_schnorr_verify, NULL, NULL, &data, 10, iters);
138+
run_benchmark("schnorr_verify", bench_schnorr_verify, NULL, NULL, &data, 10, iters);
139139
#endif
140140

141141
secp256k1_context_destroy(data.ctx);

src/secp256k1/src/modules/extrakeys/main_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ int secp256k1_keypair_create(const secp256k1_context* ctx, secp256k1_keypair *ke
180180

181181
ret = secp256k1_ec_pubkey_create_helper(&ctx->ecmult_gen_ctx, &sk, &pk, seckey32);
182182
secp256k1_keypair_save(keypair, &sk, &pk);
183-
memczero(keypair, sizeof(*keypair), !ret);
183+
secp256k1_memczero(keypair, sizeof(*keypair), !ret);
184184

185185
secp256k1_scalar_clear(&sk);
186186
return ret;

src/secp256k1/src/modules/schnorrsig/main_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64
179179
secp256k1_scalar_add(&e, &e, &k);
180180
secp256k1_scalar_get_b32(&sig64[32], &e);
181181

182-
memczero(sig64, 64, !ret);
182+
secp256k1_memczero(sig64, 64, !ret);
183183
secp256k1_scalar_clear(&k);
184184
secp256k1_scalar_clear(&sk);
185185
memset(seckey, 0, sizeof(seckey));

src/secp256k1/src/secp256k1.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p
583583

584584
ret = secp256k1_ec_pubkey_create_helper(&ctx->ecmult_gen_ctx, &seckey_scalar, &p, seckey);
585585
secp256k1_pubkey_save(pubkey, &p);
586-
memczero(pubkey, sizeof(*pubkey), !ret);
586+
secp256k1_memczero(pubkey, sizeof(*pubkey), !ret);
587587

588588
secp256k1_scalar_clear(&seckey_scalar);
589589
return ret;

src/secp256k1/src/tests.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -5453,18 +5453,18 @@ void run_ecdsa_openssl(void) {
54535453
# include "modules/schnorrsig/tests_impl.h"
54545454
#endif
54555455

5456-
void run_memczero_test(void) {
5456+
void run_secp256k1_memczero_test(void) {
54575457
unsigned char buf1[6] = {1, 2, 3, 4, 5, 6};
54585458
unsigned char buf2[sizeof(buf1)];
54595459

5460-
/* memczero(..., ..., 0) is a noop. */
5460+
/* secp256k1_memczero(..., ..., 0) is a noop. */
54615461
memcpy(buf2, buf1, sizeof(buf1));
5462-
memczero(buf1, sizeof(buf1), 0);
5462+
secp256k1_memczero(buf1, sizeof(buf1), 0);
54635463
CHECK(secp256k1_memcmp_var(buf1, buf2, sizeof(buf1)) == 0);
54645464

5465-
/* memczero(..., ..., 1) zeros the buffer. */
5465+
/* secp256k1_memczero(..., ..., 1) zeros the buffer. */
54665466
memset(buf2, 0, sizeof(buf2));
5467-
memczero(buf1, sizeof(buf1) , 1);
5467+
secp256k1_memczero(buf1, sizeof(buf1) , 1);
54685468
CHECK(secp256k1_memcmp_var(buf1, buf2, sizeof(buf1)) == 0);
54695469
}
54705470

@@ -5741,7 +5741,7 @@ int main(int argc, char **argv) {
57415741
#endif
57425742

57435743
/* util tests */
5744-
run_memczero_test();
5744+
run_secp256k1_memczero_test();
57455745

57465746
run_cmov_tests();
57475747

src/secp256k1/src/util.h

+8-2
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz
202202
#endif
203203

204204
/* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */
205-
static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) {
205+
static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) {
206206
unsigned char *p = (unsigned char *)s;
207207
/* Access flag with a volatile-qualified lvalue.
208208
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)
260260
# define SECP256K1_WIDEMUL_INT128 1
261261
#elif defined(USE_FORCE_WIDEMUL_INT64)
262262
# define SECP256K1_WIDEMUL_INT64 1
263-
#elif defined(__SIZEOF_INT128__)
263+
#elif defined(UINT128_MAX) || defined(__SIZEOF_INT128__)
264264
# define SECP256K1_WIDEMUL_INT128 1
265265
#else
266266
# define SECP256K1_WIDEMUL_INT64 1
267267
#endif
268268
#if defined(SECP256K1_WIDEMUL_INT128)
269+
# if !defined(UINT128_MAX) && defined(__SIZEOF_INT128__)
269270
SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t;
270271
SECP256K1_GNUC_EXT typedef __int128 int128_t;
272+
#define UINT128_MAX ((uint128_t)(-1))
273+
#define INT128_MAX ((int128_t)(UINT128_MAX >> 1))
274+
#define INT128_MIN (-INT128_MAX - 1)
275+
/* No (U)INT128_C macros because compilers providing __int128 do not support 128-bit literals. */
276+
# endif
271277
#endif
272278

273279
#endif /* SECP256K1_UTIL_H */

0 commit comments

Comments
 (0)