This post is older than 2 years and might not be relevant anymore
More Info: Consider searching for newer posts

Bug? in Nordic example for finding fault address in HardFault_Handler

I think there is a bug in Nordic provided code in this thread about fetching the offending PC in a hardfault handler: devzone.nordicsemi.com/.../

The provided code is:

void HardFault_Handler(void)
{
    uint32_t *sp = (uint32_t *) __get_MSP(); // Get stack pointer
    uint32_t ia = sp[24/4]; // Get instruction address from stack

    printf("Hard Fault at address: 0x%08x\r\n", (unsigned int)ia);
    while(1)
        ;
}

AFAIK the compiler generates a preamble that adjusts the stack for local variables in the ISR, before the provided code accesses MSP. Here is the dissambled code:

        HardFault_Handler():
000016d8:   push    {r4, r7, lr}
000016da:   sub     sp, #12
000016dc:   add     r7, sp, #0
 177        __ASM volatile ("MRS %0, msp\n" : "=r" (result) );

So the provided code gets the PC Program Counter (address of the offending instruction) using the wrong offset (6 words) into the stack (should be 12 words?)

I could be wrong. But in my testing the provided code doesn't seem to return a valid address from the code section of flash. I am using GNU ARM gcc, and C++. Maybe other compilers generate different assembler or I am missing something else?

I also added: attribute ((interrupt ("IRQ"))) to the handler in the code provided. Also, I am also using the same code to find an address where brownout occurred, in the POWER_CLOCK_ISRHandler.

Parents
  • I tested modified code: uint32_t ia = sp[12]; and it seems to work. I suppose the original code with the naked attribute might work also? In the assembler, the first push saves the unsaved registers that are used in the ISR, and the next line allocates 12 bytes on the stack for the local vars (but why does it allocate 8 bytes for a pointer?) Above I said "words" but I should have said "uint32_t's", the PC of the fault is 12 uint32_t past the current SP. So the provided code is C and intrinsics, and is portable except for the "12" which might depend on your compiler and attributes you give the function and whether you are using the SD or an RTOS (which might require the PSP instead of the MSP?)

Reply
  • I tested modified code: uint32_t ia = sp[12]; and it seems to work. I suppose the original code with the naked attribute might work also? In the assembler, the first push saves the unsaved registers that are used in the ISR, and the next line allocates 12 bytes on the stack for the local vars (but why does it allocate 8 bytes for a pointer?) Above I said "words" but I should have said "uint32_t's", the PC of the fault is 12 uint32_t past the current SP. So the provided code is C and intrinsics, and is portable except for the "12" which might depend on your compiler and attributes you give the function and whether you are using the SD or an RTOS (which might require the PSP instead of the MSP?)

Children
No Data
Related