From 7a09af73ec7f4a11a1404dff6276e09072d06983 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 20 Jan 2019 15:10:09 -0500 Subject: Improve struct compatibility with CPython --- py/binary.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'py/binary.c') diff --git a/py/binary.c b/py/binary.c index ca851c936..9c3a49e8f 100644 --- a/py/binary.c +++ b/py/binary.c @@ -49,7 +49,7 @@ size_t mp_binary_get_size(char struct_type, char val_type, mp_uint_t *palign) { switch (struct_type) { case '<': case '>': switch (val_type) { - case 'b': case 'B': + case 'b': case 'B': case 'x': size = 1; break; case 'h': case 'H': size = 2; break; @@ -79,7 +79,7 @@ size_t mp_binary_get_size(char struct_type, char val_type, mp_uint_t *palign) { // particular (or any) ABI. switch (val_type) { case BYTEARRAY_TYPECODE: - case 'b': case 'B': + case 'b': case 'B': case 'x': align = size = 1; break; case 'h': case 'H': align = alignof(short); @@ -126,6 +126,7 @@ mp_obj_t mp_binary_get_val_array(char typecode, void *p, mp_uint_t index) { break; case BYTEARRAY_TYPECODE: case 'B': + case 'x': // value will be discarded val = ((unsigned char*)p)[index]; break; case 'h': @@ -364,6 +365,8 @@ void mp_binary_set_val_array_from_int(char typecode, void *p, mp_uint_t index, m case 'B': ((unsigned char*)p)[index] = val; break; + case 'x': + ((unsigned char*)p)[index] = 0; case 'h': ((short*)p)[index] = val; break; -- cgit v1.2.3 From 095c844004bcd8680a4bf68901adbd9cac6a4302 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Wed, 8 May 2019 23:50:35 -0400 Subject: Add overflow checks for int to bytes conversions For both small and long integers, raise an exception if calling struct.pack, adding an element to an array.array, or formatting an int with int.to_bytes would overflow the requested size. --- py/binary.c | 18 ++++++++++++++---- py/objint.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ py/objint.h | 1 + 3 files changed, 60 insertions(+), 4 deletions(-) (limited to 'py/binary.c') diff --git a/py/binary.c b/py/binary.c index 9c3a49e8f..0e4a1d5f6 100644 --- a/py/binary.c +++ b/py/binary.c @@ -304,15 +304,18 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte ** break; } #endif - default: + default: { + bool signed_type = is_signed(val_type); #if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE if (MP_OBJ_IS_TYPE(val_in, &mp_type_int)) { + mp_obj_int_buffer_overflow_check(val_in, size, signed_type); mp_obj_int_to_bytes_impl(val_in, struct_type == '>', size, p); return; } else #endif { val = mp_obj_get_int(val_in); + mp_obj_int_buffer_overflow_check(val_in, size, signed_type); // zero/sign extend if needed if (BYTES_PER_WORD < 8 && size > sizeof(val)) { int c = (is_signed(val_type) && (mp_int_t)val < 0) ? 0xff : 0x00; @@ -322,6 +325,7 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte ** } } } + } } mp_binary_set_int(MIN((size_t)size, sizeof(val)), struct_type == '>', p, val); @@ -343,16 +347,22 @@ void mp_binary_set_val_array(char typecode, void *p, mp_uint_t index, mp_obj_t v ((mp_obj_t*)p)[index] = val_in; break; #endif - default: + default: { + size_t size = mp_binary_get_size('@', typecode, NULL); + bool signed_type = is_signed(typecode); + #if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE if (MP_OBJ_IS_TYPE(val_in, &mp_type_int)) { - size_t size = mp_binary_get_size('@', typecode, NULL); + mp_obj_int_buffer_overflow_check(val_in, size, signed_type); mp_obj_int_to_bytes_impl(val_in, MP_ENDIANNESS_BIG, size, (uint8_t*)p + index * size); return; } #endif - mp_binary_set_val_array_from_int(typecode, p, index, mp_obj_get_int(val_in)); + mp_int_t val = mp_obj_get_int(val_in); + mp_obj_int_buffer_overflow_check(val_in, size, signed_type); + mp_binary_set_val_array_from_int(typecode, p, index, val); + } } } diff --git a/py/objint.c b/py/objint.c index fd746d331..fc672b112 100644 --- a/py/objint.c +++ b/py/objint.c @@ -300,6 +300,49 @@ char *mp_obj_int_formatted(char **buf, size_t *buf_size, size_t *fmt_size, mp_co return b; } +void mp_obj_int_buffer_overflow_check(mp_obj_t self_in, size_t nbytes, bool is_signed) +{ + if (is_signed) { + // edge = 1 << (nbytes * 8 - 1) + mp_obj_t edge = mp_binary_op(MP_BINARY_OP_INPLACE_LSHIFT, + mp_obj_new_int(1), + mp_obj_new_int(nbytes * 8 - 1)); + + // if self >= edge, we don't fit + if (mp_binary_op(MP_BINARY_OP_MORE_EQUAL, self_in, edge) == mp_const_true) { + goto raise; + } + + // edge = -edge + edge = mp_unary_op(MP_UNARY_OP_NEGATIVE, edge); + + // if self < edge, we don't fit + if (mp_binary_op(MP_BINARY_OP_LESS, self_in, edge) == mp_const_true) { + goto raise; + } + } else { + if (mp_obj_int_sign(self_in) < 0) { + // Negative numbers never fit in an unsigned value + goto raise; + } + + // edge = 1 << (nbytes * 8) + mp_obj_t edge = mp_binary_op(MP_BINARY_OP_INPLACE_LSHIFT, + mp_obj_new_int(1), + mp_obj_new_int(nbytes * 8)); + + // if self >= edge, we don't fit + if (mp_binary_op(MP_BINARY_OP_MORE_EQUAL, self_in, edge) == mp_const_true) { + goto raise; + } + } + + return; + +raise: + mp_raise_ValueError_varg(translate("value would overflow a %d byte buffer"), nbytes); +} + #if MICROPY_LONGINT_IMPL == MICROPY_LONGINT_IMPL_NONE int mp_obj_int_sign(mp_obj_t self_in) { @@ -435,6 +478,8 @@ STATIC mp_obj_t int_to_bytes(size_t n_args, const mp_obj_t *args) { byte *data = (byte*)vstr.buf; memset(data, 0, len); + mp_obj_int_buffer_overflow_check(args[0], len, false); + #if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE if (!MP_OBJ_IS_SMALL_INT(args[0])) { mp_obj_int_to_bytes_impl(args[0], big_endian, len, data); diff --git a/py/objint.h b/py/objint.h index 4b95acde9..654404ac9 100644 --- a/py/objint.h +++ b/py/objint.h @@ -53,6 +53,7 @@ char *mp_obj_int_formatted(char **buf, size_t *buf_size, size_t *fmt_size, mp_co int base, const char *prefix, char base_char, char comma); char *mp_obj_int_formatted_impl(char **buf, size_t *buf_size, size_t *fmt_size, mp_const_obj_t self_in, int base, const char *prefix, char base_char, char comma); +void mp_obj_int_buffer_overflow_check(mp_obj_t self_in, size_t nbytes, bool is_signed); mp_int_t mp_obj_int_hash(mp_obj_t self_in); mp_obj_t mp_obj_int_from_bytes_impl(bool big_endian, size_t len, const byte *buf); void mp_obj_int_to_bytes_impl(mp_obj_t self_in, bool big_endian, size_t len, byte *buf); -- cgit v1.2.3 From d103ac1d6325a075be3fafe812858584435f3144 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 12 May 2019 00:10:53 -0400 Subject: Handle truth values; speed up smallint checks --- py/binary.c | 8 ++++++-- py/objint.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- py/objint.h | 5 +++++ 3 files changed, 53 insertions(+), 5 deletions(-) (limited to 'py/binary.c') diff --git a/py/binary.c b/py/binary.c index 0e4a1d5f6..6b46425cf 100644 --- a/py/binary.c +++ b/py/binary.c @@ -308,6 +308,7 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte ** bool signed_type = is_signed(val_type); #if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE if (MP_OBJ_IS_TYPE(val_in, &mp_type_int)) { + // It's a longint. mp_obj_int_buffer_overflow_check(val_in, size, signed_type); mp_obj_int_to_bytes_impl(val_in, struct_type == '>', size, p); return; @@ -315,7 +316,8 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte ** #endif { val = mp_obj_get_int(val_in); - mp_obj_int_buffer_overflow_check(val_in, size, signed_type); + // Small int checking is separate, to be fast. + mp_small_int_buffer_overflow_check(val, size, signed_type); // zero/sign extend if needed if (BYTES_PER_WORD < 8 && size > sizeof(val)) { int c = (is_signed(val_type) && (mp_int_t)val < 0) ? 0xff : 0x00; @@ -353,6 +355,7 @@ void mp_binary_set_val_array(char typecode, void *p, mp_uint_t index, mp_obj_t v #if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE if (MP_OBJ_IS_TYPE(val_in, &mp_type_int)) { + // It's a long int. mp_obj_int_buffer_overflow_check(val_in, size, signed_type); mp_obj_int_to_bytes_impl(val_in, MP_ENDIANNESS_BIG, size, (uint8_t*)p + index * size); @@ -360,7 +363,8 @@ void mp_binary_set_val_array(char typecode, void *p, mp_uint_t index, mp_obj_t v } #endif mp_int_t val = mp_obj_get_int(val_in); - mp_obj_int_buffer_overflow_check(val_in, size, signed_type); + // Small int checking is separate, to be fast. + mp_small_int_buffer_overflow_check(val, size, signed_type); mp_binary_set_val_array_from_int(typecode, p, index, val); } } diff --git a/py/objint.c b/py/objint.c index 80f6dfe55..3b3a3d9c0 100644 --- a/py/objint.c +++ b/py/objint.c @@ -300,6 +300,8 @@ char *mp_obj_int_formatted(char **buf, size_t *buf_size, size_t *fmt_size, mp_co return b; } +#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE + void mp_obj_int_buffer_overflow_check(mp_obj_t self_in, size_t nbytes, bool is_signed) { if (is_signed) { @@ -329,7 +331,43 @@ void mp_obj_int_buffer_overflow_check(mp_obj_t self_in, size_t nbytes, bool is_s } } - mp_raise_OverflowError_varg(translate("value would overflow a %d byte buffer"), nbytes); + mp_raise_OverflowError_varg(translate("value must fit in %d byte(s)"), nbytes); +} + +#endif // MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE + +void mp_small_int_buffer_overflow_check(mp_int_t val, size_t nbytes, bool is_signed) { + // Fast path for zero. + if (val == 0) return; + if (!is_signed) { + if (val >= 0) { + // Using signed constants here, not UINT8_MAX, etc. to avoid any unintended conversions. + if (val <= 0xff) return; // Small values fit in any number of nbytes. + if (nbytes == 2 && val <= 0xffff) return; +#if !defined(__LP64__) + // 32-bit ints and pointers + if (nbytes >= 4) return; // Any mp_int_t will fit. +#else + // 64-bit ints and pointers + if (nbytes == 4 && val <= 0xffffffff) return; + if (nbytes >= 8) return; // Any mp_int_t will fit. +#endif + } // Negative, fall through to failure. + } else { + // signed + if (val >= INT8_MIN && val <= INT8_MAX) return; // Small values fit in any number of nbytes. + if (nbytes == 2 && val >= INT16_MIN && val <= INT16_MAX) return; +#if !defined(__LP64__) + // 32-bit ints and pointers + if (nbytes >= 4) return; // Any mp_int_t will fit. +#else + // 64-bit ints and pointers + if (nbytes == 4 && val >= INT32_MIN && val <= INT32_MAX) return; + if (nbytes >= 8) return; // Any mp_int_t will fit. +#endif + } // Fall through to failure. + + mp_raise_OverflowError_varg(translate("value must fit in %d byte(s)"), nbytes); } #if MICROPY_LONGINT_IMPL == MICROPY_LONGINT_IMPL_NONE @@ -467,15 +505,16 @@ STATIC mp_obj_t int_to_bytes(size_t n_args, const mp_obj_t *args) { byte *data = (byte*)vstr.buf; memset(data, 0, len); - mp_obj_int_buffer_overflow_check(args[0], len, false); - #if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE if (!MP_OBJ_IS_SMALL_INT(args[0])) { + mp_obj_int_buffer_overflow_check(args[0], len, false); mp_obj_int_to_bytes_impl(args[0], big_endian, len, data); } else #endif { mp_int_t val = MP_OBJ_SMALL_INT_VALUE(args[0]); + // Small int checking is separate, to be fast. + mp_small_int_buffer_overflow_check(val, len, false); size_t l = MIN((size_t)len, sizeof(val)); mp_binary_set_int(l, big_endian, data + (big_endian ? (len - l) : 0), val); } diff --git a/py/objint.h b/py/objint.h index 654404ac9..e8c9bc3e0 100644 --- a/py/objint.h +++ b/py/objint.h @@ -53,7 +53,12 @@ char *mp_obj_int_formatted(char **buf, size_t *buf_size, size_t *fmt_size, mp_co int base, const char *prefix, char base_char, char comma); char *mp_obj_int_formatted_impl(char **buf, size_t *buf_size, size_t *fmt_size, mp_const_obj_t self_in, int base, const char *prefix, char base_char, char comma); +#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE void mp_obj_int_buffer_overflow_check(mp_obj_t self_in, size_t nbytes, bool is_signed); +#endif + +void mp_small_int_buffer_overflow_check(mp_int_t val, size_t nbytes, bool is_signed); + mp_int_t mp_obj_int_hash(mp_obj_t self_in); mp_obj_t mp_obj_int_from_bytes_impl(bool big_endian, size_t len, const byte *buf); void mp_obj_int_to_bytes_impl(mp_obj_t self_in, bool big_endian, size_t len, byte *buf); -- cgit v1.2.3 From 0d96f1906b66139beebc13c55078aaf610c76968 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Tue, 8 Oct 2019 10:48:25 +0900 Subject: mp_binary_get_int: avoid undefined behavior Left shift of negative numbers is undefined in the "C" standard. Multiplying by 256 has the intended effect (in the absence of integer overflow, anyway), can be implemented using the same shift instruction, but does not invoke undefined behavior. This problem was found using clang 7's scan-build static analyzer. --- py/binary.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'py/binary.c') diff --git a/py/binary.c b/py/binary.c index 6b46425cf..2ec12fa93 100644 --- a/py/binary.c +++ b/py/binary.c @@ -184,7 +184,7 @@ long long mp_binary_get_int(mp_uint_t size, bool is_signed, bool big_endian, con val = -1; } for (uint i = 0; i < size; i++) { - val <<= 8; + val *= 256; val |= *src; src += delta; } -- cgit v1.2.3