Simplify ecc_mod, and prepare for separate result argument.

* ecc-mod.c (ecc_mod): More unified handling of final carry
folding. Also eliminates a goto statement.
* testsuite/ecc-mod-test.c (test_fixed): Add another test case

diff --git a/ChangeLog b/ChangeLog
index 6626c6eac2c2efc63b74a7a34fe8ff196f4d7d5c..dd534f00d0a3c52d17a3d35272680be74049cec9 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2020-10-29  Niels Möller  <nisse@lysator.liu.se>
 
+       * ecc-mod.c (ecc_mod): More unified handling of final carry
+       folding. Also eliminates a goto statement.
+       * testsuite/ecc-mod-test.c (test_fixed): Add another test case
+
        * blowfish.c (blowfish_set_key): Add casts to uint32_t. Avoids
        undefined behavior, since shifting an 8-bit value left by 24 bits
        overflows the range of signed int. Reported by Guido Vranken.

diff --git a/ecc-mod.c b/ecc-mod.c
index fd3b315d9217706bb33d532f557dd729b7014e70..38a0d4f9e6082333f2f8d7109291eb9cbbdfedab 100644 (file)
--- a/ecc-mod.c
+++ b/ecc-mod.c
@@ -68,17 +68,10 @@ ecc_mod (const struct ecc_modulo *m, mp_limb_t *rp)
          rp[rn-1] = rp[rn+sn-1]
            + mpn_add_n (rp + rn - sn - 1, rp + rn - sn - 1, rp + rn - 1, sn);
        }
-      goto final_limbs;
     }
   else
     {
-      /* The loop below always runs at least once. But the analyzer
-        doesn't realize that, and complains about hi being used later
-        on without a well defined value. */
-#ifdef __clang_analyzer__
-      hi = 0;
-#endif
-      while (rn >= 2 * mn - bn)
+      while (rn > 2 * mn - bn)
        {
          rn -= sn;
 
@@ -91,17 +84,16 @@ ecc_mod (const struct ecc_modulo *m, mp_limb_t *rp)
        }
     }
 
-  if (rn > mn)
-    {
-    final_limbs:
-      sn = rn - mn;
-      
-      for (i = 0; i < sn; i++)
-       rp[mn+i] = mpn_addmul_1 (rp + i, m->B, bn, rp[mn+i]);
-
-      hi = mpn_add_n (rp + bn, rp + bn, rp + mn, sn);
-      hi = sec_add_1 (rp + bn + sn, rp + bn + sn, mn - bn - sn, hi);
-    }
+  assert (rn > mn);
+  rn -= mn;
+  assert (rn <= sn);
+
+  for (i = 0; i < rn; i++)
+    rp[mn+i] = mpn_addmul_1 (rp + i, m->B, bn, rp[mn+i]);
+
+  hi = mpn_add_n (rp + bn, rp + bn, rp + mn, rn);
+  if (rn < sn)
+    hi = sec_add_1 (rp + bn + rn, rp + bn + rn, sn - rn, hi);
 
   shift = m->size * GMP_NUMB_BITS - m->bit_size;
   if (shift > 0)
@@ -113,7 +105,7 @@ ecc_mod (const struct ecc_modulo *m, mp_limb_t *rp)
     }
   else
     {
-      hi = mpn_cnd_add_n (hi, rp, rp, m->B_shifted, mn);
+      hi = mpn_cnd_add_n (hi, rp, rp, m->B, mn);
       assert (hi == 0);
     }
 }

diff --git a/testsuite/ecc-mod-test.c b/testsuite/ecc-mod-test.c
index 41933b6f0ee225b34c7a1e41ac682f1e8cf11d51..d8c0e068caff708d6cf7d142441df04aa1447f1a 100644 (file)
--- a/testsuite/ecc-mod-test.c
+++ b/testsuite/ecc-mod-test.c
@@ -123,6 +123,10 @@ test_fixed (void)
   test_one ("p", &_nettle_secp_384r1.p, r);
   test_one ("q", &_nettle_secp_384r1.q, r);
 
+  /* Triggered a carry bug in development version. */
+  mpz_set_str (r, "fffffffffffffffffffffffe00000fffffffffffffffffffffffffffe00000000000000000000000000000000000fffffffc000000000000000007ffffffffff", 16);
+  test_one ("p", &_nettle_secp_224r1.p, r);
+
   mpz_clear (r);
 }