From fe8a8cd100551264d03dbc71e52bafd19832b3cc Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 19 Apr 2023 17:40:28 +0100 Subject: [PATCH 1/8] Size/perf optimisation for mbedtls_mpi_core_clz Signed-off-by: Dave Rodgman --- library/bignum_core.c | 30 +++++++++++++++ tests/suites/test_suite_bignum_core.function | 37 +++++++++++++++++++ tests/suites/test_suite_bignum_core.misc.data | 35 ++++++++++++++++++ 3 files changed, 102 insertions(+) diff --git a/library/bignum_core.c b/library/bignum_core.c index c6d92fb061..9b6c414eb4 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -35,6 +35,36 @@ size_t mbedtls_mpi_core_clz(mbedtls_mpi_uint a) { +#if defined(__has_builtin) +#if __has_builtin(__builtin_clz) + if (sizeof(mbedtls_mpi_uint) == sizeof(unsigned int)) { + // __builtin_clz is undefined if a == 0 + if (a == 0) { + return sizeof(mbedtls_mpi_uint) * 8; + } else { + return (size_t) __builtin_clz(a); + } + } +#endif +#if __has_builtin(__builtin_clzl) + if (sizeof(mbedtls_mpi_uint) == sizeof(unsigned long)) { + if (a == 0) { + return sizeof(mbedtls_mpi_uint) * 8; + } else { + return (size_t) __builtin_clzl(a); + } + } +#endif +#if __has_builtin(__builtin_clzll) + if (sizeof(mbedtls_mpi_uint) == sizeof(unsigned long long)) { + if (a == 0) { + return sizeof(mbedtls_mpi_uint) * 8; + } else { + return (size_t) __builtin_clzll(a); + } + } +#endif +#endif size_t j; mbedtls_mpi_uint mask = (mbedtls_mpi_uint) 1 << (biL - 1); diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index e084b83252..d80054c236 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -309,6 +309,43 @@ exit: } /* END_CASE */ + +/* BEGIN_CASE */ +void mpi_core_clz(int lz, int tz) +{ + if ((size_t) (lz + tz) >= (sizeof(mbedtls_mpi_uint) * 8)) { + // can't fit required number of leading and trailing zeros - skip test + goto exit; + } + + mbedtls_mpi_uint x; + size_t expected; + + // generate x with lz leading zeros and tz trailing zeros, all other bits set. + if (lz == -1) { + // special case: all zero + x = 0; + expected = sizeof(mbedtls_mpi_uint) * 8; + } else { + expected = lz; + if ((lz + tz) > 0) { + // some zero bits + uint32_t s = (sizeof(mbedtls_mpi_uint) * 8 - lz - tz); + x = ((1ULL << s) - 1) << tz; + } else { + // all bits set + x = ~0ULL; + } + } + + size_t n = mbedtls_mpi_core_clz(x); + TEST_EQUAL(n, expected); +exit: + ; +} +/* END_CASE */ + + /* BEGIN_CASE */ void mpi_core_lt_ct(char *input_X, char *input_Y, int exp_ret) { diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index b61d708d6e..c245b02d37 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -491,3 +491,38 @@ mpi_core_fill_random:42:0:-1:0:MBEDTLS_ERR_MPI_BAD_INPUT_DATA Fill random core: 42 bytes, 5 missing limbs mpi_core_fill_random:42:0:-5:0:MBEDTLS_ERR_MPI_BAD_INPUT_DATA +CLZ: 0 0: all ones +mpi_core_clz:0:0 + +CLZ: -1 -1: all zeros +mpi_core_clz:-1:-1 + +CLZ: 1 0 +mpi_core_clz:1:0 + +CLZ: 1 1 +mpi_core_clz:1:1 + +CLZ: 4 5 +mpi_core_clz:4:5 + +CLZ: 8 16 +mpi_core_clz:8:16 + +CLZ: 31 0 +mpi_core_clz:31:0 + +CLZ: 32 0 +mpi_core_clz:32:0 + +CLZ: 33 0 +mpi_core_clz:33:0 + +CLZ: 63 0 +mpi_core_clz:63:0 + +CLZ: 64 0 +mpi_core_clz:64:0 + +CLZ: 100000 0: skip overly long input +mpi_core_clz:100000:0 From d54cb83584c1702de390f62e5eabbd435e6e56c7 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 19 Apr 2023 18:38:54 +0100 Subject: [PATCH 2/8] Fix tests Signed-off-by: Dave Rodgman --- tests/suites/test_suite_bignum_core.function | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index d80054c236..a832a51232 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -331,10 +331,10 @@ void mpi_core_clz(int lz, int tz) if ((lz + tz) > 0) { // some zero bits uint32_t s = (sizeof(mbedtls_mpi_uint) * 8 - lz - tz); - x = ((1ULL << s) - 1) << tz; + x = ((((mbedtls_mpi_uint) 1) << s) - 1) << tz; } else { // all bits set - x = ~0ULL; + x = ~((mbedtls_mpi_uint) 0); } } From 880a6b34c29860d480c5c40425635147220bc10b Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 20 Apr 2023 09:50:31 +0100 Subject: [PATCH 3/8] Further size optimisation Signed-off-by: Dave Rodgman --- library/bignum_core.c | 44 ++++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 9b6c414eb4..1f3a57c054 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -33,35 +33,25 @@ #include "bn_mul.h" #include "constant_time_internal.h" -size_t mbedtls_mpi_core_clz(mbedtls_mpi_uint a) +inline size_t mbedtls_mpi_core_clz(mbedtls_mpi_uint a) { + /* Note: the result is undefined for a == 0 + * (because this is the behaviour of __builtin_clz). + */ #if defined(__has_builtin) #if __has_builtin(__builtin_clz) if (sizeof(mbedtls_mpi_uint) == sizeof(unsigned int)) { - // __builtin_clz is undefined if a == 0 - if (a == 0) { - return sizeof(mbedtls_mpi_uint) * 8; - } else { - return (size_t) __builtin_clz(a); - } + return (size_t) __builtin_clz(a); } #endif #if __has_builtin(__builtin_clzl) if (sizeof(mbedtls_mpi_uint) == sizeof(unsigned long)) { - if (a == 0) { - return sizeof(mbedtls_mpi_uint) * 8; - } else { - return (size_t) __builtin_clzl(a); - } + return (size_t) __builtin_clzl(a); } #endif #if __has_builtin(__builtin_clzll) if (sizeof(mbedtls_mpi_uint) == sizeof(unsigned long long)) { - if (a == 0) { - return sizeof(mbedtls_mpi_uint) * 8; - } else { - return (size_t) __builtin_clzll(a); - } + return (size_t) __builtin_clzll(a); } #endif #endif @@ -81,21 +71,19 @@ size_t mbedtls_mpi_core_clz(mbedtls_mpi_uint a) size_t mbedtls_mpi_core_bitlen(const mbedtls_mpi_uint *A, size_t A_limbs) { - size_t i, j; + int i; + size_t j; - if (A_limbs == 0) { - return 0; - } - - for (i = A_limbs - 1; i > 0; i--) { - if (A[i] != 0) { - break; + if (A_limbs != 0) { + for (i = (int) A_limbs - 1; i >= 0; i--) { + if (A[i] != 0) { + j = biL - mbedtls_mpi_core_clz(A[i]); + return (i * biL) + j; + } } } - j = biL - mbedtls_mpi_core_clz(A[i]); - - return (i * biL) + j; + return 0; } /* Convert a big-endian byte array aligned to the size of mbedtls_mpi_uint From 678e63007c35d9c6eb0dd4d5c81556d78b327a24 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 20 Apr 2023 12:28:59 +0100 Subject: [PATCH 4/8] Remove test-case for all-zero Signed-off-by: Dave Rodgman --- tests/suites/test_suite_bignum_core.function | 24 ++++++------------- tests/suites/test_suite_bignum_core.misc.data | 3 --- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index a832a51232..6f810ff722 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -319,27 +319,17 @@ void mpi_core_clz(int lz, int tz) } mbedtls_mpi_uint x; - size_t expected; - - // generate x with lz leading zeros and tz trailing zeros, all other bits set. - if (lz == -1) { - // special case: all zero - x = 0; - expected = sizeof(mbedtls_mpi_uint) * 8; + if ((lz + tz) > 0) { + // some zero bits + uint32_t s = (sizeof(mbedtls_mpi_uint) * 8 - lz - tz); + x = ((((mbedtls_mpi_uint) 1) << s) - 1) << tz; } else { - expected = lz; - if ((lz + tz) > 0) { - // some zero bits - uint32_t s = (sizeof(mbedtls_mpi_uint) * 8 - lz - tz); - x = ((((mbedtls_mpi_uint) 1) << s) - 1) << tz; - } else { - // all bits set - x = ~((mbedtls_mpi_uint) 0); - } + // all bits set + x = ~((mbedtls_mpi_uint) 0); } size_t n = mbedtls_mpi_core_clz(x); - TEST_EQUAL(n, expected); + TEST_EQUAL(n, lz); exit: ; } diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index c245b02d37..ba86029977 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -494,9 +494,6 @@ mpi_core_fill_random:42:0:-5:0:MBEDTLS_ERR_MPI_BAD_INPUT_DATA CLZ: 0 0: all ones mpi_core_clz:0:0 -CLZ: -1 -1: all zeros -mpi_core_clz:-1:-1 - CLZ: 1 0 mpi_core_clz:1:0 From bbf881053d1801a7cbf22edfc534e7291a3fa300 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 21 Apr 2023 12:54:40 +0100 Subject: [PATCH 5/8] Document undefined case. Clarify test code. Signed-off-by: Dave Rodgman --- library/bignum_core.c | 13 ++++++++++--- tests/suites/test_suite_bignum_core.function | 15 +++++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 1f3a57c054..998c06c8fd 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -33,11 +33,18 @@ #include "bn_mul.h" #include "constant_time_internal.h" +/** + * \brief Count leading zeros + * + * \warning The result is undefined if \p a == 0 + * + * \param a The value to operate on + * + * \return The number of leading zeros, if \p a != 0. If \p a == 0, the result + * is undefined. + */ inline size_t mbedtls_mpi_core_clz(mbedtls_mpi_uint a) { - /* Note: the result is undefined for a == 0 - * (because this is the behaviour of __builtin_clz). - */ #if defined(__has_builtin) #if __has_builtin(__builtin_clz) if (sizeof(mbedtls_mpi_uint) == sizeof(unsigned int)) { diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 6f810ff722..53aa002ce6 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -311,25 +311,28 @@ exit: /* BEGIN_CASE */ -void mpi_core_clz(int lz, int tz) +void mpi_core_clz(int leading_zeros, int trailing_zeros) { - if ((size_t) (lz + tz) >= (sizeof(mbedtls_mpi_uint) * 8)) { + if ((size_t) (leading_zeros + trailing_zeros) >= (sizeof(mbedtls_mpi_uint) * 8)) { // can't fit required number of leading and trailing zeros - skip test goto exit; } + // Construct a test input value where the count of leading zeros and + // trailing zeros is given in the test case, and we add ones to fill + // the gap. mbedtls_mpi_uint x; - if ((lz + tz) > 0) { + if ((leading_zeros + trailing_zeros) > 0) { // some zero bits - uint32_t s = (sizeof(mbedtls_mpi_uint) * 8 - lz - tz); - x = ((((mbedtls_mpi_uint) 1) << s) - 1) << tz; + uint32_t s = (sizeof(mbedtls_mpi_uint) * 8 - leading_zeros - trailing_zeros); + x = ((((mbedtls_mpi_uint) 1) << s) - 1) << trailing_zeros; } else { // all bits set x = ~((mbedtls_mpi_uint) 0); } size_t n = mbedtls_mpi_core_clz(x); - TEST_EQUAL(n, lz); + TEST_EQUAL(n, leading_zeros); exit: ; } From 0f16d560aaaad0360ed9fd545432a53550bb64bb Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 24 Apr 2023 12:53:29 +0100 Subject: [PATCH 6/8] Fix documentation Signed-off-by: Dave Rodgman --- library/bignum_core.c | 10 ---------- library/bignum_core.h | 5 ++++- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 998c06c8fd..ba6c0c13ae 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -33,16 +33,6 @@ #include "bn_mul.h" #include "constant_time_internal.h" -/** - * \brief Count leading zeros - * - * \warning The result is undefined if \p a == 0 - * - * \param a The value to operate on - * - * \return The number of leading zeros, if \p a != 0. If \p a == 0, the result - * is undefined. - */ inline size_t mbedtls_mpi_core_clz(mbedtls_mpi_uint a) { #if defined(__has_builtin) diff --git a/library/bignum_core.h b/library/bignum_core.h index b3d05a34ef..158d2b3222 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -101,10 +101,13 @@ (((X)[(i) / ciL] >> (((i) % ciL) * 8)) & 0xff) /** Count leading zero bits in a given integer. + * + * \warning The result is undefined if \p a == 0 * * \param a Integer to count leading zero bits. * - * \return The number of leading zero bits in \p a. + * \return The number of leading zero bits in \p a, if \p a != 0. + * If \p a == 0, the result is undefined. */ size_t mbedtls_mpi_core_clz(mbedtls_mpi_uint a); From 2e863ecde9017d4a4f9fd407354e30a2a05e2d32 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 25 Apr 2023 17:34:59 +0100 Subject: [PATCH 7/8] Remove unnecessary if to save 16 bytes Signed-off-by: Dave Rodgman --- library/bignum_core.c | 10 ++++------ tests/suites/test_suite_bignum_core.function | 6 +++++- tests/suites/test_suite_bignum_core.misc.data | 3 +++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index ba6c0c13ae..a99b3be2bb 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -71,12 +71,10 @@ size_t mbedtls_mpi_core_bitlen(const mbedtls_mpi_uint *A, size_t A_limbs) int i; size_t j; - if (A_limbs != 0) { - for (i = (int) A_limbs - 1; i >= 0; i--) { - if (A[i] != 0) { - j = biL - mbedtls_mpi_core_clz(A[i]); - return (i * biL) + j; - } + for (i = ((int) A_limbs) - 1; i >= 0; i--) { + if (A[i] != 0) { + j = biL - mbedtls_mpi_core_clz(A[i]); + return (i * biL) + j; } } diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 53aa002ce6..0cd1507b37 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -301,7 +301,11 @@ void mpi_core_bitlen(char *input_X, int nr_bits) mbedtls_mpi_uint *X = NULL; size_t limbs; - TEST_EQUAL(mbedtls_test_read_mpi_core(&X, &limbs, input_X), 0); + if (strlen(input_X) == 0) { + limbs = 0; + } else { + TEST_EQUAL(mbedtls_test_read_mpi_core(&X, &limbs, input_X), 0); + } TEST_EQUAL(mbedtls_mpi_core_bitlen(X, limbs), nr_bits); exit: diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index ba86029977..9e1b892fd7 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -155,6 +155,9 @@ mpi_core_bitlen:"10":5 Test mbedtls_mpi_core_bitlen 0x0a mpi_core_bitlen:"a":4 +Test mbedtls_mpi_core_bitlen: 0 limbs +mpi_core_bitlen:"":0 + Test mbedtls_mpi_core_bitlen: 0 (1 limb) mpi_core_bitlen:"0":0 From 4f30a6aa59d1fcc35f5d04ded96efae2d6e3132b Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 25 Apr 2023 18:07:29 +0100 Subject: [PATCH 8/8] Remove undesirable test Signed-off-by: Dave Rodgman --- tests/suites/test_suite_bignum_core.function | 6 +----- tests/suites/test_suite_bignum_core.misc.data | 3 --- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 0cd1507b37..53aa002ce6 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -301,11 +301,7 @@ void mpi_core_bitlen(char *input_X, int nr_bits) mbedtls_mpi_uint *X = NULL; size_t limbs; - if (strlen(input_X) == 0) { - limbs = 0; - } else { - TEST_EQUAL(mbedtls_test_read_mpi_core(&X, &limbs, input_X), 0); - } + TEST_EQUAL(mbedtls_test_read_mpi_core(&X, &limbs, input_X), 0); TEST_EQUAL(mbedtls_mpi_core_bitlen(X, limbs), nr_bits); exit: diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index 9e1b892fd7..ba86029977 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -155,9 +155,6 @@ mpi_core_bitlen:"10":5 Test mbedtls_mpi_core_bitlen 0x0a mpi_core_bitlen:"a":4 -Test mbedtls_mpi_core_bitlen: 0 limbs -mpi_core_bitlen:"":0 - Test mbedtls_mpi_core_bitlen: 0 (1 limb) mpi_core_bitlen:"0":0