From 84a730fd41fba82ded7518089780e815db83c3ad Mon Sep 17 00:00:00 2001 From: Daniel Green Date: Sun, 19 Mar 2017 15:02:46 -0400 Subject: [PATCH 1/3] Correctly detect+handle overflow in mp_get_int64 Handles the fact that for 64-bit 2's complement numbers the maximum positive value is 2**63 - 1, but the maximum negative value is 2**63. Uses the mp_get_long_long now available with the newer version of libtommath. Also, instead of counting the bits every time, only do it during the too-large-value error path. --- src/6model/reprs/P6bigint.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/6model/reprs/P6bigint.c b/src/6model/reprs/P6bigint.c index d5e006ff4b..b0341f7418 100644 --- a/src/6model/reprs/P6bigint.c +++ b/src/6model/reprs/P6bigint.c @@ -4,30 +4,27 @@ #define MIN(x,y) ((x)<(y)?(x):(y)) #endif -/* A forced 64-bit version of mp_get_long, since on some platforms long is - * not all that long. */ -static MVMuint64 mp_get_int64(MVMThreadContext *tc, mp_int * a) { - int i, bits; +/* Handles both int and uint cases. */ +static MVMuint64 mp_get_int64(MVMThreadContext *tc, mp_int * a, int sign) { MVMuint64 res; + MVMuint64 signed_max = 9223372036854775807ULL; if (a->used == 0) { - return 0; + return 0; } - bits = mp_count_bits(a); - if (bits > 64) { - MVM_exception_throw_adhoc(tc, "Cannot unbox %d bit wide bigint into native integer", bits); + /* For 64-bit 2's complement numbers the positive max is 2**63-1 + * but the negative max is 2**63. */ + if (sign && MP_NEG == SIGN(a)) { + ++signed_max; } - /* get number of digits of the lsb we have to read */ - i = MIN(a->used,(int)((sizeof(MVMuint64)*CHAR_BIT+DIGIT_BIT-1)/DIGIT_BIT))-1; - - /* get most significant digit of result */ - res = DIGIT(a,i); - - while (--i >= 0) { - res = (res << DIGIT_BIT) | DIGIT(a,i); + res = mp_get_long_long(a); + if (res == 0 || (sign && res > signed_max)) { + /* The mp_int was either so big it overflowed a MVMuint64 or was bigger than a signed result could be. */ + MVM_exception_throw_adhoc(tc, "Cannot unbox %d bit wide bigint into native integer", mp_count_bits(a)); } + return res; } @@ -92,10 +89,10 @@ static MVMint64 get_int(MVMThreadContext *tc, MVMSTable *st, MVMObject *root, vo if (MVM_BIGINT_IS_BIG(body)) { mp_int *i = body->u.bigint; if (MP_NEG == SIGN(i)) { - return -mp_get_int64(tc, i); + return -mp_get_int64(tc, i, 1); } else { - return mp_get_int64(tc, i); + return mp_get_int64(tc, i, 1); } } else { @@ -123,7 +120,7 @@ static MVMuint64 get_uint(MVMThreadContext *tc, MVMSTable *st, MVMObject *root, if (MP_NEG == SIGN(i)) MVM_exception_throw_adhoc(tc, "Cannot unbox negative bigint into native unsigned integer"); else - return mp_get_int64(tc, i); + return mp_get_int64(tc, i, 0); } else { return body->u.smallint.value; From 65f35b235633ede375a628e61df0ac2ff22e2996 Mon Sep 17 00:00:00 2001 From: Daniel Green Date: Tue, 21 Mar 2017 20:33:34 -0400 Subject: [PATCH 2/3] Rename `sign` to `is_signed` --- src/6model/reprs/P6bigint.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/6model/reprs/P6bigint.c b/src/6model/reprs/P6bigint.c index b0341f7418..aca5ca8e6d 100644 --- a/src/6model/reprs/P6bigint.c +++ b/src/6model/reprs/P6bigint.c @@ -5,7 +5,7 @@ #endif /* Handles both int and uint cases. */ -static MVMuint64 mp_get_int64(MVMThreadContext *tc, mp_int * a, int sign) { +static MVMuint64 mp_get_int64(MVMThreadContext *tc, mp_int * a, int is_signed) { MVMuint64 res; MVMuint64 signed_max = 9223372036854775807ULL; @@ -15,12 +15,12 @@ static MVMuint64 mp_get_int64(MVMThreadContext *tc, mp_int * a, int sign) { /* For 64-bit 2's complement numbers the positive max is 2**63-1 * but the negative max is 2**63. */ - if (sign && MP_NEG == SIGN(a)) { + if (is_signed && MP_NEG == SIGN(a)) { ++signed_max; } res = mp_get_long_long(a); - if (res == 0 || (sign && res > signed_max)) { + if (res == 0 || (is_signed && res > signed_max)) { /* The mp_int was either so big it overflowed a MVMuint64 or was bigger than a signed result could be. */ MVM_exception_throw_adhoc(tc, "Cannot unbox %d bit wide bigint into native integer", mp_count_bits(a)); } From c5eb7d5bb4def159f22803a64c6c135d66d8393d Mon Sep 17 00:00:00 2001 From: Daniel Green Date: Fri, 24 Mar 2017 18:52:15 -0400 Subject: [PATCH 3/3] Split into mp_get_int64 and mp_get_uint64 Also, handle negatives in mp_get_int64. --- src/6model/reprs/P6bigint.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/6model/reprs/P6bigint.c b/src/6model/reprs/P6bigint.c index aca5ca8e6d..5629e5defe 100644 --- a/src/6model/reprs/P6bigint.c +++ b/src/6model/reprs/P6bigint.c @@ -4,8 +4,8 @@ #define MIN(x,y) ((x)<(y)?(x):(y)) #endif -/* Handles both int and uint cases. */ -static MVMuint64 mp_get_int64(MVMThreadContext *tc, mp_int * a, int is_signed) { +/* Get a native int64 from an mp_int. */ +static MVMint64 mp_get_int64(MVMThreadContext *tc, mp_int * a) { MVMuint64 res; MVMuint64 signed_max = 9223372036854775807ULL; @@ -15,13 +15,30 @@ static MVMuint64 mp_get_int64(MVMThreadContext *tc, mp_int * a, int is_signed) { /* For 64-bit 2's complement numbers the positive max is 2**63-1 * but the negative max is 2**63. */ - if (is_signed && MP_NEG == SIGN(a)) { + if (MP_NEG == SIGN(a)) { ++signed_max; } res = mp_get_long_long(a); - if (res == 0 || (is_signed && res > signed_max)) { - /* The mp_int was either so big it overflowed a MVMuint64 or was bigger than a signed result could be. */ + if (res == 0 || res > signed_max) { + /* The mp_int was bigger than a signed result could be. */ + MVM_exception_throw_adhoc(tc, "Cannot unbox %d bit wide bigint into native integer", mp_count_bits(a)); + } + + return MP_NEG == SIGN(a) ? -res : res; +} + +/* Get a native uint64 from an mp_int. */ +static MVMuint64 mp_get_uint64(MVMThreadContext *tc, mp_int * a) { + MVMuint64 res; + + if (a->used == 0) { + return 0; + } + + res = mp_get_long_long(a); + if (res == 0) { + /* The mp_int was so big it overflowed a MVMuint64. */ MVM_exception_throw_adhoc(tc, "Cannot unbox %d bit wide bigint into native integer", mp_count_bits(a)); } @@ -88,12 +105,7 @@ static MVMint64 get_int(MVMThreadContext *tc, MVMSTable *st, MVMObject *root, vo MVMP6bigintBody *body = (MVMP6bigintBody *)data; if (MVM_BIGINT_IS_BIG(body)) { mp_int *i = body->u.bigint; - if (MP_NEG == SIGN(i)) { - return -mp_get_int64(tc, i, 1); - } - else { - return mp_get_int64(tc, i, 1); - } + return mp_get_int64(tc, i); } else { return body->u.smallint.value; @@ -120,7 +132,7 @@ static MVMuint64 get_uint(MVMThreadContext *tc, MVMSTable *st, MVMObject *root, if (MP_NEG == SIGN(i)) MVM_exception_throw_adhoc(tc, "Cannot unbox negative bigint into native unsigned integer"); else - return mp_get_int64(tc, i, 0); + return mp_get_uint64(tc, i); } else { return body->u.smallint.value;