Repository: bitcoin-core/secp256k1
Pull request #696 (2019-11-28 19:27:04)

Run a Travis test on s390x (big endian)

sipa: > checking whether byte ordering is bigendian... yes

\o/
elichai: It worked on core :P
But still satisfying to see.
There's nothing more satisfying than seeing that your code works on an edge case you never tested before l 
jonasnick: Cool! What's missing for un-WIP'ing? How about turning on the ECDH and recovery module?
real-or-random: > Cool! What's missing for un-WIP'ing? How about turning on the ECDH and recovery module?

@sipa Could you do this, and maybe also add a comment to the Travis file that this is a big-endian host?
sipa: Rebased, and added ECDH/recovery.

I did notice valgrind ctime test failure on this line in src/scalar_4x64_impl.h:

```c
    no |= (a->d[1] < SECP256K1_N_1);
```

so I've disabled the ctime test for now.
real-or-random: > ```c
>     no |= (a->d[1] < SECP256K1_N_1);
> ```

and two lines below too:

https://github.com/bitcoin-core/secp256k1/blob/master/src/scalar_4x64_impl.h#L65

> 
> so I've disabled the ctime test for now.

makes sense.

real-or-random: ACK c5e43bb8d6c60c5f8ee38c89ac4d0d755e6515d0 Travis works and says it's big endian

The travis failure is a randomness test again (#68), see https://travis-ci.org/github/bitcoin-core/secp256k1/jobs/713125311#L497 

If you want, you can squash this. Otherwise I'll just merge it in the next days if noone objects.

Patch:
diff --git a/.travis.yml b/.travis.yml
index a6ad6fb..491da6c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -83,6 +83,10 @@ matrix:
             - valgrind
             - libtool-bin
             - libc6-dbg:i386
+    # S390x build (big endian system)
+    - compiler: gcc
+      env: HOST=s390x-unknown-linux-gnu ECDH=yes RECOVERY=yes EXPERIMENTAL=yes CTIMETEST=
+      arch: s390x
 
 # We use this to install macOS dependencies instead of the built in `homebrew` plugin,
 # because in xcode earlier than 11 they have a bug requiring updating the system which overall takes ~8 minutes.
diff --git a/contrib/travis.sh b/contrib/travis.sh
index 2c0cbcd..3909d16 100755
--- a/contrib/travis.sh
+++ b/contrib/travis.sh
@@ -3,6 +3,10 @@
 set -e
 set -x
 
+if [ -n "$HOST" ]
+then
+    export USE_HOST="--host=$HOST"
+fi
 if [ "$HOST" = "i686-linux-gnu" ]
 then
     export CC="$CC -m32"
@@ -16,8 +20,7 @@ fi
     --enable-experimental="$EXPERIMENTAL" --enable-endomorphism="$ENDOMORPHISM" \
     --with-field="$FIELD" --with-bignum="$BIGNUM" --with-asm="$ASM" --with-scalar="$SCALAR" \
     --enable-ecmult-static-precomputation="$STATICPRECOMPUTATION" --with-ecmult-gen-precision="$ECMULTGENPRECISION" \
-    --enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" \
-    --host="$HOST" $EXTRAFLAGS
+    --enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" "$EXTRAFLAGS" "$USE_HOST"
 
 if [ -n "$BUILD" ]
 then
diff --git a/src/ecmult_const_impl.h b/src/ecmult_const_impl.h
index 55b61e4..e28cdce 100644
--- a/src/ecmult_const_impl.h
+++ b/src/ecmult_const_impl.h
@@ -208,7 +208,7 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
         int n;
         int j;
         for (j = 0; j < WINDOW_A - 1; ++j) {
-            secp256k1_gej_double(r, r);
+            secp256k1_gej_double_nonzero(r, r);
         }
 
         n = wnaf_1[i];
diff --git a/src/group.h b/src/group.h
index 6185be0..863644f 100644
--- a/src/group.h
+++ b/src/group.h
@@ -95,8 +95,8 @@ static int secp256k1_gej_is_infinity(const secp256k1_gej *a);
 /** Check whether a group element's y coordinate is a quadratic residue. */
 static int secp256k1_gej_has_quad_y_var(const secp256k1_gej *a);
 
-/** Set r equal to the double of a. Constant time. */
-static void secp256k1_gej_double(secp256k1_gej *r, const secp256k1_gej *a);
+/** Set r equal to the double of a, a cannot be infinity. Constant time. */
+static void secp256k1_gej_double_nonzero(secp256k1_gej *r, const secp256k1_gej *a);
 
 /** Set r equal to the double of a. If rzr is not-NULL this sets *rzr such that r->z == a->z * *rzr (where infinity means an implicit z = 0). */
 static void secp256k1_gej_double_var(secp256k1_gej *r, const secp256k1_gej *a, secp256k1_fe *rzr);
diff --git a/src/group_impl.h b/src/group_impl.h
index fbfd348..43b039b 100644
--- a/src/group_impl.h
+++ b/src/group_impl.h
@@ -303,7 +303,7 @@ static int secp256k1_ge_is_valid_var(const secp256k1_ge *a) {
     return secp256k1_fe_equal_var(&y2, &x3);
 }
 
-static SECP256K1_INLINE void secp256k1_gej_double(secp256k1_gej *r, const secp256k1_gej *a) {
+static SECP256K1_INLINE void secp256k1_gej_double_nonzero(secp256k1_gej *r, const secp256k1_gej *a) {
     /* Operations: 3 mul, 4 sqr, 0 normalize, 12 mul_int/add/negate.
      *
      * Note that there is an implementation described at
@@ -313,7 +313,8 @@ static SECP256K1_INLINE void secp256k1_gej_double(secp256k1_gej *r, const secp25
      */
     secp256k1_fe t1,t2,t3,t4;
 
-    r->infinity = a->infinity;
+    VERIFY_CHECK(!secp256k1_gej_is_infinity(a));
+    r->infinity = 0;
 
     secp256k1_fe_mul(&r->z, &a->z, &a->y);
     secp256k1_fe_mul_int(&r->z, 2);       /* Z' = 2*Y*Z (2) */
@@ -362,7 +363,7 @@ static void secp256k1_gej_double_var(secp256k1_gej *r, const secp256k1_gej *a, s
         secp256k1_fe_mul_int(rzr, 2);
     }
 
-    secp256k1_gej_double(r, a);
+    secp256k1_gej_double_nonzero(r, a);
 }
 
 static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_gej *b, secp256k1_fe *rzr) {


Test patch:
diff --git a/src/tests.c b/src/tests.c
index 58d2ad2..563563d 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -2218,9 +2218,6 @@ void test_ge(void) {
                 /* Normal doubling. */
                 secp256k1_gej_double_var(&resj, &gej[i2], NULL);
                 ge_equals_gej(&ref, &resj);
-                /* Constant-time doubling. */
-                secp256k1_gej_double(&resj, &gej[i2]);
-                ge_equals_gej(&ref, &resj);
             }
 
             /* Test adding opposites. */
diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c
index 8f34670..8cca1ce 100644
--- a/src/tests_exhaustive.c
+++ b/src/tests_exhaustive.c
@@ -141,8 +141,10 @@ void test_exhaustive_addition(const secp256k1_ge *group, const secp256k1_gej *gr
     /* Check doubling */
     for (i = 0; i < order; i++) {
         secp256k1_gej tmp;
-        secp256k1_gej_double(&tmp, &groupj[i]);
-        ge_equals_gej(&group[(2 * i) % order], &tmp);
+        if (i > 0) {
+            secp256k1_gej_double_nonzero(&tmp, &groupj[i]);
+            ge_equals_gej(&group[(2 * i) % order], &tmp);
+        }
         secp256k1_gej_double_var(&tmp, &groupj[i], NULL);
         ge_equals_gej(&group[(2 * i) % order], &tmp);
     }