fix: take into account MR comments

This commit is contained in:
Omar Chebib 2024-12-30 12:22:52 +08:00
parent ead2c8655e
commit d6cd339e46
12 changed files with 210 additions and 95 deletions

View File

@ -126,15 +126,15 @@ menu "ESP System Settings"
unwinding and generation of both .eh_frame and .eh_frame_hdr sections, resulting in a bigger binary
size (20% to 100% larger). The main purpose of this option is to be able to have a backtrace parsed
and printed by the program itself, regardless of the serial monitor used.
This option shall NOT be used for production.
This option is not recommended to be used for production.
config ESP_SYSTEM_USE_FRAME_POINTER
bool "Use CPU Frame Pointer register"
help
This configuration allows the compiler to allocate CPU register s0 as the frame pointer. The main usage
of the frame pointer is to be able to generate a backtrace from the panic handler on exception.
Enabling this option results in bigger code and slower code since all functions will have to populate
this register and won't be able to use it as a general-purpose register anymore.
Enabling this option results in bigger and slightly slower code since all functions will have
to populate this register and won't be able to use it as a general-purpose register anymore.
endchoice

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -9,6 +9,7 @@
#include "esp_private/panic_internal.h"
#include "esp_memory_utils.h"
#include "riscv/libunwind-riscv.h"
#include "esp_private/fp_unwind.h"
#define FP_MAX_CALLERS 64
#define RA_INDEX_IN_FP -1
@ -21,7 +22,7 @@
* @param pc Program counter of the backtrace step.
* @param sp Stack pointer of the backtrace step.
*/
void __attribute__((weak)) esp_eh_frame_generated_step(uint32_t pc, uint32_t sp)
void __attribute__((weak)) esp_fp_generated_step(uint32_t pc, uint32_t sp)
{
panic_print_str(" 0x");
panic_print_hex(pc);
@ -46,7 +47,7 @@ static inline bool esp_fp_ptr_is_data(void* ptr)
*
* @param frame[in] Frame pointer to start unrolling from.
* @param callers[out] Array of callers where 0 will store the most recent caller. Can be NULL.
* @param stacks[out] Array of callers' stacks where 0 will store the most recent caller. Can be NULL.
* @param stacks[out] Array of callers' stacks where 0 will store the most recent caller's stack. Can be NULL.
* @param depth[in] Number of maximum entries to fill in the callers array.
*
* @returns Number of entries filled in the array.
@ -65,7 +66,7 @@ uint32_t IRAM_ATTR esp_fp_get_callers(uint32_t frame, void** callers, void** sta
/**
* We continue filling the callers array until we meet a function in ROM (not compiled
* with frame pointer) or the pointer we retrieved is not in RAM (binary libraries
* not compiled with frame pointer configuration)
* not compiled with frame pointer configuration, which means what is stored in the register is not a valid pointer)
*/
while (depth > 0 && esp_fp_ptr_is_data(fp) && !esp_ptr_in_rom((void *) pc)) {
/* Dereference the RA register from the frame pointer and store it in PC */
@ -104,12 +105,12 @@ void esp_fp_print_backtrace(const void *frame_or)
void* stacks[FP_MAX_CALLERS];
panic_print_str("Backtrace:");
esp_eh_frame_generated_step(pc, sp);
esp_fp_generated_step(pc, sp);
uint32_t len = esp_fp_get_callers(fp, callers, stacks, FP_MAX_CALLERS);
for (int i = 0; i < len; i++) {
esp_eh_frame_generated_step((uint32_t) callers[i], (uint32_t) stacks[i]);
esp_fp_generated_step((uint32_t) callers[i], (uint32_t) stacks[i]);
}
panic_print_str("\r\n");

View File

@ -0,0 +1,27 @@
/*
* SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef FP_UNWIND_H
#define FP_UNWIND_H
#ifdef __cplusplus
extern "C" {
#endif
/**
* @brief Print backtrace for the given execution frame thanks to the frame pointers.
*
* @param frame_or Snapshot of the CPU registers when the program stopped its
* normal execution. This frame is usually generated on the
* stack when an exception or an interrupt occurs.
*/
void esp_fp_print_backtrace(const void *frame_or);
#ifdef __cplusplus
}
#endif
#endif // FP_UNWIND_H

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -28,10 +28,9 @@ extern void panic_print_registers(const void *frame, int core);
/* Targets based on a RISC-V CPU cannot perform backtracing that easily.
* We have two options here:
* - Perform backtracing at runtime.
* - Perform backtracing at runtime thanks to the configuration options
* CONFIG_ESP_SYSTEM_USE_EH_FRAME and CONFIG_ESP_SYSTEM_USE_FRAME_POINTER.
* - Let IDF monitor do the backtracing for us. Used during panic already.
* This could be configurable, choosing one or the other depending on
* CONFIG_ESP_SYSTEM_USE_EH_FRAME configuration option.
*
* In both cases, this takes time, and we might be in an ISR, we must
* exit this handler as fast as possible, then we will simply print
@ -64,6 +63,7 @@ esp_err_t IRAM_ATTR esp_backtrace_print(int depth)
panic_print_registers(&backtrace_frame, current_core);
esp_rom_printf("\r\n");
esp_rom_printf("Please enable CONFIG_ESP_SYSTEM_USE_FRAME_POINTER option to have a full backtrace.\r\n");
#endif // CONFIG_ESP_SYSTEM_USE_EH_FRAME
return ESP_OK;

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -24,6 +24,10 @@
#include "esp_private/cache_utils.h"
#endif
#if CONFIG_ESP_SYSTEM_USE_FRAME_POINTER
#include "esp_private/fp_unwind.h"
#endif
#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
@ -336,7 +340,6 @@ void panic_print_backtrace(const void *frame, int core)
esp_eh_frame_print_backtrace(frame);
}
#elif CONFIG_ESP_SYSTEM_USE_FRAME_POINTER
extern void esp_fp_print_backtrace(const void*);
esp_fp_print_backtrace(frame);
#else
panic_print_basic_backtrace(frame, core);

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -11,16 +11,16 @@
#include <stdlib.h>
#include "unity.h"
#include "test_utils.h"
#include "esp_rom_sys.h"
#include "esp_rom_uart.h"
#if __XTENSA__
#if CONFIG_IDF_TARGET_ARCH_XTENSA
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "xtensa_api.h" // Replace with interrupt allocator API (IDF-3891)
#include "esp_debug_helpers.h"
#include "esp_intr_alloc.h"
#include "esp_rom_sys.h"
#include "esp_rom_uart.h"
#include "hal/misc.h"
#define SW_ISR_LEVEL_1 7
@ -145,4 +145,55 @@ TEST_CASE("Test esp_backtrace_print_all_tasks()", "[esp_system]")
}
}
#endif // CONFIG_IDF_TARGET_ARCH_XTENSA
#if CONFIG_ESP_SYSTEM_USE_FRAME_POINTER
void my_putc(char c)
{
static bool first_exec = 1;
esp_rom_output_putc(c);
if (first_exec) {
first_exec = false;
*((int*) 1) = 0;
}
}
// TEST_CASE("Test backtrace detects corrupted frames", "[esp_system]")
TEST_CASE("Backtrace detects ROM functions", "[esp_system]")
{
esp_rom_install_channel_putc(1, my_putc);
esp_rom_printf("foo");
}
static void __attribute__((naked)) non_framed_function(void)
{
asm volatile(
"add sp, sp, -16\n"
/* Save anything on the top of the stack, not the frame pointer nor RA */
"sw zero, 12(sp)\n"
"sw s0, 8(sp)\n"
/* Set s0 to a constant that cannot be interpreted as an address */
"li s0, 0x42\n"
/* Fail to trigger an exception */
"sw s0, (s0)\n"
"ret\n"
::
);
}
/**
* Test that backtracing detects when the frame is it unwinding encounters a function (or routine)
* that was not compiles with frame pointer option. This does NOT guarantee that all the call stack
* will be valid (some data pointers may be interpreted as frames), but it guarantees that no
* exception will be triggered.
*/
TEST_CASE("Backtrace detects corrupted frames", "[esp_system]")
{
/* Add some prints to make sure the compiler doesn't optimize it with a tail call */
non_framed_function();
printf("ERROR: must not reach this point\n");
}
#endif

View File

@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
# SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
# SPDX-License-Identifier: CC0-1.0
import pytest
from pytest_embedded import Dut
@ -32,3 +32,27 @@ def test_stack_smash_protection(dut: Dut) -> None:
dut.write('"stack smashing protection"')
dut.expect_exact('Stack smashing protect failure!')
dut.expect_exact('Rebooting...')
@pytest.mark.generic
@pytest.mark.parametrize(
'config',
[
# Testing this feature on a single RISC-V target is enough
pytest.param('framepointer', marks=[pytest.mark.esp32c3]),
]
)
def test_frame_pointer_backtracing(dut: Dut) -> None:
dut.expect_exact('Press ENTER to see the list of tests')
dut.write('"Backtrace detects corrupted frames"')
dut.expect_exact('Guru Meditation Error')
# The backtrace should be composed of a single entry
dut.expect(r'Backtrace: 0x[0-9a-f]{8}:0x[0-9a-f]{8}\s*[\r]?\n')
dut.expect_exact('Rebooting...')
dut.expect_exact('Press ENTER to see the list of tests')
dut.write('"Backtrace detects ROM functions"')
dut.expect_exact('Guru Meditation Error')
# The backtrace should have two entries
dut.expect(r'Backtrace: 0x[0-9a-f]{8}:0x[0-9a-f]{8} 0x[0-9a-f]{8}:0x[0-9a-f]{8}\s*[\r]?\n')
dut.expect_exact('Rebooting...')

View File

@ -0,0 +1 @@
CONFIG_ESP_SYSTEM_USE_FRAME_POINTER=y

View File

@ -94,8 +94,6 @@ def test_heap_misc_options(dut: Dut) -> None:
@pytest.mark.generic
@pytest.mark.esp32
@pytest.mark.esp32c3
@pytest.mark.parametrize(
'config',
[

View File

@ -0,0 +1,3 @@
CONFIG_IDF_TARGET="esp32"
CONFIG_SPIRAM=y
CONFIG_HEAP_TRACING_STANDALONE=y

View File

@ -0,0 +1,3 @@
CONFIG_IDF_TARGET="esp32c3"
CONFIG_ESP_SYSTEM_USE_FRAME_POINTER=y
CONFIG_HEAP_TRACING_STANDALONE=y

View File

@ -269,79 +269,6 @@ The following code snippet demonstrates how application code would typically ini
The output from the heap trace has a similar format to the following example:
.. only:: CONFIG_IDF_TARGET_ARCH_XTENSA
.. code-block:: none
====== Heap Trace: 8 records (8 capacity) ======
6 bytes (@ 0x3fc9f620, Internal) allocated CPU 0 ccount 0x1a31ac84 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
9 bytes (@ 0x3fc9f630, Internal) allocated CPU 0 ccount 0x1a31b618 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
12 bytes (@ 0x3fc9f640, Internal) allocated CPU 0 ccount 0x1a31bfac caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
15 bytes (@ 0x3fc9f650, Internal) allocated CPU 0 ccount 0x1a31c940 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
18 bytes (@ 0x3fc9f664, Internal) allocated CPU 0 ccount 0x1a31d2d4 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
21 bytes (@ 0x3fc9f67c, Internal) allocated CPU 0 ccount 0x1a31dc68 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
24 bytes (@ 0x3fc9f698, Internal) allocated CPU 0 ccount 0x1a31e600 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
6 bytes (@ 0x3fc9f6b4, Internal) allocated CPU 0 ccount 0x1a320698 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
====== Heap Trace Summary ======
Mode: Heap Trace All
6 bytes alive in trace (1/8 allocations)
records: 8 (8 capacity, 8 high water mark)
total allocations: 9
total frees: 8
================================
.. only:: CONFIG_IDF_TARGET_ARCH_RISCV
.. code-block:: none
@ -363,6 +290,79 @@ The output from the heap trace has a similar format to the following example:
total frees: 8
================================
Or the following example, when the ``CONFIG_ESP_SYSTEM_USE_FRAME_POINTER`` option is enabled and the stack depth is configured properly:
.. code-block:: none
====== Heap Trace: 8 records (8 capacity) ======
6 bytes (@ 0x3fc9f620, Internal) allocated CPU 0 ccount 0x1a31ac84 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
9 bytes (@ 0x3fc9f630, Internal) allocated CPU 0 ccount 0x1a31b618 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
12 bytes (@ 0x3fc9f640, Internal) allocated CPU 0 ccount 0x1a31bfac caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
15 bytes (@ 0x3fc9f650, Internal) allocated CPU 0 ccount 0x1a31c940 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
18 bytes (@ 0x3fc9f664, Internal) allocated CPU 0 ccount 0x1a31d2d4 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
21 bytes (@ 0x3fc9f67c, Internal) allocated CPU 0 ccount 0x1a31dc68 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
24 bytes (@ 0x3fc9f698, Internal) allocated CPU 0 ccount 0x1a31e600 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
freed by 0x403839e4:0x42008096
0x403839e4: free at /path/to/idf/examples/components/newlib/heap.c:40
0x42008096: test_func_74 at /path/to/idf/examples/components/heap/test_apps/heap_tests/main/test_heap_trace.c:104 (discriminator 3)
6 bytes (@ 0x3fc9f6b4, Internal) allocated CPU 0 ccount 0x1a320698 caller 0x40376321:0x40376379
0x40376321: heap_caps_malloc at /path/to/idf/examples/components/heap/heap_caps.c:84
0x40376379: heap_caps_malloc_default at /path/to/idf/examples/components/heap/heap_caps.c:110
====== Heap Trace Summary ======
Mode: Heap Trace All
6 bytes alive in trace (1/8 allocations)
records: 8 (8 capacity, 8 high water mark)
total allocations: 9
total frees: 8
================================
.. note::
The above example output uses :doc:`IDF Monitor </api-guides/tools/idf-monitor>` to automatically decode PC addresses to their source files and line numbers.
@ -396,6 +396,10 @@ In ``HEAP_TRACE_ALL``:
The depth of the call stack recorded for each trace entry can be configured in the project configuration menu, under ``Heap Memory Debugging`` > ``Enable heap tracing`` > :ref:`CONFIG_HEAP_TRACING_STACK_DEPTH`. Up to 32 stack frames can be recorded for each allocation (the default is 2). Each additional stack frame increases the memory usage of each ``heap_trace_record_t`` record by eight bytes.
.. only:: CONFIG_IDF_TARGET_ARCH_RISCV
By default, the depth of the call stack recorded for each trace entry is 0, which means that only the direct caller of the memory allocation function can be retrieve. However, when the ``CONFIG_ESP_SYSTEM_USE_FRAME_POINTER`` option is enabled, this call stack depth can be configured in the project configuration menu, under ``Heap Memory Debugging`` > ``Enable heap tracing`` > :ref:`CONFIG_HEAP_TRACING_STACK_DEPTH`. Up to 32 stack frames can be recorded for each allocation (the default is 2). Each additional stack frame increases the memory usage of each ``heap_trace_record_t`` record by eight bytes.
Finally, the total number of the 'leaked' bytes (bytes allocated but not freed while the trace is running) is printed together with the total number of allocations it represents.
Using hashmap for increased performance