Skip to content

Commit

Permalink
Merge #1345: field: Static-assert that int args affecting magnitude a…
Browse files Browse the repository at this point in the history
…re constant

be8ff3a field: Static-assert that int args affecting magnitude are constant (Tim Ruffing)

Pull request description:

  See #1001.

  Try to revert the lines in `tests.c` to see the error message in action.

ACKs for top commit:
  sipa:
    ACK be8ff3a. Verified by introducing some non-constant expressions and seeing compilation fail.
  theStack:
    ACK be8ff3a

Tree-SHA512: 8befec6ee64959cdc7f3e29b4b622410794cfaf69e9df8df17600390a93bc787dba5cf86239de6eb2e99c038b9aca5461e4b3c82f0e0c4cf066ad7c689941b19
  • Loading branch information
real-or-random committed Jun 27, 2023
2 parents 4494a36 + be8ff3a commit 3aef6ab
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 18 deletions.
24 changes: 18 additions & 6 deletions src/field.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ static const secp256k1_fe secp256k1_const_beta = SECP256K1_FE_CONST(
# define secp256k1_fe_set_b32_mod secp256k1_fe_impl_set_b32_mod
# define secp256k1_fe_set_b32_limit secp256k1_fe_impl_set_b32_limit
# define secp256k1_fe_get_b32 secp256k1_fe_impl_get_b32
# define secp256k1_fe_negate secp256k1_fe_impl_negate
# define secp256k1_fe_mul_int secp256k1_fe_impl_mul_int
# define secp256k1_fe_negate_unchecked secp256k1_fe_impl_negate_unchecked
# define secp256k1_fe_mul_int_unchecked secp256k1_fe_impl_mul_int_unchecked
# define secp256k1_fe_add secp256k1_fe_impl_add
# define secp256k1_fe_mul secp256k1_fe_impl_mul
# define secp256k1_fe_sqr secp256k1_fe_impl_sqr
Expand Down Expand Up @@ -214,11 +214,17 @@ static void secp256k1_fe_get_b32(unsigned char *r, const secp256k1_fe *a);
/** Negate a field element.
*
* On input, r does not need to be initialized. a must be a valid field element with
* magnitude not exceeding m. m must be an integer in [0,31].
* magnitude not exceeding m. m must be an integer constant expression in [0,31].
* Performs {r = -a}.
* On output, r will not be normalized, and will have magnitude m+1.
*/
static void secp256k1_fe_negate(secp256k1_fe *r, const secp256k1_fe *a, int m);
#define secp256k1_fe_negate(r, a, m) ASSERT_INT_CONST_AND_DO(m, secp256k1_fe_negate_unchecked(r, a, m))

/** Like secp256k1_fe_negate_unchecked but m is not checked to be an integer constant expression.
*
* Should not be called directly outside of tests.
*/
static void secp256k1_fe_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m);

/** Add a small integer to a field element.
*
Expand All @@ -229,12 +235,18 @@ static void secp256k1_fe_add_int(secp256k1_fe *r, int a);

/** Multiply a field element with a small integer.
*
* On input, r must be a valid field element. a must be an integer in [0,32].
* On input, r must be a valid field element. a must be an integer constant expression in [0,32].
* The magnitude of r times a must not exceed 32.
* Performs {r *= a}.
* On output, r's magnitude is multiplied by a, and r will not be normalized.
*/
static void secp256k1_fe_mul_int(secp256k1_fe *r, int a);
#define secp256k1_fe_mul_int(r, a) ASSERT_INT_CONST_AND_DO(a, secp256k1_fe_mul_int_unchecked(r, a))

/** Like secp256k1_fe_mul_int but a is not checked to be an integer constant expression.
*
* Should not be called directly outside of tests.
*/
static void secp256k1_fe_mul_int_unchecked(secp256k1_fe *r, int a);

/** Increment a field element by another.
*
Expand Down
4 changes: 2 additions & 2 deletions src/field_10x26_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ static void secp256k1_fe_impl_get_b32(unsigned char *r, const secp256k1_fe *a) {
r[31] = a->n[0] & 0xff;
}

SECP256K1_INLINE static void secp256k1_fe_impl_negate(secp256k1_fe *r, const secp256k1_fe *a, int m) {
SECP256K1_INLINE static void secp256k1_fe_impl_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m) {
/* For all legal values of m (0..31), the following properties hold: */
VERIFY_CHECK(0x3FFFC2FUL * 2 * (m + 1) >= 0x3FFFFFFUL * 2 * m);
VERIFY_CHECK(0x3FFFFBFUL * 2 * (m + 1) >= 0x3FFFFFFUL * 2 * m);
Expand All @@ -365,7 +365,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_negate(secp256k1_fe *r, const sec
r->n[9] = 0x03FFFFFUL * 2 * (m + 1) - a->n[9];
}

SECP256K1_INLINE static void secp256k1_fe_impl_mul_int(secp256k1_fe *r, int a) {
SECP256K1_INLINE static void secp256k1_fe_impl_mul_int_unchecked(secp256k1_fe *r, int a) {
r->n[0] *= a;
r->n[1] *= a;
r->n[2] *= a;
Expand Down
4 changes: 2 additions & 2 deletions src/field_5x52_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ static void secp256k1_fe_impl_get_b32(unsigned char *r, const secp256k1_fe *a) {
r[31] = a->n[0] & 0xFF;
}

SECP256K1_INLINE static void secp256k1_fe_impl_negate(secp256k1_fe *r, const secp256k1_fe *a, int m) {
SECP256K1_INLINE static void secp256k1_fe_impl_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m) {
/* For all legal values of m (0..31), the following properties hold: */
VERIFY_CHECK(0xFFFFEFFFFFC2FULL * 2 * (m + 1) >= 0xFFFFFFFFFFFFFULL * 2 * m);
VERIFY_CHECK(0xFFFFFFFFFFFFFULL * 2 * (m + 1) >= 0xFFFFFFFFFFFFFULL * 2 * m);
Expand All @@ -329,7 +329,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_negate(secp256k1_fe *r, const sec
r->n[4] = 0x0FFFFFFFFFFFFULL * 2 * (m + 1) - a->n[4];
}

SECP256K1_INLINE static void secp256k1_fe_impl_mul_int(secp256k1_fe *r, int a) {
SECP256K1_INLINE static void secp256k1_fe_impl_mul_int_unchecked(secp256k1_fe *r, int a) {
r->n[0] *= a;
r->n[1] *= a;
r->n[2] *= a;
Expand Down
12 changes: 6 additions & 6 deletions src/field_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -289,23 +289,23 @@ SECP256K1_INLINE static void secp256k1_fe_get_b32(unsigned char *r, const secp25
secp256k1_fe_impl_get_b32(r, a);
}

static void secp256k1_fe_impl_negate(secp256k1_fe *r, const secp256k1_fe *a, int m);
SECP256K1_INLINE static void secp256k1_fe_negate(secp256k1_fe *r, const secp256k1_fe *a, int m) {
static void secp256k1_fe_impl_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m);
SECP256K1_INLINE static void secp256k1_fe_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m) {
secp256k1_fe_verify(a);
VERIFY_CHECK(m >= 0 && m <= 31);
VERIFY_CHECK(a->magnitude <= m);
secp256k1_fe_impl_negate(r, a, m);
secp256k1_fe_impl_negate_unchecked(r, a, m);
r->magnitude = m + 1;
r->normalized = 0;
secp256k1_fe_verify(r);
}

static void secp256k1_fe_impl_mul_int(secp256k1_fe *r, int a);
SECP256K1_INLINE static void secp256k1_fe_mul_int(secp256k1_fe *r, int a) {
static void secp256k1_fe_impl_mul_int_unchecked(secp256k1_fe *r, int a);
SECP256K1_INLINE static void secp256k1_fe_mul_int_unchecked(secp256k1_fe *r, int a) {
secp256k1_fe_verify(r);
VERIFY_CHECK(a >= 0 && a <= 32);
VERIFY_CHECK(a*r->magnitude <= 32);
secp256k1_fe_impl_mul_int(r, a);
secp256k1_fe_impl_mul_int_unchecked(r, a);
r->magnitude *= a;
r->normalized = 0;
secp256k1_fe_verify(r);
Expand Down
4 changes: 2 additions & 2 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ static void random_field_element_magnitude(secp256k1_fe *fe) {
}
secp256k1_fe_clear(&zero);
secp256k1_fe_negate(&zero, &zero, 0);
secp256k1_fe_mul_int(&zero, n - 1);
secp256k1_fe_mul_int_unchecked(&zero, n - 1);
secp256k1_fe_add(fe, &zero);
#ifdef VERIFY
CHECK(fe->magnitude == n);
Expand Down Expand Up @@ -3234,7 +3234,7 @@ static void run_field_misc(void) {
CHECK(q.normalized && q.magnitude == 1);
#endif
for (j = 0; j < 6; j++) {
secp256k1_fe_negate(&z, &z, j+1);
secp256k1_fe_negate_unchecked(&z, &z, j+1);
secp256k1_fe_normalize_var(&q);
secp256k1_fe_cmov(&q, &z, (j&1));
#ifdef VERIFY
Expand Down
13 changes: 13 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@ static void print_buf_plain(const unsigned char *buf, size_t len) {
# define SECP256K1_INLINE inline
# endif

/** Assert statically that expr is an integer constant expression, and run stmt.
*
* Useful for example to enforce that magnitude arguments are constant.
*/
#define ASSERT_INT_CONST_AND_DO(expr, stmt) do { \
switch(42) { \
case /* ERROR: integer argument is not constant */ expr: \
break; \
default: ; \
} \
stmt; \
} while(0)

typedef struct {
void (*fn)(const char *text, void* data);
const void* data;
Expand Down

0 comments on commit 3aef6ab

Please sign in to comment.