-
Notifications
You must be signed in to change notification settings - Fork 3k
Make location meaningful in print_error_report #7609
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.
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
Conversation
`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, ARMmbed#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, ARMmbed#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.
@bremoran Updating the description to indicate this is a fix. |
@bremoran - This is good change, Thanks. The original intention was to capture the address of the mbed_error() caller but I think this probably happened as part of some refactoring afterwards. |
/morph build |
Build : SUCCESSBuild number : 2678 Triggering tests/morph test |
Test : SUCCESSBuild number : 2416 |
Exporter Build : SUCCESSBuild number : 2315 |
Make location meaningful in print_error_report
handle_error
callsMBED_CALLER_ADDR()
, but this is always a location from within platform/mbed_error.c. This is becausehandle_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: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:Since
handle_error()
will be inlined, the link register will be kept the same, soMBED_CALLER_ADDR()
will yield the expected result. However, there is no guarantee that the compiler will respect theinline
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.Description
Pull request type