Skip to content

Commit 9d68e9c

Browse files
dpgeorgedpgeorge
committed
py: Implement integer overflow checking for * and << ops.
If operation will overflow, a multi-precision integer is created.
1 parent bb4a43f commit 9d68e9c

5 files changed

Lines changed: 131 additions & 32 deletions

File tree

‎py/obj.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ typedef struct _mp_obj_base_t mp_obj_base_t;
2929
// - xxxx...xx00: a pointer to an mp_obj_base_t
3030

3131
// In SMALL_INT, next-to-highest bits is used as sign, so both must match for value in range
32+
#define MP_SMALL_INT_MIN ((mp_small_int_t)(((machine_int_t)WORD_MSBIT_HIGH) >> 1))
33+
#define MP_SMALL_INT_MAX ((mp_small_int_t)(~(MP_SMALL_INT_MIN)))
3234
#define MP_OBJ_FITS_SMALL_INT(n) ((((n) ^ ((n) << 1)) & WORD_MSBIT_HIGH) == 0)
3335
#define MP_OBJ_IS_SMALL_INT(o) ((((mp_small_int_t)(o)) & 1) != 0)
3436
#define MP_OBJ_IS_QSTR(o) ((((mp_small_int_t)(o)) & 3) == 2)
@@ -218,9 +220,7 @@ mp_obj_t mp_obj_new_cell(mp_obj_t obj);
218220
mp_obj_t mp_obj_new_int(machine_int_t value);
219221
mp_obj_t mp_obj_new_int_from_uint(machine_uint_t value);
220222
mp_obj_t mp_obj_new_int_from_long_str(const char *s);
221-
#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE
222-
mp_obj_t mp_obj_new_int_from_ll(long long val);
223-
#endif
223+
mp_obj_t mp_obj_new_int_from_ll(long long val); // this must return a multi-precision integer object (or raise an overflow exception)
224224
mp_obj_t mp_obj_new_str(const byte* data, uint len, bool make_qstr_if_not_already);
225225
mp_obj_t mp_obj_new_bytes(const byte* data, uint len);
226226
#if MICROPY_ENABLE_FLOAT

‎py/objfloat.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
#include "formatfloat.h"
1818
#endif
1919

20-
mp_obj_t mp_obj_new_float(mp_float_t value);
21-
2220
STATIC void float_print(void (*print)(void *env, const char *fmt, ...), void *env, mp_obj_t o_in, mp_print_kind_t kind) {
2321
mp_obj_float_t *o = o_in;
2422
#if MICROPY_FLOAT_IMPL == MICROPY_FLOAT_IMPL_FLOAT

‎py/objint.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ mp_obj_t mp_obj_new_int_from_long_str(const char *s) {
7171
return mp_const_none;
7272
}
7373

74+
// This is called when an integer larger than a SMALL_INT is needed (although val might still fit in a SMALL_INT)
75+
mp_obj_t mp_obj_new_int_from_ll(long long val) {
76+
nlr_jump(mp_obj_new_exception_msg(&mp_type_OverflowError, "small int overflow"));
77+
return mp_const_none;
78+
}
79+
7480
mp_obj_t mp_obj_new_int_from_uint(machine_uint_t value) {
7581
// SMALL_INT accepts only signed numbers, of one bit less size
7682
// then word size, which totals 2 bits less for unsigned numbers.

‎py/objint_mpz.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ mp_obj_t mp_obj_new_int(machine_int_t value) {
161161

162162
mp_obj_t mp_obj_new_int_from_ll(long long val) {
163163
mp_obj_int_t *o = mp_obj_int_new_mpz();
164-
mpz_set_from_int(&o->mpz, val);
164+
mpz_set_from_ll(&o->mpz, val);
165165
return o;
166166
}
167167

‎py/runtime.c‎

Lines changed: 121 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -455,16 +455,23 @@ mp_obj_t rt_unary_op(int op, mp_obj_t arg) {
455455
if (MP_OBJ_IS_SMALL_INT(arg)) {
456456
mp_small_int_t val = MP_OBJ_SMALL_INT_VALUE(arg);
457457
switch (op) {
458-
case RT_UNARY_OP_BOOL: return MP_BOOL(val != 0);
459-
case RT_UNARY_OP_POSITIVE: break;
460-
case RT_UNARY_OP_NEGATIVE: val = -val; break;
461-
case RT_UNARY_OP_INVERT: val = ~val; break;
462-
default: assert(0); val = 0;
463-
}
464-
if (MP_OBJ_FITS_SMALL_INT(val)) {
465-
return MP_OBJ_NEW_SMALL_INT(val);
458+
case RT_UNARY_OP_BOOL:
459+
return MP_BOOL(val != 0);
460+
case RT_UNARY_OP_POSITIVE:
461+
return arg;
462+
case RT_UNARY_OP_NEGATIVE:
463+
// check for overflow
464+
if (val == MP_SMALL_INT_MIN) {
465+
return mp_obj_new_int(-val);
466+
} else {
467+
return MP_OBJ_NEW_SMALL_INT(-val);
468+
}
469+
case RT_UNARY_OP_INVERT:
470+
return MP_OBJ_NEW_SMALL_INT(~val);
471+
default:
472+
assert(0);
473+
return arg;
466474
}
467-
return mp_obj_new_int(val);
468475
} else {
469476
mp_obj_type_t *type = mp_obj_get_type(arg);
470477
if (type->unary_op != NULL) {
@@ -532,6 +539,15 @@ mp_obj_t rt_binary_op(int op, mp_obj_t lhs, mp_obj_t rhs) {
532539
mp_small_int_t lhs_val = MP_OBJ_SMALL_INT_VALUE(lhs);
533540
if (MP_OBJ_IS_SMALL_INT(rhs)) {
534541
mp_small_int_t rhs_val = MP_OBJ_SMALL_INT_VALUE(rhs);
542+
// This is a binary operation: lhs_val op rhs_val
543+
// We need to be careful to handle overflow; see CERT INT32-C
544+
// Operations that can overflow:
545+
// + result always fits in machine_int_t, then handled by SMALL_INT check
546+
// - result always fits in machine_int_t, then handled by SMALL_INT check
547+
// * checked explicitly
548+
// / if lhs=MIN and rhs=-1; result always fits in machine_int_t, then handled by SMALL_INT check
549+
// % if lhs=MIN and rhs=-1; result always fits in machine_int_t, then handled by SMALL_INT check
550+
// << checked explicitly
535551
switch (op) {
536552
case RT_BINARY_OP_OR:
537553
case RT_BINARY_OP_INPLACE_OR: lhs_val |= rhs_val; break;
@@ -540,41 +556,117 @@ mp_obj_t rt_binary_op(int op, mp_obj_t lhs, mp_obj_t rhs) {
540556
case RT_BINARY_OP_AND:
541557
case RT_BINARY_OP_INPLACE_AND: lhs_val &= rhs_val; break;
542558
case RT_BINARY_OP_LSHIFT:
543-
case RT_BINARY_OP_INPLACE_LSHIFT: lhs_val <<= rhs_val; break;
559+
case RT_BINARY_OP_INPLACE_LSHIFT: {
560+
if (rhs_val < 0) {
561+
// negative shift not allowed
562+
nlr_jump(mp_obj_new_exception_msg(&mp_type_ValueError, "negative shift count"));
563+
} else if (rhs_val >= BITS_PER_WORD || lhs_val > (MP_SMALL_INT_MAX >> rhs_val) || lhs_val < (MP_SMALL_INT_MIN >> rhs_val)) {
564+
// left-shift will overflow, so use higher precision integer
565+
lhs = mp_obj_new_int_from_ll(lhs_val);
566+
goto generic_binary_op;
567+
} else {
568+
// use standard precision
569+
lhs_val <<= rhs_val;
570+
}
571+
break;
572+
}
544573
case RT_BINARY_OP_RSHIFT:
545-
case RT_BINARY_OP_INPLACE_RSHIFT: lhs_val >>= rhs_val; break;
574+
case RT_BINARY_OP_INPLACE_RSHIFT:
575+
if (rhs_val < 0) {
576+
// negative shift not allowed
577+
nlr_jump(mp_obj_new_exception_msg(&mp_type_ValueError, "negative shift count"));
578+
} else {
579+
// standard precision is enough for right-shift
580+
lhs_val >>= rhs_val;
581+
}
582+
break;
546583
case RT_BINARY_OP_ADD:
547584
case RT_BINARY_OP_INPLACE_ADD: lhs_val += rhs_val; break;
548585
case RT_BINARY_OP_SUBTRACT:
549586
case RT_BINARY_OP_INPLACE_SUBTRACT: lhs_val -= rhs_val; break;
550587
case RT_BINARY_OP_MULTIPLY:
551-
case RT_BINARY_OP_INPLACE_MULTIPLY: lhs_val *= rhs_val; break;
588+
case RT_BINARY_OP_INPLACE_MULTIPLY: {
589+
590+
// If long long type exists and is larger than machine_int_t, then
591+
// we can use the following code to perform overflow-checked multiplication.
592+
// Otherwise (eg in x64 case) we must use the branching code below.
593+
#if 0
594+
// compute result using long long precision
595+
long long res = (long long)lhs_val * (long long)rhs_val;
596+
if (res > MP_SMALL_INT_MAX || res < MP_SMALL_INT_MIN) {
597+
// result overflowed SMALL_INT, so return higher precision integer
598+
return mp_obj_new_int_from_ll(res);
599+
} else {
600+
// use standard precision
601+
lhs_val = (mp_small_int_t)res;
602+
}
603+
#endif
604+
605+
if (lhs_val > 0) { // lhs_val is positive
606+
if (rhs_val > 0) { // lhs_val and rhs_val are positive
607+
if (lhs_val > (MP_SMALL_INT_MAX / rhs_val)) {
608+
goto mul_overflow;
609+
}
610+
} else { // lhs_val positive, rhs_val nonpositive
611+
if (rhs_val < (MP_SMALL_INT_MIN / lhs_val)) {
612+
goto mul_overflow;
613+
}
614+
} // lhs_val positive, rhs_val nonpositive
615+
} else { // lhs_val is nonpositive
616+
if (rhs_val > 0) { // lhs_val is nonpositive, rhs_val is positive
617+
if (lhs_val < (MP_SMALL_INT_MIN / rhs_val)) {
618+
goto mul_overflow;
619+
}
620+
} else { // lhs_val and rhs_val are nonpositive
621+
if (lhs_val != 0 && rhs_val < (MP_SMALL_INT_MAX / lhs_val)) {
622+
goto mul_overflow;
623+
}
624+
} // End if lhs_val and rhs_val are nonpositive
625+
} // End if lhs_val is nonpositive
626+
627+
// use standard precision
628+
return MP_OBJ_NEW_SMALL_INT(lhs_val * rhs_val);
629+
630+
mul_overflow:
631+
// use higher precision
632+
lhs = mp_obj_new_int_from_ll(lhs_val);
633+
goto generic_binary_op;
634+
635+
break;
636+
}
552637
case RT_BINARY_OP_FLOOR_DIVIDE:
553638
case RT_BINARY_OP_INPLACE_FLOOR_DIVIDE: lhs_val /= rhs_val; break;
554-
#if MICROPY_ENABLE_FLOAT
639+
#if MICROPY_ENABLE_FLOAT
555640
case RT_BINARY_OP_TRUE_DIVIDE:
556641
case RT_BINARY_OP_INPLACE_TRUE_DIVIDE: return mp_obj_new_float((mp_float_t)lhs_val / (mp_float_t)rhs_val);
557-
#endif
642+
#endif
558643

559644
// TODO implement modulo as specified by Python
560645
case RT_BINARY_OP_MODULO:
561646
case RT_BINARY_OP_INPLACE_MODULO: lhs_val %= rhs_val; break;
562647

563-
// TODO check for negative power, and overflow
564648
case RT_BINARY_OP_POWER:
565649
case RT_BINARY_OP_INPLACE_POWER:
566-
{
567-
int ans = 1;
568-
while (rhs_val > 0) {
569-
if (rhs_val & 1) {
570-
ans *= lhs_val;
650+
if (rhs_val < 0) {
651+
#if MICROPY_ENABLE_FLOAT
652+
lhs = mp_obj_new_float(lhs_val);
653+
goto generic_binary_op;
654+
#else
655+
nlr_jump(mp_obj_new_exception_msg(&mp_type_ValueError, "negative power with no float support"));
656+
#endif
657+
} else {
658+
// TODO check for overflow
659+
machine_int_t ans = 1;
660+
while (rhs_val > 0) {
661+
if (rhs_val & 1) {
662+
ans *= lhs_val;
663+
}
664+
lhs_val *= lhs_val;
665+
rhs_val /= 2;
571666
}
572-
lhs_val *= lhs_val;
573-
rhs_val /= 2;
667+
lhs_val = ans;
574668
}
575-
lhs_val = ans;
576669
break;
577-
}
578670
case RT_BINARY_OP_LESS: return MP_BOOL(lhs_val < rhs_val); break;
579671
case RT_BINARY_OP_MORE: return MP_BOOL(lhs_val > rhs_val); break;
580672
case RT_BINARY_OP_LESS_EQUAL: return MP_BOOL(lhs_val <= rhs_val); break;
@@ -585,8 +677,9 @@ mp_obj_t rt_binary_op(int op, mp_obj_t lhs, mp_obj_t rhs) {
585677
// TODO: We just should make mp_obj_new_int() inline and use that
586678
if (MP_OBJ_FITS_SMALL_INT(lhs_val)) {
587679
return MP_OBJ_NEW_SMALL_INT(lhs_val);
680+
} else {
681+
return mp_obj_new_int(lhs_val);
588682
}
589-
return mp_obj_new_int(lhs_val);
590683
#if MICROPY_ENABLE_FLOAT
591684
} else if (MP_OBJ_IS_TYPE(rhs, &mp_type_float)) {
592685
return mp_obj_float_binary_op(op, lhs_val, rhs);
@@ -628,7 +721,9 @@ mp_obj_t rt_binary_op(int op, mp_obj_t lhs, mp_obj_t rhs) {
628721
}
629722

630723
// generic binary_op supplied by type
631-
mp_obj_type_t *type = mp_obj_get_type(lhs);
724+
mp_obj_type_t *type;
725+
generic_binary_op:
726+
type = mp_obj_get_type(lhs);
632727
if (type->binary_op != NULL) {
633728
mp_obj_t result = type->binary_op(op, lhs, rhs);
634729
if (result != MP_OBJ_NULL) {

0 commit comments

Comments
 (0)