Skip to content

Commit

Permalink
Make location meaningful in print_error_report
Browse files Browse the repository at this point in the history
`handle_error` calls `MBED_CALLER_ADDR()`, but this is always a location from within platform/mbed_error.c. This is because `handle_error` is declared static. This does not cause the function to be inlined however. Instead, it is called by each function within mbed_error.c. For example, mbed_error yields this code:

```
000625c8 <mbed_error>:
   625c8:       b510            push    {r4, lr}
   625ca:       460c            mov     r4, r1
   625cc:       4611            mov     r1, r2
   625ce:       461a            mov     r2, r3
   625d0:       9b02            ldr     r3, [sp, #8]
   625d2:       f7ff feff       bl      623d4 <handle_error>
   625d6:       b968            cbnz    r0, 625f4 <mbed_error+0x2c>
   625d8:       4620            mov     r0, r4
   625da:       f7ff ff67       bl      624ac <print_error_report.constprop.0>
   625de:       f7ff fea8       bl      62332 <core_util_is_isr_active>
   625e2:       b910            cbnz    r0, 625ea <mbed_error+0x22>
   625e4:       f7ff fe9f       bl      62326 <core_util_are_interrupts_enabled>
   625e8:       b908            cbnz    r0, 625ee <mbed_error+0x26>
   625ea:       bf30            wfi
   625ec:       e7fd            b.n     625ea <mbed_error+0x22>
   625ee:       2001            movs    r0, #1
   625f0:       f000 f948       bl      62884 <__wrap_exit>
   625f4:       4800            ldr     r0, [pc, #0]    ; (625f8 <mbed_error+0x30>)
   625f6:       bd10            pop     {r4, pc}
   625f8:       80ff010f        .word   0x80ff010f
```

Note that at `625d2` there is a bl to handle error. That replaces the LR, which means that ALL calls to mbed_error will report a location of 0x625d6 or 0x625d7 (user vs. supervisor). I do not expect that this was the intention of the code. The simplest fix is to change line 99:

```C
static inline mbed_error_status_t handle_error(mbed_error_status_t error_status, unsigned int error_value, const char *filename, int line_number)
```

Since `handle_error()` will be inlined, the link register will be kept the same, so `MBED_CALLER_ADDR()` will yield the expected result. However, there is no guarantee that the compiler will respect the `inline` keyword in all circumstances.

The result is that each function that wishes to report its caller must extract its caller. This code cannot be centralised.

I have modified `mbed_error.c` to report the caller of each error reporting function, rather than the error reporting function itself.
  • Loading branch information
bremoran authored and Cruz Monrreal II committed Jul 28, 2018
1 parent b4d8c24 commit 8cc69d9
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions platform/mbed_error.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ 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);
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
static void mbed_halt_system(void)
Expand All @@ -82,7 +82,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);
handle_error(MBED_ERROR_UNKNOWN, 0, NULL, 0, MBED_CALLER_ADDR());
ERROR_REPORT(&last_error_ctx, "Fatal Run-time error");
error_in_progress = 1;

Expand All @@ -96,7 +96,7 @@ WEAK void error(const char *format, ...)
}

//Set an error status with the error handling system
static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsigned int error_value, const char *filename, int line_number)
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)
{
mbed_error_ctx current_error_ctx;

Expand All @@ -123,7 +123,7 @@ static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsign
memset(&current_error_ctx, sizeof(mbed_error_ctx), 0);
//Capture error information
current_error_ctx.error_status = error_status;
current_error_ctx.error_address = (uint32_t)MBED_CALLER_ADDR();
current_error_ctx.error_address = (uint32_t)caller;
current_error_ctx.error_value = error_value;
#ifdef MBED_CONF_RTOS_PRESENT
//Capture thread info
Expand Down Expand Up @@ -193,14 +193,14 @@ int mbed_get_error_count(void)
//Sets a 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)
{
return handle_error(error_status, error_value, filename, line_number);
return handle_error(error_status, error_value, filename, line_number, MBED_CALLER_ADDR());
}

//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)
{
//set the error reported and then halt the system
if (MBED_SUCCESS != handle_error(error_status, error_value, filename, line_number)) {
if (MBED_SUCCESS != handle_error(error_status, error_value, filename, line_number, MBED_CALLER_ADDR())) {
return MBED_ERROR_FAILED_OPERATION;
}

Expand Down

0 comments on commit 8cc69d9

Please sign in to comment.