Skip to content

More mbed_error refinements #8441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 23, 2018
Merged

More mbed_error refinements #8441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e20edbd
Add mbed_error_puts
kjbracey Oct 15, 2018
c2076b5
mbed_error: Simplify read of stack pointer
kjbracey Oct 16, 2018
25d0fba
mbed_error: remove redundant memset
kjbracey Oct 16, 2018
c3d0e2c
mbed_error: Correct full thread info print
kjbracey Oct 16, 2018
10fe322
mbed_error: Print thread names
kjbracey Oct 16, 2018
500e37f
mbed_error: Always print full filename if passed
kjbracey Oct 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions platform/mbed_board.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,30 +56,35 @@ void mbed_error_printf(const char *format, ...)

void mbed_error_vprintf(const char *format, va_list arg)
{
core_util_critical_section_enter();
char buffer[132];
int size = vsnprintf(buffer, sizeof buffer, format, arg);
if (size >= sizeof buffer) {
/* Output was truncated - indicate by overwriting last 4 bytes of buffer
* with ellipsis and newline.
* (Note that although vsnprintf always leaves a NUL terminator, we
* don't need a terminator and can use the entire buffer)
/* Output was truncated - indicate by overwriting tail of buffer
* with ellipsis, newline and null terminator.
*/
memcpy(&buffer[sizeof buffer - 4], "...\n", 4);
size = sizeof buffer;
static const char ellipsis[] = "...\n";
memcpy(&buffer[sizeof buffer - sizeof ellipsis], ellipsis, sizeof ellipsis);
}
if (size > 0) {
mbed_error_puts(buffer);
}
}

void mbed_error_puts(const char *str)
{
core_util_critical_section_enter();
#if MBED_CONF_PLATFORM_STDIO_CONVERT_NEWLINES || MBED_CONF_PLATFORM_STDIO_CONVERT_TTY_NEWLINES
char stdio_out_prev = '\0';
for (int i = 0; i < size; i++) {
if (buffer[i] == '\n' && stdio_out_prev != '\r') {
for (; *str != '\0'; str++) {
if (*str == '\n' && stdio_out_prev != '\r') {
const char cr = '\r';
write(STDERR_FILENO, &cr, 1);
}
write(STDERR_FILENO, &buffer[i], 1);
stdio_out_prev = buffer[i];
write(STDERR_FILENO, str, 1);
stdio_out_prev = *str;
}
#else
write(STDERR_FILENO, buffer, size);
write(STDERR_FILENO, str, strlen(str));
#endif
core_util_critical_section_exit();
}
Expand Down
87 changes: 36 additions & 51 deletions platform/mbed_error.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,34 +29,18 @@
#include <stdio.h>
#endif

//Helper macro to get the current SP
#define GET_CURRENT_SP(sp) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh. Why is this macro no longer needed?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SP pointer is fetched using the address of local variable, hence not needed

Copy link
Contributor Author

@kjbracey kjbracey Oct 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a very complicated way of getting the current stack pointer.

Although, on reflection, I now think the current stack pointer is not what you wanted anyway. It was being used as part of a "show current thread" line, where the current SP is being shown next to the stack size and limit for the current thread.

In a fault handler, both the complex and simple version will give you MSP, which is not going to be anywhere near the thread stack. You really wanted the PSP (or SP_usr for Cortex-A).

I'll probably adjust it again in a further patch.

(It's no big deal, as if it is a fault, you'll have the PSP visible in the preceding register dump.)

{ \
/*If in Handler mode we are always using MSP*/ \
if ( __get_IPSR() != 0U ) { \
sp = __get_MSP(); \
} else { \
/*Look into CONTROL.SPSEL value*/ \
if ((__get_CONTROL() & 2U) == 0U) { \
sp = __get_MSP();/*Read MSP*/ \
} else { \
sp = __get_PSP();/*Read PSP*/ \
} \
} \
}

#ifndef NDEBUG
#define ERROR_REPORT(ctx, error_msg) print_error_report(ctx, error_msg)
#define ERROR_REPORT(ctx, error_msg, error_filename, error_line) print_error_report(ctx, error_msg, error_filename, error_line)
static void print_error_report(const mbed_error_ctx *ctx, const char *, const char *error_filename, int error_line);
#else
#define ERROR_REPORT(ctx, error_msg) ((void) 0)
#define ERROR_REPORT(ctx, error_msg, error_filename, error_line) ((void) 0)
#endif

static uint8_t error_in_progress = 0;
static int error_count = 0;
static mbed_error_ctx first_error_ctx = {0};
static mbed_error_ctx last_error_ctx = {0};
static mbed_error_hook_t error_hook = NULL;
static void print_error_report(mbed_error_ctx *ctx, const char *);
static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsigned int error_value, const char *filename, int line_number, void *caller);

//Helper function to halt the system
Expand All @@ -83,7 +67,7 @@ WEAK void error(const char *format, ...)

//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");
ERROR_REPORT(&last_error_ctx, "Fatal Run-time error", NULL, 0);
error_in_progress = 1;

#ifndef NDEBUG
Expand Down Expand Up @@ -132,16 +116,12 @@ static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsign
current_error_ctx.thread_entry_address = (uint32_t)current_thread->thread_addr;
current_error_ctx.thread_stack_size = current_thread->stack_size;
current_error_ctx.thread_stack_mem = (uint32_t)current_thread->stack_mem;
#ifdef TARGET_CORTEX_M
GET_CURRENT_SP(current_error_ctx.thread_current_sp);
#endif //TARGET_CORTEX_M

current_error_ctx.thread_current_sp = (uint32_t)&current_error_ctx; // Address local variable to get a stack pointer
#endif //MBED_CONF_RTOS_PRESENT

#if MBED_CONF_PLATFORM_ERROR_FILENAME_CAPTURE_ENABLED
//Capture filename/linenumber if provided
//Index for tracking error_filename
memset(&current_error_ctx.error_filename, 0, MBED_CONF_PLATFORM_MAX_ERROR_FILENAME_LEN);
strncpy(current_error_ctx.error_filename, filename, MBED_CONF_PLATFORM_MAX_ERROR_FILENAME_LEN);
current_error_ctx.error_line_number = line_number;
#endif
Expand Down Expand Up @@ -205,7 +185,7 @@ WEAK mbed_error_status_t mbed_error(mbed_error_status_t error_status, const char
}

//On fatal errors print the error context/report
ERROR_REPORT(&last_error_ctx, error_msg);
ERROR_REPORT(&last_error_ctx, error_msg, filename, line_number);
mbed_halt_system();

return MBED_ERROR_FAILED_OPERATION;
Expand Down Expand Up @@ -290,15 +270,20 @@ mbed_error_status_t mbed_clear_all_errors(void)
return status;
}

static const char *name_or_unnamed(const char *name)
{
return name ? name : "<unnamed>";
}

#if MBED_CONF_PLATFORM_ERROR_ALL_THREADS_INFO && defined(MBED_CONF_RTOS_PRESENT)
/* Prints info of a thread(using osRtxThread_t struct)*/
static void print_thread(osRtxThread_t *thread)
static void print_thread(const osRtxThread_t *thread)
{
mbed_error_printf("\nState: 0x%08X Entry: 0x%08X Stack Size: 0x%08X Mem: 0x%08X SP: 0x%08X", thread->state, thread->thread_addr, thread->stack_size, (uint32_t)thread->stack_mem, thread->sp);
mbed_error_printf("\n%s State: 0x%X Entry: 0x%08X Stack Size: 0x%08X Mem: 0x%08X SP: 0x%08X", name_or_unnamed(thread->name), thread->state, thread->thread_addr, thread->stack_size, (uint32_t)thread->stack_mem, thread->sp);
}

/* Prints thread info from a list */
static void print_threads_info(osRtxThread_t *threads)
static void print_threads_info(const osRtxThread_t *threads)
{
while (threads != NULL) {
print_thread(threads);
Expand All @@ -308,7 +293,7 @@ static void print_threads_info(osRtxThread_t *threads)
#endif

#ifndef NDEBUG
static void print_error_report(mbed_error_ctx *ctx, const char *error_msg)
static void print_error_report(const mbed_error_ctx *ctx, const char *error_msg, const char *error_filename, int error_line)
{
uint32_t error_code = MBED_GET_ERROR_CODE(ctx->error_status);
uint32_t error_module = MBED_GET_ERROR_MODULE(ctx->error_status);
Expand Down Expand Up @@ -357,41 +342,41 @@ static void print_error_report(mbed_error_ctx *ctx, const char *error_msg)
//Nothing to do here, just print the error info down
break;
}
mbed_error_printf(error_msg);
mbed_error_puts(error_msg);
mbed_error_printf("\nLocation: 0x%X", ctx->error_address);

#if MBED_CONF_PLATFORM_ERROR_FILENAME_CAPTURE_ENABLED && !defined(NDEBUG)
if ((NULL != ctx->error_filename[0]) && (ctx->error_line_number != 0)) {
//for string, we must pass address of a ptr which has the address of the string
mbed_error_printf("\nFile:%s+%d", ctx->error_filename, ctx->error_line_number);
/* We print the filename passed in, not any filename in the context. This
* avoids the console print for mbed_error being limited to the presence
* and length of the filename storage. Note that although the MBED_ERROR
* macro compiles out filenames unless platform.error-filename-capture-enabled
* is turned on, MBED_ASSERT always passes filenames, and other direct
* users of mbed_error() may also choose to.
*/
if (error_filename) {
mbed_error_puts("\nFile: ");
mbed_error_puts(error_filename);
mbed_error_printf("+%d", error_line);
}
#endif

mbed_error_printf("\nError Value: 0x%X", ctx->error_value);
#ifdef TARGET_CORTEX_M
mbed_error_printf("\nCurrent Thread: Id: 0x%X Entry: 0x%X StackSize: 0x%X StackMem: 0x%X SP: 0x%X ",
#ifdef MBED_CONF_RTOS_PRESENT
mbed_error_printf("\nCurrent Thread: %s Id: 0x%X Entry: 0x%X StackSize: 0x%X StackMem: 0x%X SP: 0x%X ",
name_or_unnamed(((osRtxThread_t *)ctx->thread_id)->name),
ctx->thread_id, ctx->thread_entry_address, ctx->thread_stack_size, ctx->thread_stack_mem, ctx->thread_current_sp);
#else
//For Cortex-A targets we dont have support to capture the current SP
mbed_error_printf("\nCurrent Thread: Id: 0x%X Entry: 0x%X StackSize: 0x%X StackMem: 0x%X ",
ctx->thread_id, ctx->thread_entry_address, ctx->thread_stack_size, ctx->thread_stack_mem);
#endif //TARGET_CORTEX_M
#endif

#if MBED_CONF_PLATFORM_ERROR_ALL_THREADS_INFO && defined(MBED_CONF_RTOS_PRESENT)
mbed_error_printf("\nNext:");
print_thread(osRtxInfo.thread.run.next);

mbed_error_printf("\nReady:");
print_threads_info(osRtxInfo.thread.ready.thread_list);

mbed_error_printf("\nWait:");
osRtxThread_t *threads = (osRtxThread_t *)&osRtxInfo.thread.wait_list;
print_threads_info(threads);
print_threads_info(osRtxInfo.thread.wait_list);

mbed_error_printf("\nDelay:");
threads = (osRtxThread_t *)&osRtxInfo.thread.delay_list;
print_threads_info(threads);

mbed_error_printf("\nIdle:");
threads = (osRtxThread_t *)&osRtxInfo.thread.idle;
print_threads_info(threads);
print_threads_info(osRtxInfo.thread.delay_list);
#endif
mbed_error_printf(MBED_CONF_PLATFORM_ERROR_DECODE_HTTP_URL_STR, ctx->error_status);
mbed_error_printf("\n-- MbedOS Error Info --\n");
Expand Down
17 changes: 17 additions & 0 deletions platform/mbed_interface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ void mbed_die(void);
* handling a crash.
*
* @note Synchronization level: Interrupt safe
* @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.
*
* @param format C string that contains data stream to be printed.
* Code snippets below show valid format.
Expand All @@ -149,6 +152,20 @@ void mbed_error_printf(const char *format, ...);
*/
void mbed_error_vprintf(const char *format, va_list arg);

/** Print out an error message. This is typically called when
* handling a crash.
*
* Unlike mbed_error_printf, there is no limit to the maximum output
* length. Unlike standard puts, but like standard fputs, this does not
* append a '\n' character.
*
* @note Synchronization level: Interrupt safe
*
* @param str C string that contains data stream to be printed.
*
*/
void mbed_error_puts(const char *str);

/** @deprecated Renamed to mbed_error_vprintf to match functionality */
MBED_DEPRECATED_SINCE("mbed-os-5.11",
"Renamed to mbed_error_vprintf to match functionality.")
Expand Down