diff --git a/components/esp_rom/esp32s2/ld/esp32s2.rom.newlib-funcs.ld b/components/esp_rom/esp32s2/ld/esp32s2.rom.newlib-funcs.ld index c859f655b4..fc976bbbaf 100644 --- a/components/esp_rom/esp32s2/ld/esp32s2.rom.newlib-funcs.ld +++ b/components/esp_rom/esp32s2/ld/esp32s2.rom.newlib-funcs.ld @@ -10,8 +10,8 @@ abs = 0x40000618; __ascii_mbtowc = 0x40007a04; __ascii_wctomb = 0x400018d0; -__assert = 0x4001a430; -__assert_func = 0x4001a408; +PROVIDE ( __assert = 0x4001a430 ); +PROVIDE ( __assert_func = 0x4001a408 ); bzero = 0x400078c8; _cleanup_r = 0x4001a480; creat = 0x4000788c; diff --git a/components/esp_system/panic.c b/components/esp_system/panic.c index 5c5b4f0d3d..b37f1cd3d0 100644 --- a/components/esp_system/panic.c +++ b/components/esp_system/panic.c @@ -362,7 +362,7 @@ void esp_panic_handler(panic_info_t *info) } -void __attribute__((noreturn)) panic_abort(const char *details) +void IRAM_ATTR __attribute__((noreturn)) panic_abort(const char *details) { g_panic_abort = true; s_panic_abort_details = (char*) details; diff --git a/components/freertos/test/test_freertos_mutex.c b/components/freertos/test/test_freertos_mutex.c index e4b68bb6a0..bd082c068a 100644 --- a/components/freertos/test/test_freertos_mutex.c +++ b/components/freertos/test/test_freertos_mutex.c @@ -14,7 +14,7 @@ static void mutex_release_task(void* arg) TEST_FAIL_MESSAGE("should not be reached"); } -TEST_CASE("mutex released not by owner causes an assert", "[freertos][reset=abort,SW_CPU_RESET]") +TEST_CASE("mutex released not by owner causes an assert", "[freertos][reset=assert,SW_CPU_RESET]") { SemaphoreHandle_t mutex = xSemaphoreCreateMutex(); xSemaphoreTake(mutex, portMAX_DELAY); diff --git a/components/newlib/CMakeLists.txt b/components/newlib/CMakeLists.txt index 924a27212b..077db45299 100644 --- a/components/newlib/CMakeLists.txt +++ b/components/newlib/CMakeLists.txt @@ -6,6 +6,7 @@ endif() set(srcs "abort.c" + "assert.c" "heap.c" "locks.c" "poll.c" @@ -27,7 +28,7 @@ list(APPEND ldfragments newlib.lf) idf_component_register(SRCS "${srcs}" INCLUDE_DIRS "${include_dirs}" PRIV_INCLUDE_DIRS priv_include - PRIV_REQUIRES soc + PRIV_REQUIRES soc spi_flash LDFRAGMENTS "${ldfragments}") # Toolchain libraries require code defined in this component @@ -36,11 +37,12 @@ target_link_libraries(${COMPONENT_LIB} INTERFACE c m gcc "$ +#include +#include "esp_system.h" +#include "esp_spi_flash.h" +#include "soc/soc_memory_layout.h" + +#define ASSERT_STR "assert failed: " +#define CACHE_DISABLED_STR "" + +#if __XTENSA__ +#define INST_LEN 3 +#elif __riscv +#define INST_LEN 4 +#endif + +static inline void ra_to_str(char *addr) +{ + addr[0] = '0'; + addr[1] = 'x'; + itoa((uint32_t)(__builtin_return_address(0) - INST_LEN), addr + 2, 16); +} + +/* Overriding assert function so that whenever assert is called from critical section, + * it does not lead to a crash of its own. + */ +void __attribute__((noreturn)) __assert_func(const char *file, int line, const char *func, const char *expr) +{ +#if CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT + char buff[sizeof(ASSERT_STR) + 11 + 1] = ASSERT_STR; + + ra_to_str(&buff[sizeof(ASSERT_STR) - 1]); + + esp_system_abort(buff); +#else + char addr[11] = { 0 }; + char buff[200]; + char lbuf[5]; + uint32_t rem_len = sizeof(buff) - 1; + uint32_t off = 0; + + itoa(line, lbuf, 10); + + if (!spi_flash_cache_enabled()) { + if (esp_ptr_in_drom(file)) { + file = CACHE_DISABLED_STR; + } + + if (esp_ptr_in_drom(func)) { + ra_to_str(addr); + func = addr; + } + + if (esp_ptr_in_drom(expr)) { + expr = CACHE_DISABLED_STR; + } + } + + const char *str[] = {ASSERT_STR, func ? func : "\b", " ", file, ":", lbuf, " (", expr, ")"}; + + for (int i = 0; i < sizeof(str) / sizeof(str[0]); i++) { + uint32_t len = strlen(str[i]); + uint32_t cpy_len = MIN(len, rem_len); + memcpy(buff + off, str[i], cpy_len); + rem_len -= cpy_len; + off += cpy_len; + if (rem_len == 0) { + break; + } + } + buff[off] = '\0'; + esp_system_abort(buff); +#endif /* CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT */ +} + +void __attribute__((noreturn)) __assert(const char *file, int line, const char *failedexpr) +{ + __assert_func(file, line, NULL, failedexpr); +} + +/* No-op function, used to force linker to include these changes */ +void newlib_include_assert_impl(void) +{ +} diff --git a/components/newlib/component.mk b/components/newlib/component.mk index 5f79c910e7..4cca270a4c 100644 --- a/components/newlib/component.mk +++ b/components/newlib/component.mk @@ -15,10 +15,12 @@ endif COMPONENT_PRIV_INCLUDEDIRS := priv_include COMPONENT_SRCDIRS := . port -# Forces the linker to include heap, syscalls, and pthread from this component, +# Forces the linker to include heap, syscalls, pthread, and assert from this component, # instead of the implementations provided by newlib. COMPONENT_ADD_LDFLAGS += -u newlib_include_heap_impl COMPONENT_ADD_LDFLAGS += -u newlib_include_syscalls_impl +COMPONENT_ADD_LDFLAGS += -u newlib_include_pthread_impl +COMPONENT_ADD_LDFLAGS += -u newlib_include_assert_impl COMPONENT_ADD_LDFRAGMENTS += newlib.lf diff --git a/components/newlib/newlib.lf b/components/newlib/newlib.lf index 63a93164a2..5f1c382f8d 100644 --- a/components/newlib/newlib.lf +++ b/components/newlib/newlib.lf @@ -3,3 +3,4 @@ archive: libnewlib.a entries: heap (noflash) abort (noflash) + assert (noflash) diff --git a/components/newlib/platform_include/assert.h b/components/newlib/platform_include/assert.h index 77d4cf08f5..39db39a6f0 100644 --- a/components/newlib/platform_include/assert.h +++ b/components/newlib/platform_include/assert.h @@ -19,23 +19,34 @@ #pragma once #include #include -#include "esp_compiler.h" +#include #include_next -#if defined(CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT) && !defined(NDEBUG) - #undef assert - #define assert(__e) (likely(__e)) ? (void)0 : abort() -#else - /* moved part of toolchain provided assert to there then - * we can tweak the original assert macro to perform likely - * before deliver it to original toolchain implementation - */ - #undef assert - #ifdef NDEBUG - # define assert(__e) ((void)0) - #else - # define assert(__e) (likely(__e) ? (void)0 : __assert_func (__FILE__, __LINE__, \ - __ASSERT_FUNC, #__e)) - #endif +/* moved part of libc provided assert to here allows + * tweaking the assert macro to use __builtin_expect() + * and reduce jumps in the "asserts OK" code path + * + * Note: using __builtin_expect() not likely() to avoid defining the likely + * macro in namespace of non-IDF code that may include this standard header. + */ +#undef assert + +/* __FILENAME__ points to the file name instead of path + filename + * e.g __FILE__ points to "/apps/test.c" where as __FILENAME__ points to "test.c" + */ +#define __FILENAME__ (__builtin_strrchr( "/" __FILE__, '/') + 1) + +#if defined(NDEBUG) + +#define assert(__e) ((void)(__e)) + +#elif CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT + +#define assert(__e) (__builtin_expect(!!(__e), 1) ? (void)0 : __assert_func(NULL, 0, NULL, NULL)) + +#else // !CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT + +#define assert(__e) (__builtin_expect(!!(__e), 1) ? (void)0 : __assert_func (__FILENAME__, __LINE__, \ + __ASSERT_FUNC, #__e)) #endif diff --git a/tools/test_apps/system/panic/app_test.py b/tools/test_apps/system/panic/app_test.py index 971ed2a735..a6393c47d0 100644 --- a/tools/test_apps/system/panic/app_test.py +++ b/tools/test_apps/system/panic/app_test.py @@ -267,6 +267,11 @@ def test_panic_abort(env, _extra_data): test.abort_inner(env, 'panic') +@panic_test(target=['ESP32']) +def test_panic_abort_cache_disabled(env, _extra_data): + test.abort_cached_disabled_inner(env, 'panic') + + @panic_test() def test_coredump_abort_uart_elf_crc(env, _extra_data): test.abort_inner(env, 'coredump_uart_elf_crc') @@ -292,5 +297,17 @@ def test_gdbstub_abort(env, _extra_data): test.abort_inner(env, 'gdbstub') +# test_assert + +@panic_test(target=['ESP32', 'ESP32S2']) +def test_panic_assert(env, _extra_data): + test.assert_inner(env, 'panic') + + +@panic_test(target=['ESP32']) +def test_panic_assert_cache_disabled(env, _extra_data): + test.assert_cached_disabled_inner(env, 'panic') + + if __name__ == '__main__': run_all(__file__, sys.argv[1:]) diff --git a/tools/test_apps/system/panic/main/test_panic_main.c b/tools/test_apps/system/panic/main/test_panic_main.c index b7e42a1c3f..00a5cbc22d 100644 --- a/tools/test_apps/system/panic/main/test_panic_main.c +++ b/tools/test_apps/system/panic/main/test_panic_main.c @@ -14,6 +14,7 @@ static const char* get_test_name(void); /* functions which cause an exception/panic in different ways */ static void test_abort(void); +static void test_abort_cache_disabled(void); static void test_int_wdt(void); static void test_task_wdt(void); static void test_storeprohibited(void); @@ -22,6 +23,8 @@ static void test_int_wdt_cache_disabled(void); static void test_stack_overflow(void); static void test_illegal_instruction(void); static void test_instr_fetch_prohibited(void); +static void test_assert(void); +static void test_assert_cache_disabled(void); void app_main(void) @@ -44,6 +47,7 @@ void app_main(void) } HANDLE_TEST(test_abort); + HANDLE_TEST(test_abort_cache_disabled); HANDLE_TEST(test_int_wdt); HANDLE_TEST(test_task_wdt); HANDLE_TEST(test_storeprohibited); @@ -52,6 +56,8 @@ void app_main(void) HANDLE_TEST(test_stack_overflow); HANDLE_TEST(test_illegal_instruction); HANDLE_TEST(test_instr_fetch_prohibited); + HANDLE_TEST(test_assert); + HANDLE_TEST(test_assert_cache_disabled); #undef HANDLE_TEST @@ -65,6 +71,12 @@ static void test_abort(void) abort(); } +static void IRAM_ATTR test_abort_cache_disabled(void) +{ + esp_flash_default_chip->os_func->start(esp_flash_default_chip->os_func_data); + abort(); +} + static void test_int_wdt(void) { portDISABLE_INTERRUPTS(); @@ -100,6 +112,17 @@ static void IRAM_ATTR test_int_wdt_cache_disabled(void) } } +static void test_assert(void) +{ + assert(0); +} + +static void IRAM_ATTR test_assert_cache_disabled(void) +{ + esp_flash_default_chip->os_func->start(esp_flash_default_chip->os_func_data); + assert(0); +} + /** * This function overwrites the stack beginning from the valid area continuously towards and beyond * the end of the stack (stack base) of the current task. diff --git a/tools/test_apps/system/panic/panic_tests.py b/tools/test_apps/system/panic/panic_tests.py index fde69a5787..c890f4ae7d 100644 --- a/tools/test_apps/system/panic/panic_tests.py +++ b/tools/test_apps/system/panic/panic_tests.py @@ -51,11 +51,13 @@ def task_wdt_inner(env, test_name): dut.expect_backtrace() dut.expect_elf_sha256() dut.expect_none('Guru Meditation') - test_common(dut, test_name, expected_backtrace=[ - # Backtrace interrupted when abort is called, IDF-842. - # Task WDT calls abort internally. - 'panic_abort', 'esp_system_abort' - ]) + if ('gdbstub' in test_name): + test_common(dut, test_name, expected_backtrace=[ + # Backtrace interrupted when abort is called, IDF-842 + 'panic_abort', 'esp_system_abort' + ]) + else: + test_common(dut, test_name) def int_wdt_inner(env, test_name): @@ -103,10 +105,40 @@ def abort_inner(env, test_name): dut.expect_backtrace() dut.expect_elf_sha256() dut.expect_none('Guru Meditation', 'Re-entered core dump') - test_common(dut, test_name, expected_backtrace=[ - # Backtrace interrupted when abort is called, IDF-842 - 'panic_abort', 'esp_system_abort' - ]) + if ('gdbstub' in test_name): + test_common(dut, test_name, expected_backtrace=[ + # Backtrace interrupted when abort is called, IDF-842 + 'panic_abort', 'esp_system_abort' + ]) + else: + test_common(dut, test_name) + + +def abort_cached_disabled_inner(env, test_name): + with get_dut(env, test_name, 'test_abort_cache_disabled') as dut: + dut.expect(re.compile(r'abort\(\) was called at PC [0-9xa-f]+ on core 0')) + dut.expect_backtrace() + dut.expect_elf_sha256() + dut.expect_none('Guru Meditation', 'Re-entered core dump') + test_common(dut, test_name) + + +def assert_inner(env, test_name): + with get_dut(env, test_name, 'test_assert') as dut: + dut.expect(re.compile(r'(assert failed:[\s\w\(\)]*?\s[\.\w\/]*\.(?:c|cpp|h|hpp):\d*.*)')) + dut.expect_backtrace() + dut.expect_elf_sha256() + dut.expect_none('Guru Meditation', 'Re-entered core dump') + test_common(dut, test_name) + + +def assert_cached_disabled_inner(env, test_name): + with get_dut(env, test_name, 'test_assert_cache_disabled') as dut: + dut.expect(re.compile(r'(assert failed: [0-9xa-fA-F]+.*)')) + dut.expect_backtrace() + dut.expect_elf_sha256() + dut.expect_none('Guru Meditation', 'Re-entered core dump') + test_common(dut, test_name) def storeprohibited_inner(env, test_name): diff --git a/tools/unit-test-app/unit_test.py b/tools/unit-test-app/unit_test.py index bb5ee78887..7a69dbd63d 100755 --- a/tools/unit-test-app/unit_test.py +++ b/tools/unit-test-app/unit_test.py @@ -37,6 +37,7 @@ RESET_PATTERN = re.compile(r'(rst:0x[0-9a-fA-F]*\s\([\w].*?\),boot:0x[0-9a-fA-F] EXCEPTION_PATTERN = re.compile(r"(Guru Meditation Error: Core\s+\d panic'ed \([\w].*?\))") ABORT_PATTERN = re.compile(r'(abort\(\) was called at PC 0x[a-fA-F\d]{8} on core \d)') +ASSERT_PATTERN = re.compile(r'(assert failed: .*)') FINISH_PATTERN = re.compile(r'1 Tests (\d) Failures (\d) Ignored') END_LIST_STR = r'\r?\nEnter test for running' TEST_PATTERN = re.compile(r'\((\d+)\)\s+"([^"]+)" ([^\r\n]+)\r?\n(' + END_LIST_STR + r')?') @@ -268,6 +269,7 @@ def run_one_normal_case(dut, one_case, junit_test_case): dut.expect_any((RESET_PATTERN, handle_exception_reset), (EXCEPTION_PATTERN, handle_exception_reset), (ABORT_PATTERN, handle_exception_reset), + (ASSERT_PATTERN, handle_exception_reset), (FINISH_PATTERN, handle_test_finish), (UT_APP_BOOT_UP_DONE, handle_reset_finish), timeout=timeout_value) @@ -622,6 +624,7 @@ def run_one_multiple_stage_case(dut, one_case, junit_test_case): dut.expect_any((RESET_PATTERN, handle_exception_reset), (EXCEPTION_PATTERN, handle_exception_reset), (ABORT_PATTERN, handle_exception_reset), + (ASSERT_PATTERN, handle_exception_reset), (FINISH_PATTERN, handle_test_finish), (UT_APP_BOOT_UP_DONE, handle_next_stage), timeout=timeout_value)