From 0771bd1711508f56e544c6d2f7c95c0b082d0cb2 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Sun, 7 Feb 2021 15:03:51 +0800 Subject: [PATCH] espsystem: Rearchitecture and fix eh_frame_parser bugs eh_frame_parser is architecture independent, thus the files have been rearchitectured. Some bugs have been fixed in the test. A README file has also been added to eh_frame_parser host test directory. eh_frame_parser is now able to detect empty gaps in .eh_frame_hdr table (missing DWARF information). Fix a bug occuring when parsing backtraces originated from abort(). Fix build missing dependencies issue. --- .gitlab/ci/host-test.yml | 7 ++ CMakeLists.txt | 1 + components/esp32c3/ld/esp32c3.ld | 4 +- components/esp32c3/ld/esp32c3.project.ld.in | 24 +++-- components/esp32h2/ld/esp32h2.ld | 5 + components/esp32h2/ld/esp32h2.project.ld.in | 25 ++++- components/esp_hw_support/CMakeLists.txt | 6 +- components/esp_system/CMakeLists.txt | 11 +- components/esp_system/Kconfig | 10 +- .../{port/arch/riscv => }/eh_frame_parser.c | 101 +++++++++++++----- .../port => include}/eh_frame_parser.h | 14 ++- components/esp_system/port/CMakeLists.txt | 8 +- .../esp_system/port/arch/riscv/panic_arch.c | 24 +++-- .../port/include/port/eh_frame_parser.h | 27 ----- .../{port => riscv}/eh_frame_parser_impl.h | 0 .../port/soc/esp32c3/CMakeLists.txt | 1 - .../riscv => }/test_eh_frame_parser/Makefile | 2 +- .../esp_system/test_eh_frame_parser/README.md | 42 ++++++++ .../eh_frame_parser_impl.h | 0 .../esp_private/panic_internal.h | 0 .../riscv => }/test_eh_frame_parser/linker.ld | 6 +- .../riscv => }/test_eh_frame_parser/main.c | 48 ++++++--- components/hal/CMakeLists.txt | 5 +- docs/en/api-guides/fatal-errors.rst | 33 +++++- 24 files changed, 293 insertions(+), 111 deletions(-) rename components/esp_system/{port/arch/riscv => }/eh_frame_parser.c (92%) rename components/esp_system/{port/arch/riscv/test_eh_frame_parser/port => include}/eh_frame_parser.h (68%) delete mode 100644 components/esp_system/port/include/port/eh_frame_parser.h rename components/esp_system/port/include/{port => riscv}/eh_frame_parser_impl.h (100%) rename components/esp_system/{port/arch/riscv => }/test_eh_frame_parser/Makefile (90%) create mode 100644 components/esp_system/test_eh_frame_parser/README.md rename components/esp_system/{port/arch/riscv/test_eh_frame_parser/port => test_eh_frame_parser}/eh_frame_parser_impl.h (100%) rename components/esp_system/{port/arch/riscv => }/test_eh_frame_parser/esp_private/panic_internal.h (100%) rename components/esp_system/{port/arch/riscv => }/test_eh_frame_parser/linker.ld (85%) rename components/esp_system/{port/arch/riscv => }/test_eh_frame_parser/main.c (86%) diff --git a/.gitlab/ci/host-test.yml b/.gitlab/ci/host-test.yml index ae7535d659..bb1f061ff9 100644 --- a/.gitlab/ci/host-test.yml +++ b/.gitlab/ci/host-test.yml @@ -355,3 +355,10 @@ test_log: - cd ${IDF_PATH}/components/log/host_test/log_test - idf.py build - build/test_log_host.elf + +test_eh_frame_parser: + extends: .host_test_template + script: + - cd ${IDF_PATH}/components/esp_system/test_eh_frame_parser + - make + - ./eh_frame_test diff --git a/CMakeLists.txt b/CMakeLists.txt index f98b610dc0..34e043dd41 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15,6 +15,7 @@ unset(link_options) # Add the following build specifications here, since these seem to be dependent # on config values on the root Kconfig. + if(NOT BOOTLOADER_BUILD) if(CONFIG_COMPILER_OPTIMIZATION_SIZE) diff --git a/components/esp32c3/ld/esp32c3.ld b/components/esp32c3/ld/esp32c3.ld index 42a49e24fc..66bf5d9ac4 100644 --- a/components/esp32c3/ld/esp32c3.ld +++ b/components/esp32c3/ld/esp32c3.ld @@ -118,6 +118,6 @@ REGION_ALIAS("rtc_data_location", rtc_iram_seg ); #endif #if CONFIG_ESP_SYSTEM_USE_EH_FRAME - ASSERT ((_eh_frame_end > _eh_frame), "Error: eh_frame size is null!"); - ASSERT ((_eh_frame_hdr_end > _eh_frame_hdr), "Error: eh_frame_hdr size is null!"); + ASSERT ((__eh_frame_end > __eh_frame), "Error: eh_frame size is null!"); + ASSERT ((__eh_frame_hdr_end > __eh_frame_hdr), "Error: eh_frame_hdr size is null!"); #endif diff --git a/components/esp32c3/ld/esp32c3.project.ld.in b/components/esp32c3/ld/esp32c3.project.ld.in index af50859ca0..c368cebe26 100644 --- a/components/esp32c3/ld/esp32c3.project.ld.in +++ b/components/esp32c3/ld/esp32c3.project.ld.in @@ -339,22 +339,28 @@ SECTIONS *(.srodata.*) _thread_local_end = ABSOLUTE(.); _rodata_reserved_end = ABSOLUTE(.); - . = ALIGN(4); + . = ALIGN(ALIGNOF(.eh_frame)); } > default_rodata_seg - .eh_frame : + /* Keep this section shall be at least aligned on 4 */ + .eh_frame : ALIGN(8) { - _eh_frame = ABSOLUTE(.); + __eh_frame = ABSOLUTE(.); KEEP (*(.eh_frame)) - _eh_frame_end = ABSOLUTE(.); - } > drom0_0_seg + __eh_frame_end = ABSOLUTE(.); + /* Guarantee that this section and the next one will be merged by making + * them adjacent. */ + . = ALIGN(ALIGNOF(.eh_frame_hdr)); + } > default_rodata_seg - .eh_frame_hdr : + /* To avoid any exception in C++ exception frame unwinding code, this section + * shall be aligned on 8. */ + .eh_frame_hdr : ALIGN(8) { - _eh_frame_hdr = ABSOLUTE(.); + __eh_frame_hdr = ABSOLUTE(.); KEEP (*(.eh_frame_hdr)) - _eh_frame_hdr_end = ABSOLUTE(.); - } > drom0_0_seg + __eh_frame_hdr_end = ABSOLUTE(.); + } > default_rodata_seg .flash.rodata_noload (NOLOAD) : { diff --git a/components/esp32h2/ld/esp32h2.ld b/components/esp32h2/ld/esp32h2.ld index f89de9d4a9..9672a4783b 100644 --- a/components/esp32h2/ld/esp32h2.ld +++ b/components/esp32h2/ld/esp32h2.ld @@ -116,3 +116,8 @@ REGION_ALIAS("rtc_data_location", rtc_iram_seg ); ASSERT(_flash_rodata_dummy_start == ORIGIN(default_rodata_seg), ".flash_rodata_dummy section must be placed at the beginning of the rodata segment.") #endif + +#if CONFIG_ESP_SYSTEM_USE_EH_FRAME + ASSERT ((__eh_frame_end > __eh_frame), "Error: eh_frame size is null!"); + ASSERT ((__eh_frame_hdr_end > __eh_frame_hdr), "Error: eh_frame_hdr size is null!"); +#endif diff --git a/components/esp32h2/ld/esp32h2.project.ld.in b/components/esp32h2/ld/esp32h2.project.ld.in index 459c8cb6f7..65eb17b45c 100644 --- a/components/esp32h2/ld/esp32h2.project.ld.in +++ b/components/esp32h2/ld/esp32h2.project.ld.in @@ -289,9 +289,6 @@ SECTIONS *(.gcc_except_table .gcc_except_table.*) *(.gnu.linkonce.e.*) *(.gnu.version_r) - . = (. + 3) & ~ 3; - __eh_frame = ABSOLUTE(.); - KEEP(*(.eh_frame)) . = (. + 7) & ~ 3; /* * C++ constructor and destructor tables @@ -342,7 +339,27 @@ SECTIONS *(.srodata.*) _thread_local_end = ABSOLUTE(.); _rodata_reserved_end = ABSOLUTE(.); - . = ALIGN(4); + . = ALIGN(ALIGNOF(.eh_frame)); + } > default_rodata_seg + + /* Keep this section shall be at least aligned on 4 */ + .eh_frame : ALIGN(4) + { + __eh_frame = ABSOLUTE(.); + KEEP (*(.eh_frame)) + __eh_frame_end = ABSOLUTE(.); + /* Guarantee that this section and the next one will be merged by making + * them adjacent. */ + . = ALIGN(ALIGNOF(.eh_frame_hdr)); + } > default_rodata_seg + + /* To avoid any exception in C++ exception frame unwinding code, this section + * shall be aligned on 8. */ + .eh_frame_hdr : ALIGN(8) + { + __eh_frame_hdr = ABSOLUTE(.); + KEEP (*(.eh_frame_hdr)) + __eh_frame_hdr_end = ABSOLUTE(.); } > default_rodata_seg /* Marks the end of IRAM code segment */ diff --git a/components/esp_hw_support/CMakeLists.txt b/components/esp_hw_support/CMakeLists.txt index 2fcb16e8bb..b2906aa75d 100644 --- a/components/esp_hw_support/CMakeLists.txt +++ b/components/esp_hw_support/CMakeLists.txt @@ -1,5 +1,6 @@ idf_build_get_property(target IDF_TARGET) +set(priv_requires efuse) set(requires soc) if(${target} STREQUAL "esp32") list(APPEND requires efuse) @@ -17,7 +18,10 @@ if(NOT BOOTLOADER_BUILD) "mac_addr.c" "sleep_modes.c" "regi2c_ctrl.c") - list(APPEND priv_requires esp_ipc) + list(APPEND priv_requires esp_ipc) +else() + # Requires "_esp_error_check_failed()" function + list(APPEND priv_requires "esp_system") endif() idf_component_register(SRCS ${srcs} diff --git a/components/esp_system/CMakeLists.txt b/components/esp_system/CMakeLists.txt index 257ae1e65d..fcc0d458f3 100644 --- a/components/esp_system/CMakeLists.txt +++ b/components/esp_system/CMakeLists.txt @@ -1,14 +1,15 @@ idf_build_get_property(target IDF_TARGET) -set(srcs) +set(srcs "esp_err.c") if(CONFIG_IDF_ENV_FPGA) list(APPEND srcs "fpga_overrides.c") endif() if(BOOTLOADER_BUILD) - # Bootloader relies on some Kconfig options defined in esp_system. - idf_component_register(SRCS "${srcs}") + # "_esp_error_check_failed()" requires spi_flash module + # Bootloader relies on some Kconfig options defined in esp_system. + idf_component_register(SRCS "${srcs}" REQUIRES spi_flash) else() list(APPEND srcs "crosscore_int.c" "esp_err.c" @@ -26,6 +27,10 @@ else() list(APPEND srcs "dbg_stubs.c") endif() + if(CONFIG_ESP_SYSTEM_USE_EH_FRAME) + list(APPEND srcs "eh_frame_parser.c") + endif() + idf_component_register(SRCS "${srcs}" INCLUDE_DIRS include PRIV_REQUIRES spi_flash diff --git a/components/esp_system/Kconfig b/components/esp_system/Kconfig index b7e3b96040..0c5faff0e9 100644 --- a/components/esp_system/Kconfig +++ b/components/esp_system/Kconfig @@ -98,10 +98,12 @@ menu "ESP System Settings" default n depends on IDF_TARGET_ARCH_RISCV help - Generate DWARF information in the resulting binary to perform backtracing when panics occur. Activating - this option will activate asynchronous frame unwinding and generation of both .eh_frame and .eh_frame_hdr - sections, resulting in a bigger binary size (20% to 100% larger). This option shall be not be used for - production. + Generate DWARF information for each function of the project. These information will parsed and used to + perform backtracing when panics occur. Activating this option will activate asynchronous frame 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. menu "Memory protection" diff --git a/components/esp_system/port/arch/riscv/eh_frame_parser.c b/components/esp_system/eh_frame_parser.c similarity index 92% rename from components/esp_system/port/arch/riscv/eh_frame_parser.c rename to components/esp_system/eh_frame_parser.c index 69a4556960..3a2b184b61 100644 --- a/components/esp_system/port/arch/riscv/eh_frame_parser.c +++ b/components/esp_system/eh_frame_parser.c @@ -25,13 +25,14 @@ * http://dwarfstd.org/Download.php */ -#include "port/eh_frame_parser.h" -#include "port/eh_frame_parser_impl.h" +#include "eh_frame_parser.h" #include "esp_private/panic_internal.h" #include #if CONFIG_ESP_SYSTEM_USE_EH_FRAME +#include "eh_frame_parser_impl.h" + /** * @brief Dimension of an array (number of elements) */ @@ -90,8 +91,8 @@ /** * @brief Pointers to both .eh_frame_hdr and .eh_frame sections. */ -#define EH_FRAME_HDR_ADDR (&_eh_frame_hdr) -#define EH_FRAME_ADDR (&_eh_frame) +#define EH_FRAME_HDR_ADDR (&__eh_frame_hdr) +#define EH_FRAME_ADDR (&__eh_frame) /** * @brief Structure of .eh_frame_hdr section header. @@ -259,8 +260,8 @@ typedef struct { * @brief Symbols defined by the linker. * Retrieve the addresses of both .eh_frame_hdr and .eh_frame sections. */ -extern char _eh_frame_hdr; -extern char _eh_frame; +extern char __eh_frame_hdr; +extern char __eh_frame; /** * @brief Decode multiple bytes encoded in LEB128. @@ -439,22 +440,22 @@ static const table_entry* esp_eh_frame_find_entry(const table_entry* sorted_tabl /* Signed comparisons. */ const int32_t sfun_addr = (int32_t) fun_addr; const int32_t snxt_addr = (int32_t) nxt_addr; - if (sfun_addr <= ra && snxt_addr > ra) - found = true; - else if (snxt_addr <= ra) - begin = middle + 1; - else - end = middle; + if (sfun_addr <= ra && snxt_addr > ra) + found = true; + else if (snxt_addr <= ra) + begin = middle + 1; + else + end = middle; } else { /* Unsigned comparisons. */ const uint32_t ura = (uint32_t) ra; - if (fun_addr <= ura && nxt_addr > ura) - found = true; - else if (nxt_addr <= ura) - begin = middle + 1; - else - end = middle; + if (fun_addr <= ura && nxt_addr > ura) + found = true; + else if (nxt_addr <= ura) + begin = middle + 1; + else + end = middle; } middle = (end + begin) / 2; @@ -799,10 +800,45 @@ static uint32_t esp_eh_frame_restore_caller_state(const uint32_t* fde, EXECUTION_FRAME_SP(*frame) = cfa_addr; /* If the frame was not available, it would be possible to retrieve the return address - * register thanks to CIE structure. */ - return EXECUTION_FRAME_REG(frame, ra_reg); + * register thanks to CIE structure. + * The return address points to the address the PC needs to jump to. It + * does NOT point to the instruction where the routine call occured. + * This can cause problems with functions without epilogue (i.e. function + * which last instruction is a function call). This happens when compiler + * optimization are ON or when a function is marked as "noreturn". + * + * Thus, in order to point to the call/jal instruction, we need to + * subtract at least 1 byte but not more than an instruction size. + */ + return EXECUTION_FRAME_REG(frame, ra_reg) - 2; } +/** + * @brief Test whether the DWARF information for the given PC are missing or not. + * + * @param fde FDE associated to this PC. This FDE is the one found thanks to + * `esp_eh_frame_find_entry()`. + * @param pc PC to get information from. + * + * @return true is DWARF information are missing, false else. + */ +static bool esp_eh_frame_missing_info(const uint32_t* fde, uint32_t pc) { + if (fde == NULL) { + return true; + } + + /* Get the range of this FDE entry. It is possible that there are some + * gaps between DWARF entries, in that case, the FDE entry found has + * indeed an initial_location very close to PC but doesn't reach it. + * For example, if FDE initial_location is 0x40300000 and its length is + * 0x100, but PC value is 0x40300200, then some DWARF information + * are missing as there is a gap. + * End the backtrace. */ + const uint32_t initial_location = ((uint32_t) &fde[ESP_FDE_INITLOC_IDX] + fde[ESP_FDE_INITLOC_IDX]); + const uint32_t range_length = fde[ESP_FDE_RANGELEN_IDX]; + + return (initial_location + range_length) <= pc; +} /** * @brief When one step of the backtrace is generated, output it to the serial. @@ -824,10 +860,12 @@ void __attribute__((weak)) esp_eh_frame_generated_step(uint32_t pc, uint32_t sp) * * @param frame_or Snapshot of the CPU registers when the CPU stopped its normal execution. */ -void esp_eh_frame_print_backtrace(const ExecutionFrame *frame_or) +void esp_eh_frame_print_backtrace(const void *frame_or) { + assert(frame_or != NULL); + static dwarf_regs state = { 0 }; - ExecutionFrame frame = *frame_or; + ExecutionFrame frame = *((ExecutionFrame*) frame_or); uint32_t size = 0; uint8_t* enc_values = NULL; bool end_of_backtrace = false; @@ -865,16 +903,23 @@ void esp_eh_frame_print_backtrace(const ExecutionFrame *frame_or) const table_entry* from_fun = esp_eh_frame_find_entry(sorted_table, fde_count, table_enc, EXECUTION_FRAME_PC(frame)); - if (from_fun == 0) { + /* Get absolute address of FDE entry describing the function where PC left of. */ + uint32_t* fde = NULL; + + if (from_fun != NULL) { + fde = esp_eh_frame_decode_address(&from_fun->fde_addr, table_enc); + } + + if (esp_eh_frame_missing_info(fde, EXECUTION_FRAME_PC(frame))) { /* Address was not found in the list. */ + panic_print_str("\r\nBacktrace ended abruptly: cannot find DWARF information for" + " instruction at address 0x"); + panic_print_hex(EXECUTION_FRAME_PC(frame)); + panic_print_str("\r\n"); break; } - /* Get absolute address of FDE entry describing the function where PC left of. */ - const uint32_t* fde = esp_eh_frame_decode_address(&from_fun->fde_addr, table_enc); - - /* Clean and set the DWARF register structure. - * TODO: Initialization should be done by the instruction contained by the CIE associated to the FDE. */ + /* Clean and set the DWARF register structure. */ memset(&state, 0, sizeof(dwarf_regs)); const uint32_t prev_sp = EXECUTION_FRAME_SP(frame); diff --git a/components/esp_system/port/arch/riscv/test_eh_frame_parser/port/eh_frame_parser.h b/components/esp_system/include/eh_frame_parser.h similarity index 68% rename from components/esp_system/port/arch/riscv/test_eh_frame_parser/port/eh_frame_parser.h rename to components/esp_system/include/eh_frame_parser.h index fe0b16b685..f99f63bb77 100644 --- a/components/esp_system/port/arch/riscv/test_eh_frame_parser/port/eh_frame_parser.h +++ b/components/esp_system/include/eh_frame_parser.h @@ -15,13 +15,21 @@ #ifndef EH_FRAME_PARSER_H #define EH_FRAME_PARSER_H -#include "eh_frame_parser_impl.h" +#ifdef __cplusplus +extern "C" { +#endif /** * @brief Print backtrace for the given execution frame. * - * @param frame_or Snapshot of the CPU registers when the CPU stopped its normal execution. + * @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_eh_frame_print_backtrace(const ExecutionFrame *frame_or); +void esp_eh_frame_print_backtrace(const void *frame_or); + +#ifdef __cplusplus +} +#endif #endif diff --git a/components/esp_system/port/CMakeLists.txt b/components/esp_system/port/CMakeLists.txt index d37eb2a5c8..debcacb4ea 100644 --- a/components/esp_system/port/CMakeLists.txt +++ b/components/esp_system/port/CMakeLists.txt @@ -1,4 +1,10 @@ -target_include_directories(${COMPONENT_LIB} PRIVATE . include PUBLIC soc) +set(INCLUDE_FILES "include" . include PUBLIC soc) + +if(CONFIG_IDF_TARGET_ARCH_RISCV) + list(APPEND INCLUDE_FILES "include/riscv") +endif() + +target_include_directories(${COMPONENT_LIB} PRIVATE ${INCLUDE_FILES}) target_include_directories(${COMPONENT_LIB} PUBLIC public_compat) set(srcs "cpu_start.c" "panic_handler.c" "brownout.c") diff --git a/components/esp_system/port/arch/riscv/panic_arch.c b/components/esp_system/port/arch/riscv/panic_arch.c index f629afc99b..1607dd0e3c 100644 --- a/components/esp_system/port/arch/riscv/panic_arch.c +++ b/components/esp_system/port/arch/riscv/panic_arch.c @@ -14,6 +14,8 @@ // limitations under the License. #include +#include "esp_spi_flash.h" + #include "soc/extmem_reg.h" #include "esp_private/panic_internal.h" #include "esp_private/panic_reason.h" @@ -29,7 +31,7 @@ #endif #if CONFIG_ESP_SYSTEM_USE_EH_FRAME -#include "port/eh_frame_parser.h" +#include "eh_frame_parser.h" #endif @@ -324,11 +326,7 @@ void panic_arch_fill_info(void *frame, panic_info_t *info) info->frame = ®s; } -void panic_print_backtrace(const void *frame, int core) -{ -#if CONFIG_ESP_SYSTEM_USE_EH_FRAME - esp_eh_frame_print_backtrace(frame); -#else +static void panic_print_basic_backtrace(const void *frame, int core) { // Basic backtrace panic_print_str("\r\nStack memory:\r\n"); uint32_t sp = (uint32_t)((RvExcFrame *)frame)->sp; @@ -343,6 +341,20 @@ void panic_print_backtrace(const void *frame, int core) panic_print_str(y == per_line - 1 ? "\r\n" : " "); } } +} + +void panic_print_backtrace(const void *frame, int core) +{ +#if CONFIG_ESP_SYSTEM_USE_EH_FRAME + if (!spi_flash_cache_enabled()) { + panic_print_str("\r\nWarning: SPI Flash cache is disabled, cannot process eh_frame parsing. " + "Falling back to basic backtrace.\r\n"); + panic_print_basic_backtrace(frame, core); + } else { + esp_eh_frame_print_backtrace(frame); + } +#else + panic_print_basic_backtrace(frame, core); #endif } diff --git a/components/esp_system/port/include/port/eh_frame_parser.h b/components/esp_system/port/include/port/eh_frame_parser.h deleted file mode 100644 index fe0b16b685..0000000000 --- a/components/esp_system/port/include/port/eh_frame_parser.h +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2020 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef EH_FRAME_PARSER_H -#define EH_FRAME_PARSER_H - -#include "eh_frame_parser_impl.h" - -/** - * @brief Print backtrace for the given execution frame. - * - * @param frame_or Snapshot of the CPU registers when the CPU stopped its normal execution. - */ -void esp_eh_frame_print_backtrace(const ExecutionFrame *frame_or); - -#endif diff --git a/components/esp_system/port/include/port/eh_frame_parser_impl.h b/components/esp_system/port/include/riscv/eh_frame_parser_impl.h similarity index 100% rename from components/esp_system/port/include/port/eh_frame_parser_impl.h rename to components/esp_system/port/include/riscv/eh_frame_parser_impl.h diff --git a/components/esp_system/port/soc/esp32c3/CMakeLists.txt b/components/esp_system/port/soc/esp32c3/CMakeLists.txt index f96329e14e..6f1bf116c0 100644 --- a/components/esp_system/port/soc/esp32c3/CMakeLists.txt +++ b/components/esp_system/port/soc/esp32c3/CMakeLists.txt @@ -7,7 +7,6 @@ set(srcs "clk.c" "../../arch/riscv/expression_with_stack_asm.S" "../../arch/riscv/panic_arch.c") -set(srcs ${srcs} "../../arch/riscv/eh_frame_parser.c") add_prefix(srcs "${CMAKE_CURRENT_LIST_DIR}/" ${srcs}) target_sources(${COMPONENT_LIB} PRIVATE ${srcs}) diff --git a/components/esp_system/port/arch/riscv/test_eh_frame_parser/Makefile b/components/esp_system/test_eh_frame_parser/Makefile similarity index 90% rename from components/esp_system/port/arch/riscv/test_eh_frame_parser/Makefile rename to components/esp_system/test_eh_frame_parser/Makefile index 7d82810f59..6a51f89fd9 100644 --- a/components/esp_system/port/arch/riscv/test_eh_frame_parser/Makefile +++ b/components/esp_system/test_eh_frame_parser/Makefile @@ -14,7 +14,7 @@ # limitations under the License. CC=gcc -CFLAGS=-W -fasynchronous-unwind-tables -I. -std=c99 -g -DCONFIG_ESP_SYSTEM_USE_EH_FRAME -m32 +CFLAGS=-W -fasynchronous-unwind-tables -I. -I../include/ -std=c99 -g -DCONFIG_ESP_SYSTEM_USE_EH_FRAME -m32 LDFLAGS=-Wl,--eh-frame-hdr -m32 -g -Tlinker.ld -no-pie OBJECTS=objs/eh_frame_parser.o objs/main.o HEADERS=eh_frame_parser_impl.h diff --git a/components/esp_system/test_eh_frame_parser/README.md b/components/esp_system/test_eh_frame_parser/README.md new file mode 100644 index 0000000000..23cd331710 --- /dev/null +++ b/components/esp_system/test_eh_frame_parser/README.md @@ -0,0 +1,42 @@ +# Host test for EH_FRAME_PARSER + +This test is meant to be run on a Linux x86(_64) host. The main purpose is to +trigger a SIGSEV (NULL pointer) exception at runtime in order to generate the +backtrace. It is then checked that the functions in the call stack are indeed +correctly determined in the right order. + +## Requirements + +A Linux host, x86 or x86_64. In any case, the example will be compiled with +the option `-m32`, it is then required to have gcc multilibs. + +## Compile the example + +To compile the example, simply type: +``` +make +``` + +## Run the code + +Execute the binary generated: +``` +./eh_frame_test +``` + +If everything goes well, the output should be as is: +``` +All tests passed +``` + +## Known issue + +DWARF instructions in `x86` binaries include the instruction `DW_CFA_expression`. +However, this instruction is not supported by the parser, this is why the test +may print the following message: +``` +Unsupported DWARF opcode 0: 0x10 +``` + +This is not a big problem as the functions implemented for testing the backtrace +will not generate unimplemented DWARF instructions. diff --git a/components/esp_system/port/arch/riscv/test_eh_frame_parser/port/eh_frame_parser_impl.h b/components/esp_system/test_eh_frame_parser/eh_frame_parser_impl.h similarity index 100% rename from components/esp_system/port/arch/riscv/test_eh_frame_parser/port/eh_frame_parser_impl.h rename to components/esp_system/test_eh_frame_parser/eh_frame_parser_impl.h diff --git a/components/esp_system/port/arch/riscv/test_eh_frame_parser/esp_private/panic_internal.h b/components/esp_system/test_eh_frame_parser/esp_private/panic_internal.h similarity index 100% rename from components/esp_system/port/arch/riscv/test_eh_frame_parser/esp_private/panic_internal.h rename to components/esp_system/test_eh_frame_parser/esp_private/panic_internal.h diff --git a/components/esp_system/port/arch/riscv/test_eh_frame_parser/linker.ld b/components/esp_system/test_eh_frame_parser/linker.ld similarity index 85% rename from components/esp_system/port/arch/riscv/test_eh_frame_parser/linker.ld rename to components/esp_system/test_eh_frame_parser/linker.ld index 33644e50d2..f562209141 100644 --- a/components/esp_system/port/arch/riscv/test_eh_frame_parser/linker.ld +++ b/components/esp_system/test_eh_frame_parser/linker.ld @@ -18,12 +18,12 @@ SECTIONS { .eh_frame_hdr : { - _eh_frame_hdr = ABSOLUTE(.); - *(.eh_frame_hdr) *(.eh_frame_entry .eh_frame_entry.*) + __eh_frame_hdr = ABSOLUTE(.); + *(.eh_frame_hdr) *(.eh_frame_entry .eh_frame_entry.*) } .eh_frame : ONLY_IF_RO { - _eh_frame = ABSOLUTE(.); + __eh_frame = ABSOLUTE(.); KEEP (*(.eh_frame)) *(.eh_frame.*) } } diff --git a/components/esp_system/port/arch/riscv/test_eh_frame_parser/main.c b/components/esp_system/test_eh_frame_parser/main.c similarity index 86% rename from components/esp_system/port/arch/riscv/test_eh_frame_parser/main.c rename to components/esp_system/test_eh_frame_parser/main.c index 4bd0752470..cdde0ed341 100644 --- a/components/esp_system/port/arch/riscv/test_eh_frame_parser/main.c +++ b/components/esp_system/test_eh_frame_parser/main.c @@ -20,7 +20,8 @@ * parsing `eh_frame` and `eh_frame_hdr`. */ -#define _POSIX_C_SOURCE 199309L +#define _POSIX_C_SOURCE 200809L +#define _DEFAULT_SOURCE #include #include #include @@ -29,7 +30,8 @@ #include #include #include -#include "port/eh_frame_parser.h" +#include "../include/eh_frame_parser.h" +#include "eh_frame_parser_impl.h" /** * @brief Index of x86 registers in `greg_t` structure. @@ -53,7 +55,12 @@ * @brief Number which will determine the depth of the call stack. * Check `main()` for more information. */ -#define NUMBER_TO_TEST (4) +#define NUMBER_TO_TEST (4) + +/** + * @brief Number of iteration for function `esp_eh_frame_generated_step`. + */ +#define NUMBER_OF_ITERATION (2 * NUMBER_TO_TEST + 2 + 1) /** * @brief Define a simple linked list type and initialize one. @@ -106,7 +113,7 @@ struct functions_info funs[] = { /** * @brief Test whether the address passed as PC is part of the function which - * name is `funciton_name`. The global array `funs` is used. + * name is `function_name`. The global array `funs` is used. * * @param pc Program counter to test. (address in the program) * @param function_name Function name to check the address of. @@ -126,19 +133,20 @@ bool is_pc_in_function(const uint32_t pc, const char* function_name) return false; } +/** + * @brief Number of times `esp_eh_frame_generated_step` is called. + */ +static uint32_t iteration = 1; + /** * @brief Override the default function called when a backtrace step is * generated. */ void esp_eh_frame_generated_step(uint32_t pc, uint32_t sp) { - /* Number of times this function has been entered. */ - static uint32_t iteration = 1; - - /* The first PC in the backtrace are calls to `browse_list()`. - * +2 is due to the fact that the list contains all the numbers + /* The first PCs in the backtrace are calls to `browse_list()` + 2. + * This is due to the fact that the list contains all the numbers * between NUMBER_TO_TEST to 0 included. Moreover, another call - * is made when we meet the NULL pointer. - */ + * is made when we meet the NULL pointer. */ if (iteration > 0 && iteration <= (NUMBER_TO_TEST + 2)) { assert(is_pc_in_function(pc, "browse_list")); } else { @@ -151,13 +159,15 @@ void esp_eh_frame_generated_step(uint32_t pc, uint32_t sp) { * browse_list (NUMBER_TO_TEST + 2 iterations), is_even * (NUMBER_TO_TEST/2 calls) and is_odd (NUMBER_TO_TEST/2 calls) calls. */ - if (iteration > (2 * NUMBER_TO_TEST + 2)) + if (iteration >= NUMBER_OF_ITERATION) return; else if (iteration % 2 == 0) assert(is_pc_in_function(pc, "is_odd")); else assert(is_pc_in_function(pc, "is_even")); } + + /* Number of times this function has been entered. */ iteration++; } @@ -175,7 +185,7 @@ void signal_handler(int signal, siginfo_t *info, void *ucontext) { * Indeed, the registers index defined in ucontext.h are NOT the same * the registers index DWARF is expecting. */ ucontext_t* context = (ucontext_t*) ucontext; - greg_t *gregset = context->uc_mcontext.__gregs; + greg_t *gregset = context->uc_mcontext.gregs; x86ExcFrame frame = { .eax = gregset[REG_EAX], .ecx = gregset[REG_ECX], @@ -195,8 +205,15 @@ void signal_handler(int signal, siginfo_t *info, void *ucontext) { */ esp_eh_frame_print_backtrace(&frame); - /* No assert has been triggered, the backtrace succeeded. */ - printf("\e[32m\e[1mAll tests passed \e[0m\r\n"); + /* No assert has been triggered, the backtrace succeeded if the number of + * iterations of function `esp_eh_frame_generated_step` is correct. */ + if (iteration == NUMBER_OF_ITERATION) { + printf("\e[32m\e[1mAll tests passed \e[0m\r\n"); + } else { + printf("\e[31m\e[1mWrong length of backtrace (%d iteration, expected %d) \e[0m\r\n", + iteration, NUMBER_OF_ITERATION); + exit(1); + } /* Everything went fine, exit normally. */ exit(0); @@ -298,6 +315,7 @@ int main (int argc, char** argv) int res = sigaction(SIGSEGV, &sig, NULL); if (res) { perror("Could not override SIGSEV signal"); + return 1; } /* Trigger the segmentation fault with a complex backtrace. */ diff --git a/components/hal/CMakeLists.txt b/components/hal/CMakeLists.txt index d4a1a86df4..e7a87605b0 100644 --- a/components/hal/CMakeLists.txt +++ b/components/hal/CMakeLists.txt @@ -1,12 +1,13 @@ idf_build_get_property(target IDF_TARGET) set(srcs "wdt_hal_iram.c" - "mpu_hal.c") + "mpu_hal.c" + "cpu_hal.c") + set(includes "${target}/include" "include" "platform_port/include") if(NOT BOOTLOADER_BUILD) list(APPEND srcs - "cpu_hal.c" "rmt_hal.c" "rtc_io_hal.c" "spi_hal.c" diff --git a/docs/en/api-guides/fatal-errors.rst b/docs/en/api-guides/fatal-errors.rst index 09e80c5e3f..65354fd238 100644 --- a/docs/en/api-guides/fatal-errors.rst +++ b/docs/en/api-guides/fatal-errors.rst @@ -207,6 +207,37 @@ If :doc:`IDF Monitor ` is used, Program Counter values will b MSTATUS : 0x00001881 MTVEC : 0x40380001 MCAUSE : 0x00000007 MTVAL : 0x00000000 MHARTID : 0x00000000 + Moreover, it is also capable of generating and printing a backtrace thanks to the stack dump provided by the board in the panic handler. + The output looks like this: + + :: + + Backtrace: + + 0x42006686 in bar (ptr=ptr@entry=0x0) at ../main/hello_world_main.c:18 + 18 *ptr = 0x42424242; + #0 0x42006686 in bar (ptr=ptr@entry=0x0) at ../main/hello_world_main.c:18 + #1 0x42006692 in foo () at ../main/hello_world_main.c:22 + #2 0x420066ac in app_main () at ../main/hello_world_main.c:28 + #3 0x42015ece in main_task (args=) at /Users/user/esp/components/freertos/port/port_common.c:142 + #4 0x403859b8 in vPortEnterCritical () at /Users/user/esp/components/freertos/port/riscv/port.c:130 + #5 0x00000000 in ?? () + Backtrace stopped: frame did not save the PC + + While this is very handy efficient, it requires the user to use :doc:`IDF Monitor `. Thus, in order to generate and print a backtrace while using another monitor program, it is possible to activate :ref:`CONFIG_ESP_SYSTEM_USE_EH_FRAME` option from the menuconfig. + + This option will let the compiler generate DWARF information for each function of the project. Then, when a CPU exception occurs, the panic handler will parse these data and determine the backtrace of the task that failed. The output looks like this: + + :: + + Backtrace: 0x42009e9a:0x3fc92120 0x42009ea6:0x3fc92120 0x42009ec2:0x3fc92130 0x42024620:0x3fc92150 0x40387d7c:0x3fc92160 0xfffffffe:0x3fc92170 + + These PC:SP pairs represent PC, the Program Counter and SP, the Stack Pointer for each stack frame of the current task. + + + The main benefit of this option is that this trace is generate by the board itself. Its drawback is that it results in a larger compiled binary, with an increase that can go from 20% to 100%. Finally, it is highly advised to not use this option for production as it results in the presence of debug information within the final binary. + + To find the location where a fatal error has happened, look at the lines which follow the "Backtrace" line. Fatal error location is the top line, and subsequent lines show the call stack. .. _GDB-Stub: @@ -489,4 +520,4 @@ The types of errors reported by UBSAN can be as follows: * - ``builtin_unreachable`` - ``__builtin_unreachable`` function called. * - ``pointer_overflow`` - - Overflow in pointer arithmetic. \ No newline at end of file + - Overflow in pointer arithmetic.