Added error message if uses of _sdata corrupts BBSRAM driver statics

3 files changed, 71 insertions, 69 deletions

diff --git a/src/drivers/boards/px4fmu-v2/px4fmu2_init.c b/src/drivers/boards/px4fmu-v2/px4fmu2_init.c
index 52ee00a7d..74bbfa4c6 100644
--- a/src/drivers/boards/px4fmu-v2/px4fmu2_init.c
+++ b/src/drivers/boards/px4fmu-v2/px4fmu2_init.c
@@ -498,10 +498,13 @@ inline static void copy_reverse(stack_word_t *dest, stack_word_t *src, int size)
 
 __EXPORT void board_crashdump(uint32_t currentsp, void *tcb, uint8_t *filename, int lineno)
 {
-  /* We need a chunk of ram to save the complete contest in.
+  /* We need a chunk of ram to save the complete context in.
    * Since we are going to reboot we will use &_sdata
-   *
+   * which is the lowest memory and the amount we will save
+   * _should be_ below any resources we need herein.
+   * Unfortunately this is hard to test. See dead below
    */
+
   fullcontext_s *pdump = (fullcontext_s*)&_sdata;
 
   (void)irqsave();
@@ -553,13 +556,14 @@ __EXPORT void board_crashdump(uint32_t currentsp, void *tcb, uint8_t *filename,
     {
       pdump->info.stacks.interrupt.sp = currentsp;
 
-      pdump->info.flags |= eRegs;
+      pdump->info.flags |= (eRegsPresent | eUserStackPresent | eIntStackPresent);
       memcpy(pdump->info.regs, (void*)current_regs, sizeof(pdump->info.regs));
       pdump->info.stacks.user.sp = pdump->info.regs[REG_R13];
-      currentsp = pdump->info.stacks.user.sp;
+
     } else {
 
         /* users context */
+        pdump->info.flags |= eUserStackPresent;
 
         pdump->info.stacks.user.sp = currentsp;
     }
@@ -581,56 +585,56 @@ __EXPORT void board_crashdump(uint32_t currentsp, void *tcb, uint8_t *filename,
   pdump->info.stacks.interrupt.top = (uint32_t)&g_intstackbase;
   pdump->info.stacks.interrupt.size  = (CONFIG_ARCH_INTERRUPTSTACK & ~3);
 
-  /* If the current stack pointer is within the interrupt stack then
-   * save the interrupt stack data centered about the interrupt stack pointer
+  /* If In interrupt Context save the interrupt stack data centered
+   * about the interrupt stack pointer
    */
 
-  if (pdump->info.stacks.interrupt.sp <= pdump->info.stacks.interrupt.top &&
-      pdump->info.stacks.interrupt.sp > pdump->info.stacks.interrupt.top - pdump->info.stacks.interrupt.size)
-    {
-      pdump->info.flags |= eIntStack;
+  if ((pdump->info.flags & eIntStackPresent) != 0) {
       stack_word_t * ps = (stack_word_t *) pdump->info.stacks.interrupt.sp;
       copy_reverse(pdump->istack, &ps[arraySize(pdump->istack)/2], arraySize(pdump->istack));
-   } else {
-       pdump->info.flags |= eInvalidIntStack;
-   }
+    }
+
+  /* Is it Invalid? */
+
+  if (!(pdump->info.stacks.interrupt.sp <= pdump->info.stacks.interrupt.top &&
+        pdump->info.stacks.interrupt.sp > pdump->info.stacks.interrupt.top - pdump->info.stacks.interrupt.size)) {
+        pdump->info.flags |= eInvalidIntStackPrt;
+  }
 
 #endif
 
-  /*  If the saved context of the interrupted process's stack pointer lies within the
-   * allocated user stack memory then save the user stack centered about the user sp
+  /* If In interrupt context or User save the user stack data centered
+   * about the user stack pointer
    */
-  if (currentsp <= pdump->info.stacks.user.top &&
-      currentsp  > pdump->info.stacks.user.top - pdump->info.stacks.user.size)
+  if ((pdump->info.flags & eUserStackPresent) != 0)
     {
-      pdump->info.flags |= eUserStack;
       stack_word_t * ps = (stack_word_t *) pdump->info.stacks.user.sp;
       copy_reverse(pdump->ustack, &ps[arraySize(pdump->ustack)/2], arraySize(pdump->ustack));
-    } else {
-        pdump->info.flags |= eInvalidUserStack;
     }
 
-  /* Oh boy we have a real hot mess on our hands so save above and below the
-   * current sp
-   */
+  /* Is it Invalid? */
 
-  if ((pdump->info.flags & eStackValid) == 0)
+  if (!(pdump->info.stacks.user.sp <= pdump->info.stacks.user.top &&
+      pdump->info.stacks.user.sp > pdump->info.stacks.user.top - pdump->info.stacks.user.size))
     {
-      pdump->info.flags |= (eStackUnknown | eStackValid);
-#if CONFIG_ARCH_INTERRUPTSTACK > 3
-      /* sp and above in istack */
-      stack_word_t * ps = (stack_word_t *) pdump->info.stacks.interrupt.sp;
-      copy_reverse(pdump->istack, &ps[arraySize(pdump->istack)], arraySize(pdump->istack));
-      /* below in ustack */
-      ps = (stack_word_t *) pdump->info.stacks.user.sp;
-      copy_reverse(pdump->ustack, &ps[arraySize(pdump->ustack)], arraySize(pdump->ustack));
-#else
-      /* save above and below in ustack */
-      copy_reverse(pdump->ustack, &ps[arraySize(pdump->ustack)/2], arraySize(pdump->ustack));
-#endif
+        pdump->info.flags |= eInvalidUserStackPtr;
     }
 
-  stm32_bbsram_savepanic(HARDFAULT_FILENO, (uint8_t*)pdump, sizeof(fullcontext_s));
+  int rv = stm32_bbsram_savepanic(HARDFAULT_FILENO, (uint8_t*)pdump, sizeof(fullcontext_s));
+
+  /* Test if memory got wiped because of using _sdata */
+
+  if (rv == -ENXIO) {
+      char * dead = "Memory wiped - dump not saved!";
+      while(*dead) {
+          up_lowputc(*dead++);
+      }
+  } else if (rv == -ENOSPC) {
+
+      /* hard fault again */
+
+      up_lowputc('!');
+  }
 
 
 #if defined(CONFIG_BOARD_RESET_ON_CRASH)
diff --git a/src/modules/systemlib/hardfault_log.h b/src/modules/systemlib/hardfault_log.h
index 6983a5737..c64381b42 100644
--- a/src/modules/systemlib/hardfault_log.h
+++ b/src/modules/systemlib/hardfault_log.h
@@ -232,13 +232,11 @@ typedef struct
 
 /* Flags to identify what is in the dump */
 typedef enum {
-  eRegs                 = 0x01,
-  eUserStack            = 0x02,
-  eIntStack             = 0x04,
-  eStackValid           = eUserStack | eIntStack,
-  eStackUnknown         = 0x08,
-  eInvalidUserStack     = 0x20,
-  eInvalidIntStack      = 0x40,
+  eRegsPresent          = 0x01,
+  eUserStackPresent     = 0x02,
+  eIntStackPresent      = 0x04,
+  eInvalidUserStackPtr  = 0x20,
+  eInvalidIntStackPrt   = 0x40,
 } fault_flags_t;
 
 typedef struct {
diff --git a/src/systemcmds/hardfault_log/hardfault_log.c b/src/systemcmds/hardfault_log/hardfault_log.c
index 959ea4a3a..3ac98ae95 100644
--- a/src/systemcmds/hardfault_log/hardfault_log.c
+++ b/src/systemcmds/hardfault_log/hardfault_log.c
@@ -196,7 +196,7 @@ static void identify(char *caller)
  ****************************************************************************/
 static int hardfault_get_desc(char *caller, struct bbsramd_s *desc, bool silent)
 {
-  int ret = -ENOENT;
+  int ret = ENOENT;
   int fd = open(HARDFAULT_PATH, O_RDONLY);
   if (fd < 0 ) {
       if (!silent) {
@@ -360,8 +360,8 @@ static int write_registers(uint32_t regs[], char *buffer, int max, int fd)
  ****************************************************************************/
 static int write_registers_info(int fdout, info_s *pi , char *buffer, int sz)
 {
-  int ret = -ENOENT;
-  if (pi->flags & eRegs) {
+  int ret = ENOENT;
+  if (pi->flags & eRegsPresent) {
       ret = -EIO;
       int n = snprintf(buffer, sz, " Processor registers: from 0x%08x\n", pi->current_regs);
       if (n == write(fdout, buffer, n)) {
@@ -377,9 +377,9 @@ static int write_registers_info(int fdout, info_s *pi , char *buffer, int sz)
 static int write_interrupt_stack_info(int fdout, info_s *pi, char *buffer,
                                       unsigned int sz)
 {
-  int ret = -ENOENT;
-  if (pi->flags & eIntStack) {
-      ret = write_stack_detail((pi->flags & eInvalidIntStack) != 0,
+  int ret = ENOENT;
+  if (pi->flags & eIntStackPresent) {
+      ret = write_stack_detail((pi->flags & eInvalidIntStackPrt) != 0,
                                 &pi->stacks.interrupt, "IRQ",
                                 buffer, sz, fdout);
   }
@@ -392,9 +392,9 @@ static int write_interrupt_stack_info(int fdout, info_s *pi, char *buffer,
 static int write_user_stack_info(int fdout, info_s *pi, char *buffer,
                                  unsigned int sz)
 {
-  int ret = -ENOENT;
-  if (pi->flags & eUserStack) {
-      ret = write_stack_detail((pi->flags & eInvalidUserStack) != 0,
+  int ret = ENOENT;
+  if (pi->flags & eUserStackPresent) {
+      ret = write_stack_detail((pi->flags & eInvalidUserStackPtr) != 0,
                                 &pi->stacks.user, "User", buffer, sz, fdout);
   }
   return ret;
@@ -484,10 +484,10 @@ static int write_dump_footer(char * caller, int fdout, struct timespec *ts,
 static int write_intterupt_stack(int fdin, int fdout, info_s *pi, char *buffer,
                                  unsigned int sz)
 {
-  int ret = -ENOENT;
-  if ((pi->flags & eIntStack) != 0) {
+  int ret = ENOENT;
+  if ((pi->flags & eIntStackPresent) != 0) {
     lseek(fdin, offsetof(fullcontext_s, istack), SEEK_SET);
-    ret = write_stack((pi->flags & eInvalidIntStack) != 0,
+    ret = write_stack((pi->flags & eInvalidIntStackPrt) != 0,
                 CONFIG_ISTACK_SIZE,
                 pi->stacks.interrupt.sp + CONFIG_ISTACK_SIZE/2,
                 pi->stacks.interrupt.top,
@@ -506,10 +506,10 @@ static int write_intterupt_stack(int fdin, int fdout, info_s *pi, char *buffer,
 static int write_user_stack(int fdin, int fdout, info_s *pi, char *buffer,
                             unsigned int sz)
 {
-  int ret = -ENOENT;
-  if ((pi->flags & eUserStack) != 0) {
+  int ret = ENOENT;
+  if ((pi->flags & eUserStackPresent) != 0) {
     lseek(fdin,offsetof(fullcontext_s, ustack), SEEK_SET);
-    ret = write_stack((pi->flags & eInvalidUserStack) != 0,
+    ret = write_stack((pi->flags & eInvalidUserStackPtr) != 0,
                 CONFIG_USTACK_SIZE,
                 pi->stacks.user.sp + CONFIG_USTACK_SIZE/2,
                 pi->stacks.user.top,
@@ -591,15 +591,15 @@ static int hardfault_dowrite(char * caller, int infd, int outfd,
                     switch(format) {
                     case HARDFAULT_DISPLAY_FORMAT:
                       ret = write_intterupt_stack(infd, outfd, pinfo, line, OUT_BUFFER_LEN);
-                      if (ret == OK || ret == -ENOENT) {
+                      if (ret >= OK) {
                           ret = write_user_stack(infd, outfd, pinfo, line, OUT_BUFFER_LEN);
-                          if (ret == OK || ret == -ENOENT) {
+                          if (ret >= OK) {
                               ret = write_dump_info(outfd, pinfo, desc, line, OUT_BUFFER_LEN);
-                              if (ret == OK || ret == -ENOENT) {
+                              if (ret >= OK) {
                                   ret = write_registers_info(outfd, pinfo, line, OUT_BUFFER_LEN);
-                                  if (ret == OK || ret == -ENOENT) {
+                                  if (ret >= OK) {
                                       ret = write_interrupt_stack_info(outfd, pinfo, line, OUT_BUFFER_LEN);
-                                      if (ret == OK || ret == -ENOENT) {
+                                      if (ret >= OK) {
                                           ret = write_user_stack_info(outfd, pinfo, line, OUT_BUFFER_LEN);
                                       }
                                   }
@@ -612,13 +612,13 @@ static int hardfault_dowrite(char * caller, int infd, int outfd,
                         ret = write_dump_info(outfd, pinfo, desc, line, OUT_BUFFER_LEN);
                         if (ret == OK) {
                             ret = write_registers_info(outfd, pinfo, line, OUT_BUFFER_LEN);
-                            if (ret == OK || ret == -ENOENT) {
+                            if (ret >= OK) {
                                 ret = write_interrupt_stack_info(outfd, pinfo, line, OUT_BUFFER_LEN);
-                                if (ret == OK || ret == -ENOENT) {
+                                if (ret >= OK) {
                                     ret = write_user_stack_info(outfd, pinfo, line, OUT_BUFFER_LEN);
-                                    if (ret == OK || ret == -ENOENT) {
+                                    if (ret >= OK) {
                                         ret = write_intterupt_stack(infd, outfd, pinfo, line, OUT_BUFFER_LEN);
-                                        if (ret == OK || ret == -ENOENT) {
+                                        if (ret >= OK) {
                                             ret = write_user_stack(infd, outfd, pinfo, line, OUT_BUFFER_LEN);
                                         }
                                     }
@@ -632,7 +632,7 @@ static int hardfault_dowrite(char * caller, int infd, int outfd,
                         break;
                     }
                   }
-                  if (ret == OK || ret == -ENOENT) {
+                  if (ret >= OK) {
                       ret = write_dump_footer(caller, outfd, &desc->lastwrite, line, OUT_BUFFER_LEN);
                   }
         }