Commit dc3faea0 authored by Damien George's avatar Damien George
Browse files

py/mpz: Fix bug with overflowing C-shift in division routine.

When DIG_SIZE=32, a uint32_t is used to store limbs, and no normalisation
is needed because the MSB is already set, then there will be left and
right shifts (in C) by 32 of a 32-bit variable, leading to undefined
behaviour.  This patch fixes this bug.
parent d59c2e5e
...@@ -491,7 +491,7 @@ STATIC void mpn_div(mpz_dig_t *num_dig, mp_uint_t *num_len, mpz_dig_t *den_dig, ...@@ -491,7 +491,7 @@ STATIC void mpn_div(mpz_dig_t *num_dig, mp_uint_t *num_len, mpz_dig_t *den_dig,
for (mpz_dig_t *den = den_dig, carry = 0; den < den_dig + den_len; ++den) { for (mpz_dig_t *den = den_dig, carry = 0; den < den_dig + den_len; ++den) {
mpz_dig_t d = *den; mpz_dig_t d = *den;
*den = ((d << norm_shift) | carry) & DIG_MASK; *den = ((d << norm_shift) | carry) & DIG_MASK;
carry = d >> (DIG_SIZE - norm_shift); carry = (mpz_dbl_dig_t)d >> (DIG_SIZE - norm_shift);
} }
// now need to shift numerator by same amount as denominator // now need to shift numerator by same amount as denominator
...@@ -501,7 +501,7 @@ STATIC void mpn_div(mpz_dig_t *num_dig, mp_uint_t *num_len, mpz_dig_t *den_dig, ...@@ -501,7 +501,7 @@ STATIC void mpn_div(mpz_dig_t *num_dig, mp_uint_t *num_len, mpz_dig_t *den_dig,
for (mpz_dig_t *num = num_dig, carry = 0; num < num_dig + *num_len; ++num) { for (mpz_dig_t *num = num_dig, carry = 0; num < num_dig + *num_len; ++num) {
mpz_dig_t n = *num; mpz_dig_t n = *num;
*num = ((n << norm_shift) | carry) & DIG_MASK; *num = ((n << norm_shift) | carry) & DIG_MASK;
carry = n >> (DIG_SIZE - norm_shift); carry = (mpz_dbl_dig_t)n >> (DIG_SIZE - norm_shift);
} }
// cache the leading digit of the denominator // cache the leading digit of the denominator
...@@ -618,14 +618,14 @@ STATIC void mpn_div(mpz_dig_t *num_dig, mp_uint_t *num_len, mpz_dig_t *den_dig, ...@@ -618,14 +618,14 @@ STATIC void mpn_div(mpz_dig_t *num_dig, mp_uint_t *num_len, mpz_dig_t *den_dig,
for (mpz_dig_t *den = den_dig + den_len - 1, carry = 0; den >= den_dig; --den) { for (mpz_dig_t *den = den_dig + den_len - 1, carry = 0; den >= den_dig; --den) {
mpz_dig_t d = *den; mpz_dig_t d = *den;
*den = ((d >> norm_shift) | carry) & DIG_MASK; *den = ((d >> norm_shift) | carry) & DIG_MASK;
carry = d << (DIG_SIZE - norm_shift); carry = (mpz_dbl_dig_t)d << (DIG_SIZE - norm_shift);
} }
// unnormalise numerator (remainder now) // unnormalise numerator (remainder now)
for (mpz_dig_t *num = orig_num_dig + *num_len - 1, carry = 0; num >= orig_num_dig; --num) { for (mpz_dig_t *num = orig_num_dig + *num_len - 1, carry = 0; num >= orig_num_dig; --num) {
mpz_dig_t n = *num; mpz_dig_t n = *num;
*num = ((n >> norm_shift) | carry) & DIG_MASK; *num = ((n >> norm_shift) | carry) & DIG_MASK;
carry = n << (DIG_SIZE - norm_shift); carry = (mpz_dbl_dig_t)n << (DIG_SIZE - norm_shift);
} }
// strip trailing zeros // strip trailing zeros
......
for lhs in (1000000000000000000000000, 10000000000100000000000000, 10012003400000000000000007, 12349083434598210349871029923874109871234789): for lhs in (1000000000000000000000000, 10000000000100000000000000, 10012003400000000000000007, 12349083434598210349871029923874109871234789):
for rhs in range(1, 555): for rhs in range(1, 555):
print(lhs // rhs) print(lhs // rhs)
# these check an edge case on 64-bit machines where two mpz limbs
# are used and the most significant one has the MSB set
x = 0x8000000000000000
print((x + 1) // x)
x = 0x86c60128feff5330
print((x + 1) // x)
...@@ -8,3 +8,10 @@ for i in range(11): ...@@ -8,3 +8,10 @@ for i in range(11):
y = delta * (j)# - 5) # TODO reinstate negative number test when % is working with sign correctly y = delta * (j)# - 5) # TODO reinstate negative number test when % is working with sign correctly
if y != 0: if y != 0:
print(x % y) print(x % y)
# these check an edge case on 64-bit machines where two mpz limbs
# are used and the most significant one has the MSB set
x = 0x8000000000000000
print((x + 1) % x)
x = 0x86c60128feff5330
print((x + 1) % x)
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment