From 57442c38bd33634e343c2488663a9d9be59cc276 Mon Sep 17 00:00:00 2001 From: Marius Vikhammer Date: Fri, 19 Feb 2021 11:26:21 +0800 Subject: [PATCH] core: fix cases where riscv SP were not 16 byte aligned RISC-V stack pointer should always be 16 byte aligned, but for some cases where we were doing manual SP manipulation this was not always the case. --- components/freertos/port/riscv/portasm.S | 7 ++++--- components/newlib/test/test_newlib.c | 2 +- components/newlib/test/test_shared_stack_printf.c | 9 +++++++++ components/riscv/expression_with_stack_riscv.c | 3 +-- components/riscv/expression_with_stack_riscv_asm.S | 7 ++++--- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/components/freertos/port/riscv/portasm.S b/components/freertos/port/riscv/portasm.S index 33bfc4c044..20c094b7a6 100644 --- a/components/freertos/port/riscv/portasm.S +++ b/components/freertos/port/riscv/portasm.S @@ -88,12 +88,13 @@ isr_skip_decrement: lw t2, 0x0(t0) beq t2, zero, no_switch - /* preserve return address and schedule next task */ - addi sp,sp,-4 + /* preserve return address and schedule next task + stack pointer for riscv should always be 16 byte aligned */ + addi sp,sp,-16 sw ra, 0(sp) call vTaskSwitchContext lw ra, 0(sp) - addi sp, sp, 4 + addi sp, sp, 16 /* Clears the switch pending flag */ la t0, xPortSwitchFlag diff --git a/components/newlib/test/test_newlib.c b/components/newlib/test/test_newlib.c index adca8f9ed3..552c0939ef 100644 --- a/components/newlib/test/test_newlib.c +++ b/components/newlib/test/test_newlib.c @@ -37,7 +37,7 @@ TEST_CASE("test sprintf function", "[newlib]") char *res = NULL; asprintf(&res, "%d %011i %lu %p %x %c %.4f\n", 42, 2147483647, 2147483648UL, (void *) 0x40010000, 0x40020000, 'Q', 1.0f / 137.0f); TEST_ASSERT_NOT_NULL(res); - TEST_ASSERT_EQUAL_STRING(res, "42 02147483647 2147483648 0x40010000 40020000 Q 0.0073\n"); + TEST_ASSERT_EQUAL_STRING("42 02147483647 2147483648 0x40010000 40020000 Q 0.0073\n", res); free(res); } diff --git a/components/newlib/test/test_shared_stack_printf.c b/components/newlib/test/test_shared_stack_printf.c index 2c6368b3de..a2d94dfe3a 100644 --- a/components/newlib/test/test_shared_stack_printf.c +++ b/components/newlib/test/test_shared_stack_printf.c @@ -15,7 +15,16 @@ static StackType_t *shared_stack_sp = NULL; void external_stack_function(void) { printf("Executing this printf from external stack! sp=%p\n", get_sp()); + shared_stack_sp = (StackType_t *)get_sp(); + + char *res = NULL; + /* Test return value from asprintf, this could potentially help catch a misaligned + stack pointer error */ + asprintf(&res, "%d %011i %lu %p %x %c %.4f\n", 42, 2147483647, 2147483648UL, (void *) 0x40010000, 0x40020000, 'Q', 1.0f / 137.0f); + TEST_ASSERT_NOT_NULL(res); + TEST_ASSERT_EQUAL_STRING("42 02147483647 2147483648 0x40010000 40020000 Q 0.0073\n", res); + free(res); } void another_external_stack_function(void) diff --git a/components/riscv/expression_with_stack_riscv.c b/components/riscv/expression_with_stack_riscv.c index 2581d3d62b..07d22bf3aa 100644 --- a/components/riscv/expression_with_stack_riscv.c +++ b/components/riscv/expression_with_stack_riscv.c @@ -38,8 +38,7 @@ static StackType_t *esp_switch_stack_setup(StackType_t *stack, size_t stack_size //Align stack to a 16byte boundary, as required by CPU specific: top_of_stack = (StackType_t *)(((UBaseType_t)(top_of_stack - 16) & ~0xf)); - RvExcFrame *adjusted_top_of_stack = (RvExcFrame *) top_of_stack; - adjusted_top_of_stack--; + StackType_t *adjusted_top_of_stack = top_of_stack - RV_STK_FRMSZ; #if CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK vPortSetStackWatchpoint(stack); diff --git a/components/riscv/expression_with_stack_riscv_asm.S b/components/riscv/expression_with_stack_riscv_asm.S index 44ca865315..b8a56a59ff 100644 --- a/components/riscv/expression_with_stack_riscv_asm.S +++ b/components/riscv/expression_with_stack_riscv_asm.S @@ -24,8 +24,9 @@ esp_shared_stack_invoke_function: /* Set shared stack as new stack pointer */ mv sp, a1 - /* store the ra and previous stack pointer in a safe place */ - addi sp,sp,-4 + /* store the ra and previous stack pointer in a safe place + stack pointer for riscv should always be 16 byte aligned */ + addi sp,sp,-16 sw t0, 0(sp) sw t1, 4(sp) @@ -35,7 +36,7 @@ esp_shared_stack_invoke_function: /* gets the ra and stack pointer saved previously */ lw t0, 0(sp) lw t1, 4(sp) - addi sp, sp, 4 + addi sp, sp, 16 /* restore both ra and real stack pointer of current task */ mv ra, t1