From: Niels Möller <nisse@lysator.liu.se>
Date: Thu, 29 Oct 2020 19:32:02 +0000 (+0100)
Subject: Simplify ecc_mod, and prepare for separate result argument.
X-Git-Tag: nettle_3.7rc1~52^2~30
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c17a6a09e82df9640f596ff40ab13e4107c8e889;p=thirdparty%2Fnettle.git

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 6626c6ea..dd534f00 100644
--- 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 fd3b315d..38a0d4f9 100644
--- 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 41933b6f..d8c0e068 100644
--- 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);
 }