From e2f851aae244c0c233064192c79f46fe0744e58f Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Thu, 7 Nov 2013 10:53:12 -0800 Subject: [PATCH] Enable stack overflow checking on all context switches Changes somewhere in the recent past have caused I2C operations to consume more stack space. The current failure mode is that after some debug command or infrequent battery operation, the system fails. Clean up and enable stack overflow detection by default, and add a debug command (disabled by default) to verify overflow detection works. This adds several instructions to each context switch, but it's still fairly inexpensive, and represents only a few percent increase in the size of svc_handler(). That's better than silent failures. BUG=chrome-os-partner:23938 BRANCH=none TEST=Enable CONFIG_CMD_STACKOVERFLOW, then run the 'stackoverflow' command. This should cause a stack overflow to be detected in the CONSOLE task. Change-Id: I9303aee5bd9318f1d92838b399d15fb8f6a2bbf9 Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/176113 Reviewed-by: Bill Richardson --- core/cortex-m/panic.c | 2 +- core/cortex-m/task.c | 41 +++++++++++++++++++++++++++++++++++++++-- include/config.h | 7 ++++--- include/panic.h | 7 ++++++- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/core/cortex-m/panic.c b/core/cortex-m/panic.c index 310a9d5a53..95344bbb07 100644 --- a/core/cortex-m/panic.c +++ b/core/cortex-m/panic.c @@ -327,7 +327,7 @@ static void panic_show_process_stack(const struct panic_data *pdata) /** * Display a message and reboot */ -static void panic_reboot(void) +void panic_reboot(void) { panic_puts("\n\nRebooting...\n"); system_reset(0); diff --git a/core/cortex-m/task.c b/core/cortex-m/task.c index 8ffb807b48..687572eabe 100644 --- a/core/cortex-m/task.c +++ b/core/cortex-m/task.c @@ -231,8 +231,13 @@ void svc_handler(int desched, task_id_t resched) #endif current = current_task; -#ifdef CONFIG_OVERFLOW_DETECT - ASSERT(*current->stack == STACK_UNUSED_VALUE); + +#ifdef CONFIG_DEBUG_STACK_OVERFLOW + if (*current->stack != STACK_UNUSED_VALUE) { + panic_printf("\n\nStack overflow in %s task!\n", + task_names[current - tasks]); + panic_reboot(); + } #endif if (desched && !current->events) { @@ -562,6 +567,38 @@ DECLARE_CONSOLE_COMMAND(taskready, command_task_ready, "Print/set ready tasks", NULL); +#ifdef CONFIG_CMD_STACKOVERFLOW +static void stack_overflow_recurse(int n) +{ + ccprintf("+%d", n); + + /* + * Force task context switch, since that's where we do stack overflow + * checking. + */ + msleep(10); + + stack_overflow_recurse(n+1); + + /* + * Do work after the recursion, or else the compiler uses tail-chaining + * and we don't actually consume additional stack. + */ + ccprintf("-%d", n); +} + +static int command_stackoverflow(int argc, char **argv) +{ + ccprintf("Recursing 0,"); + stack_overflow_recurse(1); + return EC_SUCCESS; +} +DECLARE_CONSOLE_COMMAND(stackoverflow, command_stackoverflow, + NULL, + "Recurse until stack overflow", + NULL); +#endif /* CONFIG_CMD_STACKOVERFLOW */ + void task_pre_init(void) { uint32_t *stack_next = (uint32_t *)task_stacks; diff --git a/include/config.h b/include/config.h index f9e2a806a3..c6e151158e 100644 --- a/include/config.h +++ b/include/config.h @@ -215,6 +215,7 @@ #undef CONFIG_CMD_RTC_ALARM #undef CONFIG_CMD_SCRATCHPAD #undef CONFIG_CMD_SLEEP +#undef CONFIG_CMD_STACKOVERFLOW /*****************************************************************************/ @@ -296,6 +297,9 @@ */ #define CONFIG_DEBUG_EXCEPTIONS +/* Check for stack overflows on every context switch */ +#define CONFIG_DEBUG_STACK_OVERFLOW + /*****************************************************************************/ /* Support DMA transfers inside the EC */ @@ -502,9 +506,6 @@ /* Support one-wire interface */ #undef CONFIG_ONEWIRE -/* Check for stack overflows on every context switch */ -#undef CONFIG_OVERFLOW_DETECT - /* Support PECI interface to x86 processor */ #undef CONFIG_PECI diff --git a/include/panic.h b/include/panic.h index 256ce84e87..2647592448 100644 --- a/include/panic.h +++ b/include/panic.h @@ -82,12 +82,17 @@ void panic_assert_fail(const char *msg, const char *func, const char *fname, int linenum); /** - * Display a panic message and reset + * Display a custom panic message and reset * * @param msg Panic message */ void panic(const char *msg); +/** + * Display a default message and reset + */ +void panic_reboot(void); + /** * Enable/disable bus fault handler *