From d9534b3d6a8a481520befaa5d936b5d51ac8ba31 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Fri, 15 Nov 2019 14:59:14 +0100 Subject: [PATCH 1/3] soc: fix backtraces containing ROM functions esp_ptr_executable would return false for pointers to ROM, which would interrupt the backtrace. This makes ROM ranges recognized as executable. --- components/soc/esp32/include/soc/soc.h | 2 +- components/soc/esp32s2beta/include/soc/soc.h | 2 ++ components/soc/include/soc/soc_memory_layout.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/components/soc/esp32/include/soc/soc.h b/components/soc/esp32/include/soc/soc.h index 72527687f2..998baa2a86 100644 --- a/components/soc/esp32/include/soc/soc.h +++ b/components/soc/esp32/include/soc/soc.h @@ -240,7 +240,7 @@ #define SOC_IROM_LOW 0x400D0000 #define SOC_IROM_HIGH 0x40400000 #define SOC_IROM_MASK_LOW 0x40000000 -#define SOC_IROM_MASK_HIGH 0x40070000 +#define SOC_IROM_MASK_HIGH 0x40064F00 #define SOC_CACHE_PRO_LOW 0x40070000 #define SOC_CACHE_PRO_HIGH 0x40078000 #define SOC_CACHE_APP_LOW 0x40078000 diff --git a/components/soc/esp32s2beta/include/soc/soc.h b/components/soc/esp32s2beta/include/soc/soc.h index 52f0a42a85..515f8d643f 100644 --- a/components/soc/esp32s2beta/include/soc/soc.h +++ b/components/soc/esp32s2beta/include/soc/soc.h @@ -184,6 +184,8 @@ #define SOC_DROM_HIGH 0x3F400000 #define SOC_IROM_LOW 0x40080000 #define SOC_IROM_HIGH 0x40c00000 +#define SOC_IROM_MASK_LOW 0x40000000 +#define SOC_IROM_MASK_HIGH 0x4001A100 #define SOC_IRAM_LOW 0x40020000 #define SOC_IRAM_HIGH 0x40070000 #define SOC_DRAM_LOW 0x3FFB0000 diff --git a/components/soc/include/soc/soc_memory_layout.h b/components/soc/include/soc/soc_memory_layout.h index 75c8c7daa6..0146630ea3 100644 --- a/components/soc/include/soc/soc_memory_layout.h +++ b/components/soc/include/soc/soc_memory_layout.h @@ -155,6 +155,7 @@ inline static bool IRAM_ATTR esp_ptr_executable(const void *p) intptr_t ip = (intptr_t) p; return (ip >= SOC_IROM_LOW && ip < SOC_IROM_HIGH) || (ip >= SOC_IRAM_LOW && ip < SOC_IRAM_HIGH) + || (ip >= SOC_IROM_MASK_LOW && ip < SOC_IROM_MASK_HIGH) || (ip >= SOC_RTC_IRAM_LOW && ip < SOC_RTC_IRAM_HIGH); } From b4aba189aba43facea0d8831863364613ee7105a Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 2 Jan 2020 18:42:19 +0100 Subject: [PATCH 2/3] heap: recognize 0x40000000 as an address terminating the backtrace On Xtensa, backtrace can not recover the two most significant bits of the address, as the window call size is encoded in these bits. Because of this, __builtin_return_address modifies these MSBs to match those of the callee, "fixing" the address. An unfortunate side effect is that the zero return address, which usually terminates the backtrace, gets converted to 0x40000000. While there is a valid instruction at this address, its occurrence in the backtrace is highly unlikely: this is the first instruction of WindowOverflow4 vector, and IDF apps switch VECBASE to an IRAM location very early at startup. --- components/heap/include/heap_trace.inc | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/components/heap/include/heap_trace.inc b/components/heap/include/heap_trace.inc index 7c3c4332ee..e6a039530b 100644 --- a/components/heap/include/heap_trace.inc +++ b/components/heap/include/heap_trace.inc @@ -26,6 +26,15 @@ inline static uint32_t get_ccount(void) return ccount; } +/* Architecture-specific return value of __builtin_return_address which + * should be interpreted as an invalid address. + */ +#ifdef __XTENSA__ +#define HEAP_ARCH_INVALID_PC 0x40000000 +#else +#define HEAP_ARCH_INVALID_PC 0x00000000 +#endif + // Caller is 2 stack frames deeper than we care about #define STACK_OFFSET 2 @@ -33,8 +42,9 @@ inline static uint32_t get_ccount(void) if (STACK_DEPTH == N) { \ return; \ } \ - callers[N] = __builtin_return_address(N+STACK_OFFSET); \ - if (!esp_ptr_executable(callers[N])) { \ + callers[N] = __builtin_return_address(N+STACK_OFFSET); \ + if (!esp_ptr_executable(callers[N]) \ + || callers[N] == (void*) HEAP_ARCH_INVALID_PC) { \ callers[N] = 0; \ return; \ } \ From 43de2cc84cfb1847ba46076d946ccc94c8418034 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 2 Jan 2020 18:50:32 +0100 Subject: [PATCH 3/3] test: add a (non-automated) case for backtraces with ROM functions --- components/esp32/test/test_backtrace.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/components/esp32/test/test_backtrace.c b/components/esp32/test/test_backtrace.c index 2c1f4d8737..1520a8dde0 100644 --- a/components/esp32/test/test_backtrace.c +++ b/components/esp32/test/test_backtrace.c @@ -69,3 +69,15 @@ TEST_CASE("Test backtrace from interrupt watchdog timeout", "[reset_reason][rese backtrace_trigger_source = ACTION_INT_WDT; recursive_func(RECUR_DEPTH, SW_ISR_LEVEL_1); //Trigger lvl 1 SW interrupt at max recursive depth } + +static void write_char_crash(char c) +{ + ets_write_char_uart(c); + *(char*) 0x00000001 = 0; +} + +TEST_CASE("Test backtrace with a ROM function", "[reset_reason][reset=StoreProhibited,SW_CPU_RESET]") +{ + ets_install_putc1(&write_char_crash); + ets_printf("foo"); +}