Merge branch 'esp32c3/override_assert_v4.3' into 'release/v4.3'

newlib: Override __assert and __assert_func (v4.3)

See merge request espressif/esp-idf!14685

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;

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;

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);

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 "$<TARGET_FILE:${newlib
 
 set_source_files_properties(heap.c PROPERTIES COMPILE_FLAGS -fno-builtin)
 
-# Forces the linker to include heap, syscall, pthread and retargetable locks from this component,
+# Forces the linker to include heap, syscall, pthread, assert, and retargetable locks from this component,
 # instead of the implementations provided by newlib.
 list(APPEND EXTRA_LINK_FLAGS "-u newlib_include_heap_impl")
 list(APPEND EXTRA_LINK_FLAGS "-u newlib_include_syscalls_impl")
 list(APPEND EXTRA_LINK_FLAGS "-u newlib_include_pthread_impl")
+list(APPEND EXTRA_LINK_FLAGS "-u newlib_include_assert_impl")
 target_link_libraries(${COMPONENT_LIB} INTERFACE "${EXTRA_LINK_FLAGS}")
 
 if(CONFIG_NEWLIB_NANO_FORMAT)

components/newlib/assert.c

@@ -0,0 +1,97 @@
+// Copyright 2021 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.
+
+#include <string.h>
+#include <sys/param.h>
+#include "esp_system.h"
+#include "esp_spi_flash.h"
+#include "soc/soc_memory_layout.h"
+
+#define ASSERT_STR              "assert failed: "
+#define CACHE_DISABLED_STR      "<cached disabled>"
+
+#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)
+{
+}

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
 

components/newlib/newlib.lf

@@ -3,3 +3,4 @@ archive: libnewlib.a
 entries:
   heap (noflash)
   abort (noflash)
+  assert (noflash)

components/newlib/platform_include/assert.h

@@ -19,23 +19,34 @@
 #pragma once
 #include <sdkconfig.h>
 #include <stdlib.h>
-#include "esp_compiler.h"
+#include <stdint.h>
 
 #include_next <assert.h>
 
-#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

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:])

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.

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):

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)