Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arch: riscv: implement frame-pointer based stack unwinding #69912

Merged
merged 3 commits into from
Apr 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions arch/riscv/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,24 @@ config RISCV_ALWAYS_SWITCH_THROUGH_ECALL
and most people should say n here to minimize context switching
overhead.

config RISCV_ENABLE_FRAME_POINTER
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to make this a generic arch kconfig (CONFIG_FRAME_POINTER), we now have two architectures doing this

bool
default y
depends on OVERRIDE_FRAME_POINTER_DEFAULT && !OMIT_FRAME_POINTER
help
Hidden option to simplify access to OVERRIDE_FRAME_POINTER_DEFAULT
and OMIT_FRAME_POINTER. It is automatically enabled when the frame
pointer unwinding is enabled.

config RISCV_EXCEPTION_STACK_TRACE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

bool
default y
depends on RISCV_ENABLE_FRAME_POINTER
depends on EXCEPTION_STACK_TRACE
imply THREAD_STACK_INFO
help
Internal config to enable runtime stack traces on fatal exceptions.

menu "RISCV Processor Options"

config INCLUDE_RESET_VECTOR
Expand Down
74 changes: 74 additions & 0 deletions arch/riscv/core/fatal.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,77 @@ static const struct z_exc_handle exceptions[] = {
#define NO_REG " "
#endif

#ifdef CONFIG_RISCV_EXCEPTION_STACK_TRACE
#define MAX_STACK_FRAMES 8

struct stackframe {
uintptr_t fp;
uintptr_t ra;
};

static bool in_stack_bound(uintptr_t addr)
{
#ifdef CONFIG_THREAD_STACK_INFO
uintptr_t start, end;

if (_current == NULL || arch_is_in_isr()) {
/* We were servicing an interrupt */
int cpu_id;

#ifdef CONFIG_SMP
cpu_id = arch_curr_cpu()->id;
#else
cpu_id = 0;
#endif

start = (uintptr_t)K_KERNEL_STACK_BUFFER(z_interrupt_stacks[cpu_id]);
end = start + CONFIG_ISR_STACK_SIZE;
#ifdef CONFIG_USERSPACE
/* TODO: handle user threads */
#endif
} else {
start = _current->stack_info.start;
end = Z_STACK_PTR_ALIGN(_current->stack_info.start + _current->stack_info.size);
}

return (addr >= start) && (addr < end);
#else
ARG_UNUSED(addr);
return true;
#endif /* CONFIG_THREAD_STACK_INFO */
}

static inline bool in_text_region(uintptr_t addr)
{
extern uintptr_t __text_region_start, __text_region_end;
ycsin marked this conversation as resolved.
Show resolved Hide resolved

return (addr >= (uintptr_t)&__text_region_start) && (addr < (uintptr_t)&__text_region_end);
}

static void unwind_stack(const z_arch_esf_t *esf)
{
uintptr_t fp = esf->s0;
uintptr_t ra;
struct stackframe *frame;

LOG_ERR("call trace:");

for (int i = 0; (i < MAX_STACK_FRAMES) && (fp != 0U) && in_stack_bound((uintptr_t)fp);) {
frame = (struct stackframe *)fp - 1;
ra = frame->ra;
if (in_text_region(ra)) {
LOG_ERR(" %2d: fp: " PR_REG " ra: " PR_REG, i, (uintptr_t)fp, ra);
/*
* Increment the iterator only if `ra` is within the text region to get the
* most out of it
*/
i++;
}
fp = frame->fp;
}
}
#endif /* CONFIG_RISCV_EXCEPTION_STACK_TRACE */

FUNC_NORETURN void z_riscv_fatal_error(unsigned int reason,
const z_arch_esf_t *esf)
{
Expand All @@ -54,6 +125,9 @@ FUNC_NORETURN void z_riscv_fatal_error(unsigned int reason,
LOG_ERR(" mepc: " PR_REG, esf->mepc);
LOG_ERR("mstatus: " PR_REG, esf->mstatus);
LOG_ERR("");
#ifdef CONFIG_RISCV_EXCEPTION_STACK_TRACE
unwind_stack(esf);
#endif /* CONFIG_RISCV_EXCEPTION_STACK_TRACE */
}
#endif /* CONFIG_EXCEPTION_DEBUG */
z_fatal_error(reason, esf);
Expand Down
4 changes: 4 additions & 0 deletions doc/releases/release-notes-3.7.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ Architectures

* ARM

* RISC-V

* Implemented frame-pointer based stack unwinding.

* Xtensa

Bluetooth
Expand Down
9 changes: 9 additions & 0 deletions tests/arch/common/stack_unwind/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright (c) 2024 Meta
# SPDX-License-Identifier: Apache-2.0

cmake_minimum_required(VERSION 3.20.0)
find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
project(stack_unwind_test)

FILE(GLOB app_sources src/*.c)
target_sources(app PRIVATE ${app_sources})
11 changes: 11 additions & 0 deletions tests/arch/common/stack_unwind/prj.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
CONFIG_TEST=y

CONFIG_LOG=y
CONFIG_LOG_BUFFER_SIZE=2048

CONFIG_EXCEPTION_STACK_TRACE=y
CONFIG_OVERRIDE_FRAME_POINTER_DEFAULT=y
CONFIG_OMIT_FRAME_POINTER=n
CONFIG_DEBUG=y
CONFIG_DEBUG_INFO=y
CONFIG_DEBUG_OPTIMIZATIONS=y
39 changes: 39 additions & 0 deletions tests/arch/common/stack_unwind/src/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright (c) 2024 Meta
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <stdio.h>
#include <stdbool.h>

#include <zephyr/kernel.h>

static void func1(int a);
static void func2(int a);

static void func2(int a)
{
printf("%d: %s\n", a, __func__);

if (a >= 5) {
k_oops();
}

func1(a + 1);
}

static void func1(int a)
{
printf("%d: %s\n", a, __func__);
func2(a + 1);
}

int main(void)
{
printf("Hello World! %s\n", CONFIG_BOARD);

func1(1);

return 0;
}
39 changes: 39 additions & 0 deletions tests/arch/common/stack_unwind/testcase.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
common:
harness: console
ignore_faults: true
ignore_qemu_crash: true
tags: kernel
tests:
arch.common.stack_unwind.riscv:
arch_allow: riscv
integration_platforms:
- qemu_riscv32
- qemu_riscv64
harness_config:
type: multi_line
regex:
- "E: call trace:"
- "E: 0: fp: \\w+ ra: \\w+"
- "E: 1: fp: \\w+ ra: \\w+"
arch.common.stack_unwind.x86:
arch_allow: x86
extra_configs:
- CONFIG_NO_OPTIMIZATIONS=y
integration_platforms:
- qemu_x86
- qemu_x86_64
harness_config:
type: multi_line
regex:
- "E: call trace:"
- "E: (E|R)IP: \\w+"
arch.common.stack_unwind.arm:
arch_allow:
- arm64
integration_platforms:
- qemu_cortex_a53
harness_config:
type: multi_line
regex:
- "E: backtrace 0: fp: \\w+ lr: \\w+"
- "E: backtrace 1: fp: \\w+ lr: \\w+"