UART: Don't allocate the object so early

jepler · jepler · commit f94d3e86cffb · 2021-12-01T20:54:39.000-06:00
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: adafruit#5493
diff --git 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);
