Merge bitcoin-core/secp256k1#1345: field: Static-assert that int args…

… affecting magnitude are constant

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

Pull request description:

  See bitcoin#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

src/field.h

@@ -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
@@ -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.
  *
@@ -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.
  *

src/field_10x26_impl.h

@@ -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);
@@ -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;

src/field_5x52_impl.h

@@ -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);
@@ -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;

src/field_impl.h

@@ -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);

src/tests.c

@@ -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);
@@ -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

src/util.h

@@ -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;