From f94d3e86cffb10dbebf99a358d1100a637b6b2ae Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 1 Dec 2021 18:07:52 -0600 Subject: [PATCH] UART: Don't allocate the object so early This object has a finalizer, so once it's no longer referenced, GC can call that finalizer and then deallocate the storage. In the case of a failure during construction (e.g., when checking `validate_obj_is_free_pin_or_none`) this will happen on an incompletely initialized structure. On samd, in particular, a newly allocated object (with construct never called) appears to be valid, so GC collecting it causes deinit() to do things, leading to a hard fault. The double creation of the UART object was necessary specifically so that the second allocation would fail. Probably there were other (single call) ways to make it fail, but this was the easiest / the one discovered in real life. Closes: #5493 --- shared-bindings/busio/UART.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/shared-bindings/busio/UART.c b/shared-bindings/busio/UART.c index 4df56c6e62..1819748992 100644 --- a/shared-bindings/busio/UART.c +++ b/shared-bindings/busio/UART.c @@ -82,12 +82,6 @@ STATIC void validate_timeout(mp_float_t timeout) { STATIC mp_obj_t busio_uart_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { #if CIRCUITPY_BUSIO_UART - // Always initially allocate the UART object within the long-lived heap. - // This is needed to avoid crashes with certain UART implementations which - // cannot accomodate being moved after creation. (See - // https://github.com/adafruit/circuitpython/issues/1056) - busio_uart_obj_t *self = m_new_ll_obj_with_finaliser(busio_uart_obj_t); - self->base.type = &busio_uart_type; enum { ARG_tx, ARG_rx, ARG_baudrate, ARG_bits, ARG_parity, ARG_stop, ARG_timeout, ARG_receiver_buffer_size, ARG_rts, ARG_cts, ARG_rs485_dir,ARG_rs485_invert}; static const mp_arg_t allowed_args[] = { @@ -140,6 +134,13 @@ STATIC mp_obj_t busio_uart_make_new(const mp_obj_type_t *type, size_t n_args, si const bool rs485_invert = args[ARG_rs485_invert].u_bool; + // Always initially allocate the UART object within the long-lived heap. + // This is needed to avoid crashes with certain UART implementations which + // cannot accomodate being moved after creation. (See + // https://github.com/adafruit/circuitpython/issues/1056) + busio_uart_obj_t *self = m_new_ll_obj_with_finaliser(busio_uart_obj_t); + self->base.type = &busio_uart_type; + common_hal_busio_uart_construct(self, tx, rx, rts, cts, rs485_dir, rs485_invert, args[ARG_baudrate].u_int, bits, parity, stop, timeout, args[ARG_receiver_buffer_size].u_int, NULL, false);