Add stack validity check and raise an error when it happens. #1294
Conversation
There was a problem hiding this comment.
Good idea!
I think we should still also sprinkle MP_STACK_CHECK calls around in a few places, like before or after imports. I think would catch the stack getting into the exception zone before the heap was smashed. See mp_stack_check() in py/stackctrl.c. That could be added as another fix for #1283.
Would this catch #1279? That would be something to test.
supervisor/shared/stack.c
Outdated
|
|
||
| inline void assert_heap_ok(void) { | ||
| if (!stack_ok()) { | ||
| mp_raise_RuntimeError(translate("Stack clobbered heap.")); |
There was a problem hiding this comment.
Since this is a very bad error, we should probably also say something like "Please restart" or "Please type ctrl-D". The heap may be damaged and may not be safe to do anything else
There was a problem hiding this comment.
Think I should make it safe mode instead? It'd work more reliably because then it wouldn't try to use the heap at all.
|
I'll take a look at this again later this week. I'm hoping to have a MIDI demo tomorrow so I need to go heads down on it. |
The backtrace cannot be given because it relies on the validity of the qstr data structures on the heap which may have been corrupted. In fact, it still can crash hard when the bytecode itself is overwritten. To fix, we'd need a way to skip gathering the backtrace completely. This also increases the default stack size on M4s so it can accomodate the stack needed by ASF4s nvm API.
This creates a common safe mode mechanic that ports can share. As a result, the nRF52 now has safe mode support as well. The common safe mode adds a 700ms delay at startup where a reset during that window will cause a reset into safe mode. This window is designated by a yellow status pixel and flashing the single led three times. A couple NeoPixel fixes are included for the nRF52 as well. Fixes micropython#1034. Fixes micropython#990. Fixes micropython#615.
|
Ok, Travis is finally happy. Please take another look @dhalbert |
The backtrace cannot be given because it relies on the validity
of the qstr data structures on the heap which may have been
corrupted.
In fact, it still can crash hard when the bytecode itself is
overwritten. To fix, we'd need a way to skip gathering the
backtrace completely.
This also increases the default stack size on M4s so it can
accomodate the stack needed by ASF4s nvm API.