Add Hardfault check

[oganigl]

Add a new module to check if there was a hardfault in the last run

[StefanCostea]

I would investigate if it is possible to emit a full backtrace of what happened, with the corresponding function calls, you probably need to receive a stack pointer and unwind from there somehow. Or check if the MCU has a branch history table that we can access. Also storing those PCs can later be transalted into the corresponding mapping in source using gcc addr2line

[oganigl]

I would investigate if it is possible to emit a full backtrace of what happened, with the corresponding function calls, you probably need to receive a stack pointer and unwind from there somehow. Or check if the MCU has a branch history table that we can access. Also storing those PCs can later be transalted into the corresponding mapping in source using gcc addr2line

Yes I'm already doing that, check my pr in template-project, I read the stack pointer and I unwind it, read also cfsr register, an keep it in the flash.

[Cantonplas]

LGTM

[jorgesg82]

It would also be nice to toggle the three user LEDs the Nucleo has if we are compiling targeting it

[g0nz4I0]

I'll combine both reviews in one (here).

In general I like the idea, blinking leds, storage of log report, debugger detection, BUT,

There are several things I have doubts with:

1-From what I remember you can only write to flash by sectors, and also erasing whole sectors, I dont see a logic for that,

e.g. let's say you have some code in the flash sector of you LOG report, and you HardFault, I dont think that would work as expected.

2- I really like the idea of the custom handler and the original one in assembly, but I think there might be a better idea to implement the HardFault Handler directly in assembly, otherwise maybe the stack management corrupts the PC and registers

Also consider moving the asm implementation to the library, and only exposing the C handler so that the user can define its own, in this case it doesnt really matter because we maintain both but it's good practice.

Basically my concerns are:

Flash Writing, double check that that works, maybe consider using the SD?
You are not doing anything with the log you recover from the flash (if that worked)
Maybe you can do something with the debugger thingy and send it via UART or RTT to print the crash report

Instead of having to call HardFaultCheck on the template project maybe you can try to take advantage of static initializtion and try to read the RST reason from RCC?
Otherwise maybe consider placing in STLIB::board, I'm curious to hear why you chose this approach

Suggestion, move the asm handler into stlib and call into a CustomHandler from the user, but this is just a comment

IMO the most important things are in the sContextFrame, which are the PC and the LR, with this you can determine the two previous calls

[g0nz4I0]

I'll combine both reviews in one (here).

In general I like the idea, blinking leds, storage of log report, BUT,

There are several things I have doubts with:

1-From what I remember you can only write to flash by sectors, and also erasing whole sectors, I dont see a logic for that,

e.g. let's say you have some code in the flash sector of you LOG report, and you HardFault, I dont think that would work as expected.

2- I really like the idea of the custom handler and the original one in assembly, but I think there might be a better idea to implement the HardFault Handler directly in assembly, otherwise maybe the stack management corrupts the PC and registers

Also consider moving the asm implementation to the library, and only exposing the C handler so that the user can define its own, in this case it doesnt really matter because we maintain both but it's good practice.

Basically my concerns are:

Flash Writing, double check that that works, maybe consider using the SD? You are not doing anything with the log you recover from the flash (if that worked) Maybe you can do something with the debugger thingy and send it via UART or RTT to print the crash report

Instead of having to call HardFaultCheck on the template project maybe you can try to take advantage of static initializtion and try to read the RST reason from RCC? Otherwise maybe consider placing in STLIB::board, I'm curious to hear why you chose this approach

Suggestion, move the asm handler into stlib and call into a CustomHandler from the user, but this is just a comment

IMO the most important things are in the sContextFrame, which are the PC and the LR, with this you can determine the two previous calls

Parece q se lo he enchufado a una IA, pero juro q no, simplemente ahora escribo y a veces parece una IA

[g0nz4I0]

I would investigate if it is possible to emit a full backtrace of what happened, with the corresponding function calls, you probably need to receive a stack pointer and unwind from there somehow. Or check if the MCU has a branch history table that we can access. Also storing those PCs can later be transalted into the corresponding mapping in source using gcc addr2line

I think with the "default" configuration you get 2 call depth, PC + LR, but I think there can be something done with the frame pointer Frame Pointer Unwinding Linked List this increases the assembly, but might be useful.

[StefanCostea]

https://github.com/armink/CmBacktrace/blob/master/cm_backtrace/cm_backtrace.c
Here is an example of some code I saw to extract the full call trace, not only the last two calls, IMO it is very important to gather the full call trace that lead to the crash.

To summerize so you do not have to go through what they do is:

sp = read_sp_register();
while(sp-- < stack_start_adrr){ //can get from linker symbol
     mem_val = *sp;
     if mem_val > text_section_start_addr && mem_val < text_section_end_addr){ // this means we now this is a PC value
          instruction_val = *(uint32_t*)mem_val;
          bool is_branch = is_bl_blx(instruction_val); //check if the instruction has the BL BLX format with a mask
          if(is_branch){
                call_trace_buffer.push(mem_val);
          }
     }

}

Basically loop through the whole stack looking for return addresses. You will still have to read the registers for the first return address though as that one has not been pushed to stack

[StefanCostea]

I would absolutely not enable frame pointer, it would hurt performance

[FoniksFox]

I'll combine both reviews in one (here).

In general I like the idea, blinking leds, storage of log report, debugger detection, BUT,

There are several things I have doubts with:

1-From what I remember you can only write to flash by sectors, and also erasing whole sectors, I dont see a logic for that,

e.g. let's say you have some code in the flash sector of you LOG report, and you HardFault, I dont think that would work as expected.

2- I really like the idea of the custom handler and the original one in assembly, but I think there might be a better idea to implement the HardFault Handler directly in assembly, otherwise maybe the stack management corrupts the PC and registers

Also consider moving the asm implementation to the library, and only exposing the C handler so that the user can define its own, in this case it doesnt really matter because we maintain both but it's good practice.

Basically my concerns are:

Flash Writing, double check that that works, maybe consider using the SD? You are not doing anything with the log you recover from the flash (if that worked) Maybe you can do something with the debugger thingy and send it via UART or RTT to print the crash report

Instead of having to call HardFaultCheck on the template project maybe you can try to take advantage of static initializtion and try to read the RST reason from RCC? Otherwise maybe consider placing in STLIB::board, I'm curious to hear why you chose this approach

Suggestion, move the asm handler into stlib and call into a CustomHandler from the user, but this is just a comment

IMO the most important things are in the sContextFrame, which are the PC and the LR, with this you can determine the two previous calls

About using the SD to log the info, we considered it, but deemed it to be too complex and unreliable. The SD card needs a full-blown initialization process that could fail in the process (say, the sd card got loose) and is reliant on many other things working.
So, it could be implemented, with direct control of the SDMMC peripheral and LL calls, but may not be the best of ideas, considering it is an extrenal component and that it needs many calls just to initialize the card...
That's just my opinion though.

[FoniksFox]

Have you tested this with the new mpu configurations? Just to make sure that the mpu configuration doesn't throw us a memfault when we're already in a hardfault...

[victor-Lopez25]

I like leaving it in a HardFault directory instead of benchmarking