From cf97793af8fbf7fe372568370c4e93875d0a79c5 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Sun, 27 Sep 2020 12:58:51 +0200 Subject: [PATCH 1/2] Add supervisor.get_previous_traceback() function. Useful for #1084. --- lib/utils/pyexec.c | 2 +- lib/utils/pyexec.h | 2 +- main.c | 52 ++++++++++++++++++++++----- shared-bindings/supervisor/__init__.c | 30 +++++++++++++++- supervisor/shared/memory.c | 2 ++ supervisor/shared/traceback.c | 29 +++++++++++++++ supervisor/shared/traceback.h | 34 ++++++++++++++++++ supervisor/supervisor.mk | 1 + 8 files changed, 140 insertions(+), 12 deletions(-) create mode 100644 supervisor/shared/traceback.c create mode 100644 supervisor/shared/traceback.h diff --git a/lib/utils/pyexec.c b/lib/utils/pyexec.c index b0249f9d30..2651189915 100644 --- a/lib/utils/pyexec.c +++ b/lib/utils/pyexec.c @@ -167,7 +167,7 @@ STATIC int parse_compile_execute(const void *source, mp_parse_input_kind_t input result->return_code = ret; if (ret != 0) { mp_obj_t return_value = (mp_obj_t)nlr.ret_val; - result->exception_type = mp_obj_get_type(return_value); + result->exception = return_value; result->exception_line = -1; if (mp_obj_is_exception_instance(return_value)) { diff --git a/lib/utils/pyexec.h b/lib/utils/pyexec.h index 4924f8776a..6f0252cfed 100644 --- a/lib/utils/pyexec.h +++ b/lib/utils/pyexec.h @@ -35,7 +35,7 @@ typedef enum { typedef struct { int return_code; - const mp_obj_type_t *exception_type; + mp_obj_t exception; int exception_line; } pyexec_result_t; diff --git a/main.c b/main.c index 00cf1acaa7..0bbf7b71ea 100755 --- a/main.c +++ b/main.c @@ -56,6 +56,7 @@ #include "supervisor/shared/safe_mode.h" #include "supervisor/shared/stack.h" #include "supervisor/shared/status_leds.h" +#include "supervisor/shared/traceback.h" #include "supervisor/shared/translate.h" #include "supervisor/shared/workflow.h" #include "supervisor/usb.h" @@ -227,7 +228,41 @@ STATIC bool maybe_run_list(const char * const * filenames, pyexec_result_t* exec return true; } -STATIC void cleanup_after_vm(supervisor_allocation* heap) { +STATIC void count_strn(void *data, const char *str, size_t len) { + *(size_t*)data += len; +} + +STATIC void cleanup_after_vm(supervisor_allocation* heap, mp_obj_t exception) { + // Get the traceback of any exception from this run off the heap. + // MP_OBJ_SENTINEL means "this run does not contribute to traceback storage, don't touch it" + // MP_OBJ_NULL (=0) means "this run completed successfully, clear any stored traceback" + if (exception != MP_OBJ_SENTINEL) { + free_memory(prev_traceback_allocation); + // ReloadException is exempt from traceback printing in pyexec_file(), so treat it as "no + // traceback" here too. + if (exception && exception != MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_reload_exception))) { + size_t traceback_len = 0; + mp_print_t print_count = {&traceback_len, count_strn}; + mp_obj_print_exception(&print_count, exception); + prev_traceback_allocation = allocate_memory(align32_size(traceback_len + 1), false, true); + // Empirically, this never fails in practice - even when the heap is totally filled up + // with single-block-sized objects referenced by a root pointer, exiting the VM frees + // up several hundred bytes, sufficient for the traceback (which tends to be shortened + // because there wasn't memory for the full one). There may be convoluted ways of + // making it fail, but at this point I believe they are not worth spending code on. + if (prev_traceback_allocation != NULL) { + vstr_t vstr; + vstr_init_fixed_buf(&vstr, traceback_len, (char*)prev_traceback_allocation->ptr); + mp_print_t print = {&vstr, (mp_print_strn_t)vstr_add_strn}; + mp_obj_print_exception(&print, exception); + ((char*)prev_traceback_allocation->ptr)[traceback_len] = '\0'; + } + } + else { + prev_traceback_allocation = NULL; + } + } + // Reset port-independent devices, like CIRCUITPY_BLEIO_HCI. reset_devices(); // Turn off the display and flush the filesystem before the heap disappears. @@ -287,7 +322,7 @@ STATIC bool run_code_py(safe_mode_t safe_mode) { pyexec_result_t result; result.return_code = 0; - result.exception_type = NULL; + result.exception = MP_OBJ_NULL; result.exception_line = 0; bool skip_repl; @@ -357,7 +392,7 @@ STATIC bool run_code_py(safe_mode_t safe_mode) { } // Finished executing python code. Cleanup includes a board reset. - cleanup_after_vm(heap); + cleanup_after_vm(heap, result.exception); // If a new next code file was set, that is a reason to keep it (obviously). Stuff this into // the options because it can be treated like any other reason-for-stickiness bit. The @@ -674,8 +709,9 @@ STATIC void __attribute__ ((noinline)) run_boot_py(safe_mode_t safe_mode) { usb_set_defaults(); #endif + pyexec_result_t result = {0, MP_OBJ_NULL, 0}; if (ok_to_run) { - bool found_boot = maybe_run_list(boot_py_filenames, NULL); + bool found_boot = maybe_run_list(boot_py_filenames, &result); (void) found_boot; #ifdef CIRCUITPY_BOOT_OUTPUT_FILE @@ -687,21 +723,18 @@ STATIC void __attribute__ ((noinline)) run_boot_py(safe_mode_t safe_mode) { #endif } - #if CIRCUITPY_USB - // Some data needs to be carried over from the USB settings in boot.py // to the next VM, while the heap is still available. // Its size can vary, so save it temporarily on the stack, // and then when the heap goes away, copy it in into a // storage_allocation. - size_t size = usb_boot_py_data_size(); uint8_t usb_boot_py_data[size]; usb_get_boot_py_data(usb_boot_py_data, size); #endif - cleanup_after_vm(heap); + cleanup_after_vm(heap, result.exception); #if CIRCUITPY_USB // Now give back the data we saved from the heap going away. @@ -736,12 +769,13 @@ STATIC int run_repl(void) { } else { exit_code = pyexec_friendly_repl(); } - cleanup_after_vm(heap); + cleanup_after_vm(heap, MP_OBJ_SENTINEL); #if CIRCUITPY_STATUS_LED status_led_init(); new_status_color(BLACK); status_led_deinit(); #endif + autoreload_resume(); return exit_code; } diff --git a/shared-bindings/supervisor/__init__.c b/shared-bindings/supervisor/__init__.c index 561be0ba29..d3ad9e6fba 100644 --- a/shared-bindings/supervisor/__init__.c +++ b/shared-bindings/supervisor/__init__.c @@ -34,6 +34,7 @@ #include "supervisor/shared/autoreload.h" #include "supervisor/shared/status_leds.h" #include "supervisor/shared/stack.h" +#include "supervisor/shared/traceback.h" #include "supervisor/shared/translate.h" #include "supervisor/shared/workflow.h" @@ -255,7 +256,33 @@ STATIC mp_obj_t supervisor_ticks_ms(void) { } MP_DEFINE_CONST_FUN_OBJ_0(supervisor_ticks_ms_obj, supervisor_ticks_ms); - +//| def get_previous_traceback() -> Optional[str]: +//| """If the last vm run ended with an exception (including the KeyboardInterrupt caused by +//| CTRL-C), returns the traceback as a string. +//| Otherwise, returns ``None``. +//| +//| An exception traceback is only preserved over a soft reload, a hard reset clears it. +//| +//| Only code (main or boot) runs are considered, not REPL runs.""" +//| ... +//| +STATIC mp_obj_t supervisor_get_previous_traceback(void) { + //TODO is this a safe and proper way of making a heap-allocated string object that points at long-lived (and likely long and unique) data outside the heap? + if (prev_traceback_allocation) { + size_t len = strlen((const char*)prev_traceback_allocation->ptr); + if (len > 0) { + mp_obj_str_t *o = m_new_obj(mp_obj_str_t); + o->base.type = &mp_type_str; + o->len = len; + //TODO is it a good assumption that callers probably aren't going to compare this string, so skip computing the hash? + o->hash = 0; + o->data = (const byte*)prev_traceback_allocation->ptr; + return MP_OBJ_FROM_PTR(o); + } + } + return mp_const_none; +} +MP_DEFINE_CONST_FUN_OBJ_0(supervisor_get_previous_traceback_obj, supervisor_get_previous_traceback); STATIC const mp_rom_map_elem_t supervisor_module_globals_table[] = { { MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_supervisor) }, @@ -268,6 +295,7 @@ STATIC const mp_rom_map_elem_t supervisor_module_globals_table[] = { { MP_ROM_QSTR(MP_QSTR_set_next_stack_limit), MP_ROM_PTR(&supervisor_set_next_stack_limit_obj) }, { MP_ROM_QSTR(MP_QSTR_set_next_code_file), MP_ROM_PTR(&supervisor_set_next_code_file_obj) }, { MP_ROM_QSTR(MP_QSTR_ticks_ms), MP_ROM_PTR(&supervisor_ticks_ms_obj) }, + { MP_ROM_QSTR(MP_QSTR_get_previous_traceback), MP_ROM_PTR(&supervisor_get_previous_traceback_obj) }, }; STATIC MP_DEFINE_CONST_DICT(supervisor_module_globals, supervisor_module_globals_table); diff --git a/supervisor/shared/memory.c b/supervisor/shared/memory.c index 3cdaeff3bb..04f2f571db 100644 --- a/supervisor/shared/memory.c +++ b/supervisor/shared/memory.c @@ -38,6 +38,8 @@ enum { 2 // next_code_allocation + 1 + // prev_traceback_allocation + + 1 #if INTERNAL_FLASH_FILESYSTEM == 0 + 1 diff --git a/supervisor/shared/traceback.c b/supervisor/shared/traceback.c new file mode 100644 index 0000000000..65c227b526 --- /dev/null +++ b/supervisor/shared/traceback.c @@ -0,0 +1,29 @@ +/* + * This file is part of the MicroPython project, http://micropython.org/ + * + * The MIT License (MIT) + * + * Copyright (c) 2020 Christian Walther + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "traceback.h" + +supervisor_allocation* prev_traceback_allocation; diff --git a/supervisor/shared/traceback.h b/supervisor/shared/traceback.h new file mode 100644 index 0000000000..6f0fa2d33a --- /dev/null +++ b/supervisor/shared/traceback.h @@ -0,0 +1,34 @@ +/* + * This file is part of the MicroPython project, http://micropython.org/ + * + * The MIT License (MIT) + * + * Copyright (c) 2020 Christian Walther + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef MICROPY_INCLUDED_SUPERVISOR_TRACEBACK_H +#define MICROPY_INCLUDED_SUPERVISOR_TRACEBACK_H + +#include "supervisor/memory.h" + +extern supervisor_allocation* prev_traceback_allocation; + +#endif // MICROPY_INCLUDED_SUPERVISOR_TRACEBACK_H diff --git a/supervisor/supervisor.mk b/supervisor/supervisor.mk index 6d06a9ada9..7484c56092 100644 --- a/supervisor/supervisor.mk +++ b/supervisor/supervisor.mk @@ -14,6 +14,7 @@ SRC_SUPERVISOR = \ supervisor/shared/stack.c \ supervisor/shared/status_leds.c \ supervisor/shared/tick.c \ + supervisor/shared/traceback.c \ supervisor/shared/translate.c NO_USB ?= $(wildcard supervisor/usb.c) From ace04ef6003387773b29ac59a9d1e0df0aca9da2 Mon Sep 17 00:00:00 2001 From: Lucian Copeland Date: Thu, 22 Jul 2021 12:57:10 -0400 Subject: [PATCH 2/2] Formatting fixes --- ports/esp32s2/common-hal/busio/SPI.h | 6 +++--- shared-bindings/supervisor/__init__.c | 8 ++++---- supervisor/shared/traceback.c | 2 +- supervisor/shared/traceback.h | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ports/esp32s2/common-hal/busio/SPI.h b/ports/esp32s2/common-hal/busio/SPI.h index 34ec8665c6..87c538babd 100644 --- a/ports/esp32s2/common-hal/busio/SPI.h +++ b/ports/esp32s2/common-hal/busio/SPI.h @@ -33,9 +33,9 @@ typedef struct { mp_obj_base_t base; - const mcu_pin_obj_t* MOSI; - const mcu_pin_obj_t* MISO; - const mcu_pin_obj_t* clock; + const mcu_pin_obj_t *MOSI; + const mcu_pin_obj_t *MISO; + const mcu_pin_obj_t *clock; spi_host_device_t host_id; diff --git a/shared-bindings/supervisor/__init__.c b/shared-bindings/supervisor/__init__.c index d3ad9e6fba..83b4736164 100644 --- a/shared-bindings/supervisor/__init__.c +++ b/shared-bindings/supervisor/__init__.c @@ -267,16 +267,16 @@ MP_DEFINE_CONST_FUN_OBJ_0(supervisor_ticks_ms_obj, supervisor_ticks_ms); //| ... //| STATIC mp_obj_t supervisor_get_previous_traceback(void) { - //TODO is this a safe and proper way of making a heap-allocated string object that points at long-lived (and likely long and unique) data outside the heap? + // TODO is this a safe and proper way of making a heap-allocated string object that points at long-lived (and likely long and unique) data outside the heap? if (prev_traceback_allocation) { - size_t len = strlen((const char*)prev_traceback_allocation->ptr); + size_t len = strlen((const char *)prev_traceback_allocation->ptr); if (len > 0) { mp_obj_str_t *o = m_new_obj(mp_obj_str_t); o->base.type = &mp_type_str; o->len = len; - //TODO is it a good assumption that callers probably aren't going to compare this string, so skip computing the hash? + // TODO is it a good assumption that callers probably aren't going to compare this string, so skip computing the hash? o->hash = 0; - o->data = (const byte*)prev_traceback_allocation->ptr; + o->data = (const byte *)prev_traceback_allocation->ptr; return MP_OBJ_FROM_PTR(o); } } diff --git a/supervisor/shared/traceback.c b/supervisor/shared/traceback.c index 65c227b526..441379460c 100644 --- a/supervisor/shared/traceback.c +++ b/supervisor/shared/traceback.c @@ -26,4 +26,4 @@ #include "traceback.h" -supervisor_allocation* prev_traceback_allocation; +supervisor_allocation *prev_traceback_allocation; diff --git a/supervisor/shared/traceback.h b/supervisor/shared/traceback.h index 6f0fa2d33a..dd3c72c8e2 100644 --- a/supervisor/shared/traceback.h +++ b/supervisor/shared/traceback.h @@ -29,6 +29,6 @@ #include "supervisor/memory.h" -extern supervisor_allocation* prev_traceback_allocation; +extern supervisor_allocation *prev_traceback_allocation; #endif // MICROPY_INCLUDED_SUPERVISOR_TRACEBACK_H