From 9175d20cebe902b8afb7d700b4b3a583f26d13c7 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 11 May 2017 15:33:16 -0700 Subject: [PATCH] arc: fix CONFIG_ARC_STACK_CHECKING - There's no clear need to disable frame pointers if this feature is used, remove this directive. - The 'top' and 'base' terms are reversed. The 'base' is the high address of the stack. The top is the lowest address, where we cannot push further down. Fixup member and offset names to correspond to how these terms are used in hardware documentation. - Use correct pointers for stack top location - Fatal exceptions now go through _NanoFatalErrorHandler to report the faulting ip and thread. Signed-off-by: Andrew Boie --- arch/arc/Kconfig | 1 + arch/arc/Makefile | 1 - arch/arc/core/fast_irq.S | 8 ++-- arch/arc/core/fatal.c | 5 ++- arch/arc/core/fault.c | 62 ++++++++------------------- arch/arc/core/offsets/offsets.c | 2 +- arch/arc/core/regular_irq.S | 8 ++-- arch/arc/core/swap.S | 8 ++-- arch/arc/core/thread.c | 2 +- arch/arc/include/kernel_arch_thread.h | 6 ++- arch/arc/include/offsets_short_arch.h | 6 ++- 11 files changed, 44 insertions(+), 65 deletions(-) diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig index 6367102e3e..44404b1a53 100644 --- a/arch/arc/Kconfig +++ b/arch/arc/Kconfig @@ -111,6 +111,7 @@ config FIRQ_STACK_SIZE config ARC_STACK_CHECKING bool "Enable Stack Checking" depends on CPU_ARCV2 + select THREAD_STACK_INFO default n help ARCV2 has a special feature allowing to check stack overflows. This diff --git a/arch/arc/Makefile b/arch/arc/Makefile index 1bd121b420..58ac46b0da 100644 --- a/arch/arc/Makefile +++ b/arch/arc/Makefile @@ -5,7 +5,6 @@ cflags-y += $(call cc-option,-ffunction-sections,) $(call cc-option,-fdata-secti # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63691 cflags-y += $(call cc-option,-fno-delete-null-pointer-checks) -cflags-$(CONFIG_ARC_STACK_CHECKING) = $(call cc-option,-fomit-frame-pointer) cflags-$(CONFIG_LTO) = $(call cc-option,-flto,) include $(srctree)/arch/$(ARCH)/soc/$(SOC_PATH)/Makefile diff --git a/arch/arc/core/fast_irq.S b/arch/arc/core/fast_irq.S index e5acb6f43f..5d1f1be28b 100644 --- a/arch/arc/core/fast_irq.S +++ b/arch/arc/core/fast_irq.S @@ -227,11 +227,11 @@ _firq_reschedule: st_s r2, [r1, _kernel_offset_to_current] #ifdef CONFIG_ARC_STACK_CHECKING - /* Use stack top and down registers from restored context */ - add r3, r2, _K_THREAD_NO_FLOAT_SIZEOF - sr r3, [_ARC_V2_KSTACK_TOP] - ld_s r3, [r2, _thread_offset_to_stack_top] + /* Use stack top and base registers from restored context */ + ld r3, [r2, _thread_offset_to_stack_base] sr r3, [_ARC_V2_KSTACK_BASE] + ld r3, [r2, _thread_offset_to_stack_top] + sr r3, [_ARC_V2_KSTACK_TOP] #endif /* * _load_callee_saved_regs expects incoming thread in r2. diff --git a/arch/arc/core/fatal.c b/arch/arc/core/fatal.c index 5204060a67..bc96153400 100644 --- a/arch/arc/core/fatal.c +++ b/arch/arc/core/fatal.c @@ -41,11 +41,14 @@ FUNC_NORETURN void _NanoFatalErrorHandler(unsigned int reason, const NANO_ESF *pEsf) { switch (reason) { + case _NANO_ERR_HW_EXCEPTION: + break; + case _NANO_ERR_INVALID_TASK_EXIT: printk("***** Invalid Exit Software Error! *****\n"); break; -#if defined(CONFIG_STACK_CANARIES) +#if defined(CONFIG_STACK_CANARIES) || defined(CONFIG_ARC_STACK_CHECKING) case _NANO_ERR_STACK_CHK_FAIL: printk("***** Stack Check Fail! *****\n"); break; diff --git a/arch/arc/core/fault.c b/arch/arc/core/fault.c index 82ccc5c3da..f76eeda19a 100644 --- a/arch/arc/core/fault.c +++ b/arch/arc/core/fault.c @@ -17,50 +17,7 @@ #include #include - -#ifdef CONFIG_PRINTK #include -#define PR_EXC(...) printk(__VA_ARGS__) -#else -#define PR_EXC(...) -#endif /* CONFIG_PRINTK */ - -#if (CONFIG_FAULT_DUMP > 0) -#define FAULT_DUMP(esf, fault) _FaultDump(esf, fault) -#else -#define FAULT_DUMP(esf, fault) \ - do { \ - (void) esf; \ - (void) fault; \ - } while ((0)) -#endif - -#if (CONFIG_FAULT_DUMP > 0) -/* - * @brief Dump information regarding fault (FAULT_DUMP > 0) - * - * Dump information regarding the fault when CONFIG_FAULT_DUMP is set to 1 - * (short form). - * - * @return N/A - */ -void _FaultDump(const NANO_ESF *esf, int fault) -{ - ARG_UNUSED(esf); - ARG_UNUSED(fault); - -#ifdef CONFIG_PRINTK - u32_t exc_addr = _arc_v2_aux_reg_read(_ARC_V2_EFA); - u32_t ecr = _arc_v2_aux_reg_read(_ARC_V2_ECR); - - PR_EXC("Exception vector: 0x%x, cause code: 0x%x, parameter 0x%x\n", - _ARC_V2_ECR_VECTOR(ecr), - _ARC_V2_ECR_CODE(ecr), - _ARC_V2_ECR_PARAMETER(ecr)); - PR_EXC("Address 0x%x\n", exc_addr); -#endif -} -#endif /* CONFIG_FAULT_DUMP */ /* * @brief Fault handler @@ -74,9 +31,24 @@ void _FaultDump(const NANO_ESF *esf, int fault) */ void _Fault(void) { + u32_t vector, code, parameter; + u32_t exc_addr = _arc_v2_aux_reg_read(_ARC_V2_EFA); u32_t ecr = _arc_v2_aux_reg_read(_ARC_V2_ECR); - FAULT_DUMP(&_default_esf, ecr); + vector = _ARC_V2_ECR_VECTOR(ecr); + code = _ARC_V2_ECR_CODE(ecr); + parameter = _ARC_V2_ECR_PARAMETER(ecr); - _SysFatalErrorHandler(_NANO_ERR_HW_EXCEPTION, &_default_esf); + printk("Exception vector: 0x%x, cause code: 0x%x, parameter 0x%x\n", + vector, code, parameter); + printk("Address 0x%x\n", exc_addr); +#ifdef CONFIG_ARC_STACK_CHECKING + /* Vector 6 = EV_ProV. Regardless of code, parameter 2 means stack + * check violation + */ + if (vector == 6 && parameter == 2) { + _NanoFatalErrorHandler(_NANO_ERR_STACK_CHK_FAIL, &_default_esf); + } +#endif + _NanoFatalErrorHandler(_NANO_ERR_HW_EXCEPTION, &_default_esf); } diff --git a/arch/arc/core/offsets/offsets.c b/arch/arc/core/offsets/offsets.c index 1a463e4ffa..09419c7fed 100644 --- a/arch/arc/core/offsets/offsets.c +++ b/arch/arc/core/offsets/offsets.c @@ -30,7 +30,7 @@ GEN_OFFSET_SYM(_thread_arch_t, intlock_key); GEN_OFFSET_SYM(_thread_arch_t, relinquish_cause); GEN_OFFSET_SYM(_thread_arch_t, return_value); #ifdef CONFIG_ARC_STACK_CHECKING -GEN_OFFSET_SYM(_thread_arch_t, stack_top); +GEN_OFFSET_SYM(_thread_arch_t, stack_base); #endif /* ARCv2-specific IRQ stack frame structure member offsets */ diff --git a/arch/arc/core/regular_irq.S b/arch/arc/core/regular_irq.S index 8f4a5d615f..d69f6b343f 100644 --- a/arch/arc/core/regular_irq.S +++ b/arch/arc/core/regular_irq.S @@ -158,11 +158,11 @@ _rirq_common_interrupt_swap: /* r2 contains pointer to new thread */ #ifdef CONFIG_ARC_STACK_CHECKING - /* Use stack top and down registers from restored context */ - add r3, r2, _K_THREAD_NO_FLOAT_SIZEOF - sr r3, [_ARC_V2_KSTACK_TOP] - ld_s r3, [r2, _thread_offset_to_stack_top] + /* Use stack top and base registers from restored context */ + ld r3, [r2, _thread_offset_to_stack_base] sr r3, [_ARC_V2_KSTACK_BASE] + ld r3, [r2, _thread_offset_to_stack_top] + sr r3, [_ARC_V2_KSTACK_TOP] #endif /* * _load_callee_saved_regs expects incoming thread in r2. diff --git a/arch/arc/core/swap.S b/arch/arc/core/swap.S index ca77540671..02747019c4 100644 --- a/arch/arc/core/swap.S +++ b/arch/arc/core/swap.S @@ -99,11 +99,11 @@ SECTION_FUNC(TEXT, __swap) /* entering here, r2 contains the new current thread */ #ifdef CONFIG_ARC_STACK_CHECKING - /* Use stack top and down registers from restored context */ - add r3, r2, _K_THREAD_NO_FLOAT_SIZEOF - sr r3, [_ARC_V2_KSTACK_TOP] - ld_s r3, [r2, _thread_offset_to_stack_top] + /* Use stack top and base registers from restored context */ + ld r3, [r2, _thread_offset_to_stack_base] sr r3, [_ARC_V2_KSTACK_BASE] + ld r3, [r2, _thread_offset_to_stack_top] + sr r3, [_ARC_V2_KSTACK_TOP] #endif /* XXX - can be moved to delay slot of _CAUSE_RIRQ ? */ st_s r2, [r1, _kernel_offset_to_current] diff --git a/arch/arc/core/thread.c b/arch/arc/core/thread.c index 1c50755d4b..5b25b79a03 100644 --- a/arch/arc/core/thread.c +++ b/arch/arc/core/thread.c @@ -84,7 +84,7 @@ void _new_thread(struct k_thread *thread, char *pStackMem, size_t stackSize, */ #ifdef CONFIG_ARC_STACK_CHECKING pInitCtx->status32 = _ARC_V2_STATUS32_SC | _ARC_V2_STATUS32_E(_ARC_V2_DEF_IRQ_LEVEL); - thread->arch.stack_top = (u32_t) stackEnd; + thread->arch.stack_base = (u32_t) stackEnd; #else pInitCtx->status32 = _ARC_V2_STATUS32_E(_ARC_V2_DEF_IRQ_LEVEL); #endif diff --git a/arch/arc/include/kernel_arch_thread.h b/arch/arc/include/kernel_arch_thread.h index a74c2a4e6d..f349cbceac 100644 --- a/arch/arc/include/kernel_arch_thread.h +++ b/arch/arc/include/kernel_arch_thread.h @@ -58,8 +58,10 @@ struct _thread_arch { unsigned int return_value; #ifdef CONFIG_ARC_STACK_CHECKING - /* top of stack for hardware stack checking */ - u32_t stack_top; + /* High address of stack region, stack grows downward from this + * location. Usesd for hardware stack checking + */ + u32_t stack_base; #endif }; diff --git a/arch/arc/include/offsets_short_arch.h b/arch/arc/include/offsets_short_arch.h index eccf45ece8..b8f78d8ccc 100644 --- a/arch/arc/include/offsets_short_arch.h +++ b/arch/arc/include/offsets_short_arch.h @@ -26,12 +26,14 @@ #define _thread_offset_to_return_value \ (___thread_t_arch_OFFSET + ___thread_arch_t_return_value_OFFSET) -#define _thread_offset_to_stack_top \ - (___thread_t_arch_OFFSET + ___thread_arch_t_stack_top_OFFSET) +#define _thread_offset_to_stack_base \ + (___thread_t_arch_OFFSET + ___thread_arch_t_stack_base_OFFSET) #define _thread_offset_to_sp \ (___thread_t_callee_saved_OFFSET + ___callee_saved_t_sp_OFFSET) +#define _thread_offset_to_stack_top \ + (___thread_t_stack_info_OFFSET + ___thread_stack_info_t_start_OFFSET) /* end - threads */