Debugging improvements.

[barthess]

First preview commit pushed in /branches/dbg_improvements

I know it looks ugly but it works. It is mostly example of how to add
such functionality into your projects. I do not know how to integrate
this code elegantly in current Chibios concepts. Hope, most of this
features will be in v3.0.

[Giovanni]

Do you have a list of changes?

Giovanni

[barthess]

Do you have a list of changes?

List of desires for v3.0 or list of changes in this test branch?

[Giovanni]

List of changes in the branch so I can evaluate them without having to browse all the code.

Giovanni

[barthess]

1. Debug UART port driver. It was designed to use after port_disable() and not interfere with any other UART during normal system functionality. It was integrated in HAL without any hacks and troubles.
2. Redefined chDbgCheck() and chDbgAssert() macros. New ones able to say file name and line number caused panic. I do not know how to properly integrate them (include in halconf.h?).
3. stm32DbgPanic() function uses debug uart to send out “BsoD” message. Or call software breakpoint if program runs under debugger. I do not know how to integrate it too.
4. Hardfault handler helps to understand reason of fault. It was useful for me during debugging broken ART accelerator on rev.A of STM32F4x. It was integrated easy because hardfault handler is regular ISR defined as weak symbol.

All code except DBGUART placed in os/various/dbg_aid/stm32_dbg_aid.c(h)

[Giovanni]

I tried to implement chDbgAssert() and chDbgCheck() using __func__ or __FUNCTION__. Unfortunately those symbols are not known at preprocessor time and are implemented as "magic variables". This means that I cannot concatenate the function name to the line number, generating an unique identifier is simply not possible in C using the preprocessor alone.

This means that I have to revert the change in all the 3.0 branch, a week or work or so...

The other points will have to wait.

I also discovered that the current implementation of chDbgAssert() has a potential flaw, the expression is always evaluated even if the assertions are disabled, the result is casted to void so the dead code is usually removed. The problem happens when the expression contains references to volatile variables, in that case the dead code is not removed, this happens in several places in the HAL, where an I/O register is referenced by an assertion. Not a big deal but it could needlessly take some flash space. This should never happen in the kernel.

Giovanni

[russian]

Hope it's fine to return to this thread.

1) I think HardFaultVector deserves it's own handler (that's for stm32 platform) - not the generic unhandled_exception - somehow I got this exception while doing something wrong with my pointers so I'd rather know what kind of error I've got so that I at least know where to start

2) what is the idea behind the error codes like SV#4? Why not human-readable error messages like chDbgPanic("SV#4 misplaced chSysLock()"); ? Is this about reducing the OS footprint?

3) any chance for an official DBG_PANIC_HOOK ? I know we can get error codes via the debugger - put please note that some errors do not occur while in debugging mode or while not compiling with debugger information - smaller code different timing etc. I've found myself implementing a HD44780 panic callback: my error was reproducible only while NOT debugging, and HD44780 is a simple-enough protocol that I do not need to invoke the OS - I am controlling the pins directly.

[Giovanni]

1) The symbols are weak, you can redefine the handlers anywhere in your code.
2) Footprint, on AVR strings take RAM space.
3) It is chSysHalt(), port_halt() is weak.

Giovanni

[barthess]

I have some propositions about usability improvements of the current unhandled_exception handler.
My patch.

Code: Select all

Index: os/common/ports/ARMCMx/compilers/GCC/vectors.c
===================================================================
--- os/common/ports/ARMCMx/compilers/GCC/vectors.c   (revision 7255)
+++ os/common/ports/ARMCMx/compilers/GCC/vectors.c   (working copy)
@@ -68,16 +68,92 @@
 } vectors_t;
 
 /**
+ * @brief   Register values for postmortem debugging.
+ */
+volatile uint32_t postmortem_r0;
+volatile uint32_t postmortem_r1;
+volatile uint32_t postmortem_r2;
+volatile uint32_t postmortem_r3;
+volatile uint32_t postmortem_r12;
+volatile uint32_t postmortem_lr; /* Link register. */
+volatile uint32_t postmortem_pc; /* Program counter. */
+volatile uint32_t postmortem_psr;/* Program status register. */
+volatile uint32_t postmortem_CFSR;
+volatile uint32_t postmortem_HFSR;
+volatile uint32_t postmortem_DFSR;
+volatile uint32_t postmortem_AFSR;
+volatile uint32_t postmortem_BFAR;
+volatile uint32_t postmortem_MMAR;
+volatile uint32_t postmortem_SCB_SHCSR;
+
+/**
+ * @brief   Evaluates to TRUE if system runs under debugger control.
+ * @note    This bit resets only by power reset.
+ */
+#define is_under_debugger() (((CoreDebug)->DHCSR) & \
+                            CoreDebug_DHCSR_C_DEBUGEN_Msk)
+
+/**
+ *
+ */
+void prvGetRegistersFromStack(uint32_t *pulFaultStackAddress){
+
+  postmortem_r0  = pulFaultStackAddress[0];
+  postmortem_r1  = pulFaultStackAddress[1];
+  postmortem_r2  = pulFaultStackAddress[2];
+  postmortem_r3  = pulFaultStackAddress[3];
+  postmortem_r12 = pulFaultStackAddress[4];
+  postmortem_lr  = pulFaultStackAddress[5];
+  postmortem_pc  = pulFaultStackAddress[6];
+  postmortem_psr = pulFaultStackAddress[7];
+
+  /* Configurable Fault Status Register. Consists of MMSR, BFSR and UFSR */
+  postmortem_CFSR = (*((volatile uint32_t *)(0xE000ED28))) ;
+
+  /* Hard Fault Status Register */
+  postmortem_HFSR = (*((volatile uint32_t *)(0xE000ED2C))) ;
+
+  /* Debug Fault Status Register */
+  postmortem_DFSR = (*((volatile uint32_t *)(0xE000ED30))) ;
+
+  /* Auxiliary Fault Status Register */
+  postmortem_AFSR = (*((volatile uint32_t *)(0xE000ED3C))) ;
+
+  /* Read the Fault Address Registers. These may not contain valid values.
+     Check BFARVALID/MMARVALID to see if they are valid values
+     MemManage Fault Address Register */
+  postmortem_MMAR = (*((volatile uint32_t *)(0xE000ED34))) ;
+  /* Bus Fault Address Register */
+  postmortem_BFAR = (*((volatile uint32_t *)(0xE000ED38))) ;
+
+  postmortem_SCB_SHCSR = SCB->SHCSR;
+
+  if (is_under_debugger()) {
+    __asm("BKPT #0\n"); // Break into the debugger
+  }
+
+  /* harmless infinite loop */
+  while(1){;}
+}
+
+/**
  * @brief   Unhandled exceptions handler.
  * @details Any undefined exception vector points to this function by default.
- *          This function simply stops the system into an infinite loop.
- *
- * @notapi
+ *          This function calls a function named prvGetRegistersFromStack()
+ *          which stores register values into RAM and stops the system
+ *          into an infinite loop.
  */
 void _unhandled_exception(void) {
-
-  while (true)
-    ;
+  __asm volatile  (
+    " tst lr, #4                                                \n"
+    " ite eq                                                    \n"
+    " mrseq r0, msp                                             \n"
+    " mrsne r0, psp                                             \n"
+    " ldr r1, [r0, #24]                                         \n"
+    " ldr r2, handler2_address_const                            \n"
+    " bx r2                                                     \n"
+    " handler2_address_const: .word prvGetRegistersFromStack    \n"
+  );
 }
 
 #if !defined(__DOXYGEN__)



or just redefine current _unhandled_exception(void) as weak symbol allowing any body to create own handlers

[russian]

[quote="barthess"]My patch.[/code]
IAR does not like labels in assembly code, is there a version of this without labels?

Code: Select all

Error[Og005]: Unknown symbol in inline assembly: "handler2_address_const" F:\stuff\rusefi_sourceforge\firmware\hw_layer\stm32f4\mpu_util.cpp 173
Error[Og006]: Syntax error in inline assembly: "Error[54]: Expression can not be forward" F:\stuff\rusefi_sourceforge\firmware\hw_layer\stm32f4\mpu_util.cpp 173
Error[Og005]: Unknown symbol in inline assembly: "handler2_address_const" F:\stuff\rusefi_sourceforge\firmware\hw_layer\stm32f4\mpu_util.cpp 175
Error[Og005]: Unknown symbol in inline assembly: ".word" F:\stuff\rusefi_sourceforge\firmware\hw_layer\stm32f4\mpu_util.cpp 175
Error[Og006]: Syntax error in inline assembly: "Error[54]: Expression can not be forward" F:\stuff\rusefi_sourceforge\firmware\hw_layer\stm32f4\mpu_util.cpp 175