dotclockframebuffer.ioexpander_send_init_sequence: gpio_data range validator wrongly limits to 0..1

[makermelissa-piclaw]

CircuitPython version

10.3.0-alpha.2 (also reproduces on every release since v8.x — bug
has been present since PR #8401 in September 2023).

Code/REPL

Any call to dotclockframebuffer.ioexpander_send_init_sequence(...)
with gpio_data_len=1 and a gpio_data value greater than 1
triggers the bug. In practice this happens during display init on
an Adafruit Qualia ESP32-S3 driving a 4" 480x480 round TFT via the
adafruit_qualia library:

from adafruit_qualia.graphics import Displays, Graphics
graphics = Graphics(Displays.SQUARE40_480X480, default_bg=None, auto_refresh=False)

Behavior

Display init aborts with:

ValueError: gpio_data must be 0..1

even though gpio_data legitimately holds a full-byte register
value (e.g. 0xff).

Description

The validator in
shared-bindings/dotclockframebuffer/__init__.c around line 107 is:

mp_arg_validate_int_range(gpio_data, 0, (1 << (max_bit * 8)) - 1, MP_QSTR_gpio_data);

max_bit is computed earlier as gpio_data_len * 8 - 1 (the highest
valid bit index inside the gpio_data field). The validator
mistakenly multiplies max_bit by 8 a second time instead of using
gpio_data_len * 8.

For gpio_data_len = 1 (the common case), max_bit = 7, so the
upper-bound expression becomes:

(1 << (7 * 8)) - 1   ==   (1 << 56) - 1

That is undefined behavior on a 32-bit mp_int_t, and in practice
wraps such that the upper bound ends up as 1. So any gpio_data
value > 1 fails validation even though a full byte is legitimate.

Historical note: the original test snippet in the introducing
commit (4b41fdb, PR #8401, "Fast(ish) special purpose bitbang spi
over i2c") used gpio_data=0xff with gpio_data_len=1, which would
have tripped the broken validator immediately — suggesting the bug
crept in between that snippet and the merged code, and was never
re-exercised.

Additional information

Fix in PR #11028: use gpio_data_len (already a length in bytes)
directly:

mp_arg_validate_int_range(gpio_data, 0, (1 << (gpio_data_len * 8)) - 1, MP_QSTR_gpio_data);

For gpio_data_len = 1 this yields the correct (1 << 8) - 1 = 255
upper bound. One-line change; no behavior change for any input that
previously passed validation.

[makermelissa]

I'm not even sure if this really is a bug, but according to openclaw, it wouldn't compile without this change. Perhaps this is the underlying cause of some other bugs. If this is the intended behavior, please feel free to close.