Merge pull request #10358 from kjbracey-arm/error_print_improvements

Error print improvements

Author: 0xc0170

drivers/UARTSerial.cpp

@@ -147,7 +147,6 @@ ssize_t UARTSerial::write_unbuffered(const char *buf_ptr, size_t length)
 
     for (size_t data_written = 0; data_written < length; data_written++) {
         SerialBase::_base_putc(*buf_ptr++);
-        data_written++;
     }
 
     return length;

platform/mbed_assert.c

@@ -23,7 +23,6 @@
 
 MBED_NORETURN void mbed_assert_internal(const char *expr, const char *file, int line)
 {
-    core_util_critical_section_enter();
     mbed_error(MBED_ERROR_ASSERTION_FAILED, expr, 0, file, line);
 }
 

platform/mbed_board.c

@@ -74,6 +74,28 @@ void mbed_error_vprintf(const char *format, va_list arg)
 
 void mbed_error_puts(const char *str)
 {
+    // Writing the string to the console in a critical section is
+    // potentially beneficial - for example in UARTSerial it
+    // forces the "unbuffered" mode that makes sure all characters
+    // go out now. If we made the call not in a critical section,
+    // it would go to the software buffer and we would be reliant
+    // on platform.stdio-flush-at-exit forcing a fsync before
+    // entering mbed_die().
+    //
+    // But this may be the very first write to the console, and hence
+    // require it to be initialized - doing this in a critical
+    // section could be problematic. So we prime it outside the
+    // critical section with a zero-length write - this forces
+    // the initialization.
+    //
+    // It's still possible that we were in a critical section
+    // or interrupt on entry anyway (eg if this is an error coming
+    // from inside RTX), so in other areas of the system we suppress
+    // things like mutex creation asserts and RTX traps while
+    // an error is in progress, so that console initialization
+    // may work.
+    write(STDERR_FILENO, str, 0);
+
     core_util_critical_section_enter();
 #if MBED_CONF_PLATFORM_STDIO_CONVERT_NEWLINES || MBED_CONF_PLATFORM_STDIO_CONVERT_TTY_NEWLINES
     char stdio_out_prev = '\0';

platform/mbed_error.c

@@ -45,7 +45,7 @@ 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)
 #endif
 
-static core_util_atomic_flag error_in_progress = CORE_UTIL_ATOMIC_FLAG_INIT;
+static bool error_in_progress;
 static core_util_atomic_flag halt_in_progress = CORE_UTIL_ATOMIC_FLAG_INIT;
 static int error_count = 0;
 static mbed_error_ctx first_error_ctx = {0};
@@ -115,7 +115,7 @@ static MBED_NORETURN void mbed_halt_system(void)
 WEAK MBED_NORETURN void error(const char *format, ...)
 {
     // Prevent recursion if error is called again during store+print attempt
-    if (!core_util_atomic_flag_test_and_set(&error_in_progress)) {
+    if (!core_util_atomic_exchange_bool(&error_in_progress, true)) {
         handle_error(MBED_ERROR_UNKNOWN, 0, NULL, 0, MBED_CALLER_ADDR());
         ERROR_REPORT(&last_error_ctx, "Fatal Run-time error", NULL, 0);
 
@@ -256,6 +256,12 @@ int mbed_get_error_count(void)
     return error_count;
 }
 
+//Reads the fatal error occurred" flag
+bool mbed_get_error_in_progress(void)
+{
+    return core_util_atomic_load_bool(&error_in_progress);
+}
+
 //Sets a non-fatal error
 mbed_error_status_t mbed_warning(mbed_error_status_t error_status, const char *error_msg, unsigned int error_value, const char *filename, int line_number)
 {
@@ -266,7 +272,7 @@ mbed_error_status_t mbed_warning(mbed_error_status_t error_status, const char *e
 WEAK MBED_NORETURN 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)
 {
     // Prevent recursion if error is called again during store+print attempt
-    if (!core_util_atomic_flag_test_and_set(&error_in_progress)) {
+    if (!core_util_atomic_exchange_bool(&error_in_progress, true)) {
         //set the error reported
         (void) handle_error(error_status, error_value, filename, line_number, MBED_CALLER_ADDR());
 

platform/mbed_error.h

@@ -23,6 +23,7 @@
 #ifndef MBED_ERROR_H
 #define MBED_ERROR_H
 
+#include <stdbool.h>
 #include "platform/mbed_retarget.h"
 #include "platform/mbed_toolchain.h"
 
@@ -1039,6 +1040,13 @@ mbed_error_status_t mbed_get_last_error(void);
  */
 int mbed_get_error_count(void);
 
+/**
+ * Returns whether we are processing a fatal mbed error.
+ * @return                  bool Whether a fatal error has occurred.
+ *
+ */
+bool mbed_get_error_in_progress(void);
+
 /**
  * Call this function to set a fatal system error and halt the system. This function will log the fatal error with the context info and prints the error report and halts the system.
  *

platform/mbed_interface.h

@@ -127,7 +127,9 @@ MBED_NORETURN void mbed_die(void);
 /** Print out an error message.  This is typically called when
  * handling a crash.
  *
- * @note Synchronization level: Interrupt safe
+ * @note Synchronization level: Interrupt safe, as long as the
+ *       FileHandle::write of the stderr device is. See mbed_error_puts
+ *       for more information.
  * @note This uses an internal 128-byte buffer to format the string,
  *       so the output may be truncated. If you need to write a potentially
  *       long string, use mbed_error_puts.
@@ -145,7 +147,9 @@ void mbed_error_printf(const char *format, ...) MBED_PRINTF(1, 2);
 /** Print out an error message.  Similar to mbed_error_printf
  * but uses a va_list.
  *
- * @note Synchronization level: Interrupt safe
+ * @note Synchronization level: Interrupt safe, as long as the
+ *       FileHandle::write of the stderr device is. See mbed_error_puts
+ *       for more information.
  *
  * @param format    C string that contains data stream to be printed.
  * @param arg       Variable arguments list
@@ -160,7 +164,13 @@ void mbed_error_vprintf(const char *format, va_list arg) MBED_PRINTF(1, 0);
  * length. Unlike standard puts, but like standard fputs, this does not
  * append a '\n' character.
  *
- * @note Synchronization level: Interrupt safe
+ * @note Synchronization level: Interrupt safe, as long as the
+ *       FileHandle::write of the stderr device is. The default
+ *       serial console is safe, either buffered or not. If the
+ *       console has not previously been initialized, an attempt
+ *       to use this from interrupt may during console initialization.
+ *       Special handling of `mbed_error` relaxes various system traps
+ *       to increase the chance of initialization working.
  *
  * @param str    C string that contains data stream to be printed.
  *

rtos/Mutex.cpp

@@ -47,7 +47,10 @@ void Mutex::constructor(const char *name)
     attr.cb_size = sizeof(_obj_mem);
     attr.attr_bits = osMutexRecursive | osMutexPrioInherit | osMutexRobust;
     _id = osMutexNew(&attr);
-    MBED_ASSERT(_id);
+    // To permit certain cases where a device may get constructed in
+    // by the attempt to print an error in a fatal shutdown, let a
+    // mutex construction error pass.
+    MBED_ASSERT(_id || mbed_get_error_in_progress());
 }
 
 osStatus Mutex::lock(void)
@@ -57,7 +60,7 @@ osStatus Mutex::lock(void)
         _count++;
     }
 
-    if (status != osOK) {
+    if (status != osOK && !mbed_get_error_in_progress()) {
         MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_KERNEL, MBED_ERROR_CODE_MUTEX_LOCK_FAILED), "Mutex lock failed", status);
     }
 
@@ -75,7 +78,7 @@ osStatus Mutex::lock(uint32_t millisec)
                     (status == osErrorResource && millisec == 0) ||
                     (status == osErrorTimeout && millisec != osWaitForever));
 
-    if (!success) {
+    if (!success && !mbed_get_error_in_progress()) {
         MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_KERNEL, MBED_ERROR_CODE_MUTEX_LOCK_FAILED), "Mutex lock failed", status);
     }
 
@@ -98,7 +101,7 @@ bool Mutex::trylock_for(uint32_t millisec)
                     (status == osErrorResource && millisec == 0) ||
                     (status == osErrorTimeout && millisec != osWaitForever));
 
-    if (!success) {
+    if (!success && !mbed_get_error_in_progress()) {
         MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_KERNEL, MBED_ERROR_CODE_MUTEX_LOCK_FAILED), "Mutex lock failed", status);
     }
 
@@ -121,15 +124,16 @@ bool Mutex::trylock_until(uint64_t millisec)
 
 osStatus Mutex::unlock()
 {
-    _count--;
-
     osStatus status = osMutexRelease(_id);
+    if (osOK == status) {
+        _count--;
+    }
 
-    if (status != osOK) {
+    if (status != osOK && !mbed_get_error_in_progress()) {
         MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_KERNEL, MBED_ERROR_CODE_MUTEX_UNLOCK_FAILED), "Mutex unlock failed", status);
     }
 
-    return osOK;
+    return status;
 }
 
 osThreadId Mutex::get_owner()

rtos/TARGET_CORTEX/mbed_rtx_handlers.c

@@ -107,29 +107,40 @@ static const char *error_msg(int32_t status)
     }
 }
 
+static void trap_rtx_error(unsigned int error_value, int32_t rtx_status, mbed_error_status_t error_status)
+{
+    // Attempts to get the console for the first time while printing an error
+    // may well cause a mutex error; in general let RTX calls fail during
+    // an error condition.
+    if (mbed_get_error_in_progress()) {
+        return;
+    }
+    MBED_ERROR1(error_status, error_msg(rtx_status), error_value);
+}
+
 void EvrRtxKernelError(int32_t status)
 {
-    MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_EVENT), error_msg(status), status);
+    trap_rtx_error(status, status, MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_EVENT));
 }
 
 void EvrRtxThreadError(osThreadId_t thread_id, int32_t status)
 {
-    MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_THREAD_EVENT), error_msg(status), thread_id);
+    trap_rtx_error((unsigned int) thread_id, status, MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_THREAD_EVENT));
 }
 
 void EvrRtxTimerError(osTimerId_t timer_id, int32_t status)
 {
-    MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_TIMER_EVENT), error_msg(status), timer_id);
+    trap_rtx_error((unsigned int) timer_id, status, MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_TIMER_EVENT));
 }
 
 void EvrRtxEventFlagsError(osEventFlagsId_t ef_id, int32_t status)
 {
-    MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_EVENT_FLAGS_EVENT), error_msg(status), ef_id);
+    trap_rtx_error((unsigned int) ef_id, status, MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_EVENT_FLAGS_EVENT));
 }
 
 void EvrRtxMutexError(osMutexId_t mutex_id, int32_t status)
 {
-    MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_MUTEX_EVENT), error_msg(status), mutex_id);
+    trap_rtx_error((unsigned int) mutex_id, status, MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_MUTEX_EVENT));
 }
 
 void EvrRtxSemaphoreError(osSemaphoreId_t semaphore_id, int32_t status)
@@ -139,17 +150,17 @@ void EvrRtxSemaphoreError(osSemaphoreId_t semaphore_id, int32_t status)
         return;
     }
 
-    MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_SEMAPHORE_EVENT), error_msg(status), semaphore_id);
+    trap_rtx_error((unsigned int) semaphore_id, status, MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_SEMAPHORE_EVENT));
 }
 
 void EvrRtxMemoryPoolError(osMemoryPoolId_t mp_id, int32_t status)
 {
-    MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_MEMORY_POOL_EVENT), error_msg(status), mp_id);
+    trap_rtx_error((unsigned int) mp_id, status, MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_MEMORY_POOL_EVENT));
 }
 
 void EvrRtxMessageQueueError(osMessageQueueId_t mq_id, int32_t status)
 {
-    MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_MESSAGE_QUEUE_EVENT), error_msg(status), mq_id);
+    trap_rtx_error((unsigned int) mq_id, status, MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_RTOS_MESSAGE_QUEUE_EVENT));
 }
 
 #endif