Setting large int in an unsigned long array causes overflow

[waveform80]

The following code raises OverflowError on the ESP32 (using MPZ for longint):

from array import array
a = array('L', (0,))
a[0] = 0x80000000

Specifically:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: overflow converting long int to machine word

Given the array's type-code is for unsigned longs, it should presumably be able to store a 32-bit integer with the top bit set. The issue proceeds more or less as follows:

• The array calls mp_binary_set_val_array (with typecode 'L', so it knows this is an unsigned value too).
• mp_binary_set_val_array calls mp_obj_get_int to convert the value to an mp_int_t, expecting to pass this onto mp_binary_set_val_array_from_int.
• Unfortunately at this point mp_obj_get_int expects a signed int and ultimately mpz_as_int_checked in mpz.c is called which indicates overflow has occurred.

One way of fixing this might be to introduce an mp_obj_get_uint routine, similar to mp_obj_get_int but which ultimately calls mpz_as_uint_checked (and which returns an mp_uint_t, naturally). Then in mp_binary_set_val_array move the conversion down into mp_binary_set_val_array_from_int and have that call mp_obj_get_int or mp_obj_get_uint depending on whether the typecode is signed or unsigned. If that sounds sensible to people, I'm happy to have a crack at implementing it?

[dpgeorge]

Thanks for the report. I'm surprised this wasn't found earlier!

There were actually issues with more than just the 'L' typecode, also for 'l', 'i', 'I' and (a MicroPython extension) 'P'. The fix was rather simple, the code to handle these types was already there in the form of mp_obj_int_to_bytes_impl, it just needed some minor tweaks. I fixed it upstream in micropython/micropython@6cfa61a

Let's keep this issue open until the changes are merged into this repo (should be soon).

[waveform80]

Ah, that's much simpler than my proposal! Excellent news - I'll keep an eye out for the merge.

[dpgeorge]

Now fixed and merged.