Adjust error paths locking and returning

Various fixes in preparation for making sure error calls do not return.

* Clear out handle_error's use of error_in_progress as a sort of spin
  lock; this is most likely to deadlock if ever activated, and conflicts
  with error's use of error_in_progress. Use a normal critical section lock.

* Make error use same mbed_halt_system helper as mbed_error.

* Make error's recursion check avoid print and proceed to halt, rather
  than returning.

* Make mbed_error use error_in_progress to avoid recursion in same way
  as error() does.

* Give mbed_halt_system its own recursion check in case of error in
  mbed_die - give it a simple fallback.

* Make the in_progress things properly atomic, just in case.
pull/8328/head
Kevin Bracey 2018-10-05 16:43:41 +03:00
parent c32984c3a8
commit 57748bd46e
1 changed files with 41 additions and 40 deletions

View File

@ -36,7 +36,8 @@ static void print_error_report(const mbed_error_ctx *ctx, const char *, const ch
#define ERROR_REPORT(ctx, error_msg, error_filename, error_line) ((void) 0) #define ERROR_REPORT(ctx, error_msg, error_filename, error_line) ((void) 0)
#endif #endif
static uint8_t error_in_progress = 0; static core_util_atomic_flag error_in_progress = CORE_UTIL_ATOMIC_FLAG_INIT;
static core_util_atomic_flag halt_in_progress = CORE_UTIL_ATOMIC_FLAG_INIT;
static int error_count = 0; static int error_count = 0;
static mbed_error_ctx first_error_ctx = {0}; static mbed_error_ctx first_error_ctx = {0};
static mbed_error_ctx last_error_ctx = {0}; static mbed_error_ctx last_error_ctx = {0};
@ -46,37 +47,41 @@ static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsign
//Helper function to halt the system //Helper function to halt the system
static void mbed_halt_system(void) static void mbed_halt_system(void)
{ {
//If not in ISR context exit, otherwise spin on WFI // Prevent recursion if halt is called again during halt attempt - try
if (core_util_is_isr_active() || !core_util_are_interrupts_enabled()) { // something simple instead.
if (core_util_atomic_flag_test_and_set(&halt_in_progress)) {
core_util_critical_section_enter();
__DSB();
for (;;) { for (;;) {
__WFI(); __WFE(); // Not WFI, as don't want to wake for pending interrupts
} }
} else {
//exit eventually calls mbed_die
exit(1);
} }
//If in ISR context, call mbed_die directly
if (core_util_is_isr_active() || !core_util_are_interrupts_enabled()) {
mbed_die();
}
// In normal context, try orderly exit(1), which eventually calls mbed_die
exit(1);
} }
WEAK void error(const char *format, ...) WEAK void error(const char *format, ...)
{ {
// Prevent recursion if error is called again during store+print attempt
// Prevent recursion if error is called again if (!core_util_atomic_flag_test_and_set(&error_in_progress)) {
if (error_in_progress) { handle_error(MBED_ERROR_UNKNOWN, 0, NULL, 0, MBED_CALLER_ADDR());
return; ERROR_REPORT(&last_error_ctx, "Fatal Run-time error", NULL, 0);
}
//Call handle_error/print_error_report permanently setting error_in_progress flag
handle_error(MBED_ERROR_UNKNOWN, 0, NULL, 0, MBED_CALLER_ADDR());
ERROR_REPORT(&last_error_ctx, "Fatal Run-time error", NULL, 0);
error_in_progress = 1;
#ifndef NDEBUG #ifndef NDEBUG
va_list arg; va_list arg;
va_start(arg, format); va_start(arg, format);
mbed_error_vprintf(format, arg); mbed_error_vprintf(format, arg);
va_end(arg); va_end(arg);
#endif #endif
exit(1); }
mbed_halt_system();
} }
//Set an error status with the error handling system //Set an error status with the error handling system
@ -91,18 +96,6 @@ static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsign
error_status = MBED_ERROR_INVALID_ARGUMENT; error_status = MBED_ERROR_INVALID_ARGUMENT;
} }
//Prevent corruption by holding out other callers
//and we also need this until we remove the "error" call completely
while (error_in_progress == 1);
//Use critsect here, as we don't want inadvertant modification of this global variable
core_util_critical_section_enter();
error_in_progress = 1;
core_util_critical_section_exit();
//Increment error count
error_count++;
//Clear the context capturing buffer //Clear the context capturing buffer
memset(&current_error_ctx, 0, sizeof(mbed_error_ctx)); memset(&current_error_ctx, 0, sizeof(mbed_error_ctx));
//Capture error information //Capture error information
@ -126,6 +119,12 @@ static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsign
current_error_ctx.error_line_number = line_number; current_error_ctx.error_line_number = line_number;
#endif #endif
//Prevent corruption by holding out other callers
core_util_critical_section_enter();
//Increment error count
error_count++;
//Capture the fist system error and store it //Capture the fist system error and store it
if (error_count == 1) { //first error if (error_count == 1) { //first error
memcpy(&first_error_ctx, &current_error_ctx, sizeof(mbed_error_ctx)); memcpy(&first_error_ctx, &current_error_ctx, sizeof(mbed_error_ctx));
@ -144,7 +143,7 @@ static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsign
error_hook(&last_error_ctx); error_hook(&last_error_ctx);
} }
error_in_progress = 0; core_util_critical_section_exit();
return MBED_SUCCESS; return MBED_SUCCESS;
} }
@ -179,13 +178,15 @@ mbed_error_status_t mbed_warning(mbed_error_status_t error_status, const char *e
//Sets a fatal error, this function is marked WEAK to be able to override this for some tests //Sets a fatal error, this function is marked WEAK to be able to override this for some tests
WEAK mbed_error_status_t mbed_error(mbed_error_status_t error_status, const char *error_msg, unsigned int error_value, const char *filename, int line_number) WEAK mbed_error_status_t mbed_error(mbed_error_status_t error_status, const char *error_msg, unsigned int error_value, const char *filename, int line_number)
{ {
//set the error reported and then halt the system // Prevent recursion if error is called again during store+print attempt
if (MBED_SUCCESS != handle_error(error_status, error_value, filename, line_number, MBED_CALLER_ADDR())) { if (!core_util_atomic_flag_test_and_set(&error_in_progress)) {
return MBED_ERROR_FAILED_OPERATION; //set the error reported
(void) handle_error(error_status, error_value, filename, line_number, MBED_CALLER_ADDR());
//On fatal errors print the error context/report
ERROR_REPORT(&last_error_ctx, error_msg, filename, line_number);
} }
//On fatal errors print the error context/report
ERROR_REPORT(&last_error_ctx, error_msg, filename, line_number);
mbed_halt_system(); mbed_halt_system();
return MBED_ERROR_FAILED_OPERATION; return MBED_ERROR_FAILED_OPERATION;