From d00b8e8a8e2979e44e6d7fa72b3a6cafd3736c6b Mon Sep 17 00:00:00 2001 From: David Sidrane Date: Thu, 5 Mar 2015 05:13:21 -1000 Subject: Added error message if uses of _sdata corrupts BBSRAM driver statics --- src/drivers/boards/px4fmu-v2/px4fmu2_init.c | 78 +++++++++++++++------------- src/modules/systemlib/hardfault_log.h | 12 ++--- src/systemcmds/hardfault_log/hardfault_log.c | 50 +++++++++--------- 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); } } -- cgit v1.2.3