From 44c070fdcd09ddcf17c7a8f83948d7cbb382c94e Mon Sep 17 00:00:00 2001 From: Yong Cong Sin Date: Fri, 16 Aug 2024 17:08:23 +0800 Subject: [PATCH] subsys/profiling: fix extra frame in the traces Check if the frame is within the text address before saving into the buffer, this eliminates the extra, uninitialized frame at the end of the unwind. Signed-off-by: Yong Cong Sin Signed-off-by: Yong Cong Sin --- subsys/profiling/perf/backends/perf_riscv.c | 11 +++++++++++ subsys/profiling/perf/backends/perf_x86.c | 11 +++++++++++ subsys/profiling/perf/backends/perf_x86_64.c | 11 +++++++++++ 3 files changed, 33 insertions(+) diff --git a/subsys/profiling/perf/backends/perf_riscv.c b/subsys/profiling/perf/backends/perf_riscv.c index 43de474eb6f..3ef09944e57 100644 --- a/subsys/profiling/perf/backends/perf_riscv.c +++ b/subsys/profiling/perf/backends/perf_riscv.c @@ -12,6 +12,13 @@ static bool valid_stack(uintptr_t addr, k_tid_t current) addr < current->stack_info.start + current->stack_info.size; } +static inline bool in_text_region(uintptr_t addr) +{ + extern uintptr_t __text_region_start, __text_region_end; + + return (addr >= (uintptr_t)&__text_region_start) && (addr < (uintptr_t)&__text_region_end); +} + /* * This function use frame pointers to unwind stack and get trace of return addresses. * Return addresses are translated in corresponding function's names using .elf file. @@ -75,6 +82,10 @@ size_t arch_perf_current_stack_trace(uintptr_t *buf, size_t size) if (idx >= size) return 0; + if (!in_text_region((uintptr_t)fp[-1])) { + break; + } + buf[idx++] = (uintptr_t)fp[-1]; new_fp = (void **)fp[-2]; diff --git a/subsys/profiling/perf/backends/perf_x86.c b/subsys/profiling/perf/backends/perf_x86.c index 1f212317626..05f42079142 100644 --- a/subsys/profiling/perf/backends/perf_x86.c +++ b/subsys/profiling/perf/backends/perf_x86.c @@ -13,6 +13,13 @@ static bool valid_stack(uintptr_t addr, k_tid_t current) addr < current->stack_info.start + current->stack_info.size; } +static inline bool in_text_region(uintptr_t addr) +{ + extern uintptr_t __text_region_start, __text_region_end; + + return (addr >= (uintptr_t)&__text_region_start) && (addr < (uintptr_t)&__text_region_end); +} + /* interruption stack frame */ struct isf { uint32_t ebp; @@ -63,6 +70,10 @@ size_t arch_perf_current_stack_trace(uintptr_t *buf, size_t size) if (idx >= size) return 0; + if (!in_text_region((uintptr_t)fp[1])) { + break; + } + buf[idx++] = (uintptr_t)fp[1]; void **new_fp = (void **)fp[0]; diff --git a/subsys/profiling/perf/backends/perf_x86_64.c b/subsys/profiling/perf/backends/perf_x86_64.c index 754450100ab..d0386a030ba 100644 --- a/subsys/profiling/perf/backends/perf_x86_64.c +++ b/subsys/profiling/perf/backends/perf_x86_64.c @@ -13,6 +13,13 @@ static bool valid_stack(uintptr_t addr, k_tid_t current) addr < current->stack_info.start + current->stack_info.size; } +static inline bool in_text_region(uintptr_t addr) +{ + extern uintptr_t __text_region_start, __text_region_end; + + return (addr >= (uintptr_t)&__text_region_start) && (addr < (uintptr_t)&__text_region_end); +} + /* * This function use frame pointers to unwind stack and get trace of return addresses. * Return addresses are translated in corresponding function's names using .elf file. @@ -49,6 +56,10 @@ size_t arch_perf_current_stack_trace(uintptr_t *buf, size_t size) if (idx >= size) return 0; + if (!in_text_region((uintptr_t)fp[1])) { + break; + } + buf[idx++] = (uintptr_t)fp[1]; void **new_fp = (void **)fp[0];