From bf6ddf25578bc79bb0be137c2c3ee22696f0ca46 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 10:05:42 +1100 Subject: [PATCH 1/3] bootloader: Allow 'silent assert' config to work in bootloader Requires adding the 'newlib' component to the bootloader project, for platform_include header. --- components/bootloader/subproject/CMakeLists.txt | 5 +++-- components/newlib/CMakeLists.txt | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/components/bootloader/subproject/CMakeLists.txt b/components/bootloader/subproject/CMakeLists.txt index 2c507035c7e..5975021d347 100644 --- a/components/bootloader/subproject/CMakeLists.txt +++ b/components/bootloader/subproject/CMakeLists.txt @@ -28,10 +28,11 @@ set(COMPONENTS micro-ecc main efuse - esp_system) + esp_system + newlib) set(BOOTLOADER_BUILD 1) include("${IDF_PATH}/tools/cmake/project.cmake") -set(common_req log esp_rom esp_common esp_hw_support hal) +set(common_req log esp_rom esp_common esp_hw_support hal newlib) if(LEGACY_INCLUDE_COMMON_HEADERS) list(APPEND common_req soc hal) endif() diff --git a/components/newlib/CMakeLists.txt b/components/newlib/CMakeLists.txt index 0f869ed475b..924a27212bf 100644 --- a/components/newlib/CMakeLists.txt +++ b/components/newlib/CMakeLists.txt @@ -1,3 +1,9 @@ +if(BOOTLOADER_BUILD) + # Bootloader builds need the platform_include directory (for assert.h), but nothing else + idf_component_register(INCLUDE_DIRS platform_include) + return() +endif() + set(srcs "abort.c" "heap.c" From 73d40cb813ddf18135e254bc5502aec435425ef1 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 10:06:36 +1100 Subject: [PATCH 2/3] lwip: Support silent assertion configuration If silent assert configuration is enabled, LWIP asserts are now 'silent' also. Also updates KConfig to note that LWIP asserts are also disabled when asserts are disabled globally (this was already the behaviour, but the config item suggested otherwise.) Progress towards https://github.com/espressif/esp-idf/issues/5873 --- components/lwip/Kconfig | 8 ++++++-- components/lwip/port/esp32/include/arch/cc.h | 13 ++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/components/lwip/Kconfig b/components/lwip/Kconfig index d9324f83a03..b9e71131e74 100644 --- a/components/lwip/Kconfig +++ b/components/lwip/Kconfig @@ -757,9 +757,13 @@ menu "LWIP" config LWIP_ESP_LWIP_ASSERT bool "Enable LWIP ASSERT checks" default y + depends on !COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE help - Enable this option allows lwip to check assert. - It is recommended to keep it open, do not close it. + Enable this option keeps LWIP assertion checks enabled. + It is recommended to keep this option enabled. + + If asserts are disabled for the entire project, they are also disabled + for LWIP and this option is ignored. menu "Hooks" diff --git a/components/lwip/port/esp32/include/arch/cc.h b/components/lwip/port/esp32/include/arch/cc.h index e74efa967f9..78935f3b5f4 100644 --- a/components/lwip/port/esp32/include/arch/cc.h +++ b/components/lwip/port/esp32/include/arch/cc.h @@ -75,13 +75,20 @@ typedef int sys_prot_t; #include #define LWIP_PLATFORM_DIAG(x) do {printf x;} while(0) -// __assert_func is the assertion failure handler from newlib, defined in assert.h -#define LWIP_PLATFORM_ASSERT(message) __assert_func(__FILE__, __LINE__, __ASSERT_FUNC, message) #ifdef NDEBUG -#define LWIP_NOASSERT + +#define LWIP_NOASSERT 1 + #else // Assertions enabled +#if CONFIG_OPTIMIZATION_ASSERTIONS_SILENT +#define LWIP_PLATFORM_ASSERT(message) abort() +#else +// __assert_func is the assertion failure handler from newlib, defined in assert.h +#define LWIP_PLATFORM_ASSERT(message) __assert_func(__FILE__, __LINE__, __ASSERT_FUNC, message) +#endif + // If assertions are on, the default LWIP_ERROR handler behaviour is to // abort w/ an assertion failure. Don't do this, instead just print the error (if LWIP_DEBUG is set) // and run the handler (same as the LWIP_ERROR behaviour if LWIP_NOASSERT is set). From e9e2b68587f9dd6b342fe40a39517bf2fa06c19a Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 15:13:50 +1100 Subject: [PATCH 3/3] freertos: Use the standard assert() function for configASSERT Unless the option for "assert and keep running" is enabled. This means that silent asserts now work for FreeRTOS, and disabling asserts now also disables them in FreeRTOS without needing a separate config change. Related to https://github.com/espressif/esp-idf/issues/6306 --- components/freertos/Kconfig | 9 ++++++++- .../port/riscv/include/freertos/FreeRTOSConfig.h | 10 +++------- .../port/xtensa/include/freertos/FreeRTOSConfig.h | 10 +++------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/components/freertos/Kconfig b/components/freertos/Kconfig index ad975c6510e..25de7e73c79 100644 --- a/components/freertos/Kconfig +++ b/components/freertos/Kconfig @@ -141,18 +141,25 @@ menu "FreeRTOS" choice FREERTOS_ASSERT prompt "FreeRTOS assertions" - default FREERTOS_ASSERT_FAIL_ABORT + default FREERTOS_ASSERT_FAIL_ABORT if !COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE + default FREERTOS_ASSERT_DISABLE if COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE help Failed FreeRTOS configASSERT() assertions can be configured to behave in different ways. + By default these behave the same as the global project assert settings. + config FREERTOS_ASSERT_FAIL_ABORT bool "abort() on failed assertions" + depends on !COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE help If a FreeRTOS configASSERT() fails, FreeRTOS will abort() and halt execution. The panic handler can be configured to handle the outcome of an abort() in different ways. + If assertions are disabled for the entire project, they are also + disabled in FreeRTOS and this option is unavailable. + config FREERTOS_ASSERT_FAIL_PRINT_CONTINUE bool "Print and continue failed assertions" help diff --git a/components/freertos/port/riscv/include/freertos/FreeRTOSConfig.h b/components/freertos/port/riscv/include/freertos/FreeRTOSConfig.h index 83d8e8bfe8d..f9229d774ad 100644 --- a/components/freertos/port/riscv/include/freertos/FreeRTOSConfig.h +++ b/components/freertos/port/riscv/include/freertos/FreeRTOSConfig.h @@ -88,7 +88,7 @@ /* configASSERT behaviour */ #ifndef __ASSEMBLER__ -#include /* for abort() */ +#include #include "esp32c3/rom/ets_sys.h" #if defined(CONFIG_FREERTOS_ASSERT_DISABLE) @@ -98,12 +98,8 @@ esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ __FUNCTION__); \ } -#else /* CONFIG_FREERTOS_ASSERT_FAIL_ABORT */ -#define configASSERT(a) if (unlikely(!(a))) { \ - esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ - __FUNCTION__); \ - abort(); \ - } +#elif defined(CONFIG_FREERTOS_ASSERT_FAIL_ABORT) +#define configASSERT(a) assert(a) #endif #if CONFIG_FREERTOS_ASSERT_ON_UNTESTED_FUNCTION diff --git a/components/freertos/port/xtensa/include/freertos/FreeRTOSConfig.h b/components/freertos/port/xtensa/include/freertos/FreeRTOSConfig.h index 7b7d69437ff..e116fad82f0 100644 --- a/components/freertos/port/xtensa/include/freertos/FreeRTOSConfig.h +++ b/components/freertos/port/xtensa/include/freertos/FreeRTOSConfig.h @@ -119,7 +119,7 @@ int xt_clock_freq(void) __attribute__((deprecated)); /* configASSERT behaviour */ #ifndef __ASSEMBLER__ -#include /* for abort() */ +#include #include "esp_rom_sys.h" #if CONFIG_IDF_TARGET_ESP32 #include "esp32/rom/ets_sys.h" // will be removed in idf v5.0 @@ -138,12 +138,8 @@ int xt_clock_freq(void) __attribute__((deprecated)); esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ __FUNCTION__); \ } -#else /* CONFIG_FREERTOS_ASSERT_FAIL_ABORT */ -#define configASSERT(a) if (unlikely(!(a))) { \ - esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ - __FUNCTION__); \ - abort(); \ - } +#elif defined(CONFIG_FREERTOS_ASSERT_FAIL_ABORT) +#define configASSERT(a) assert(a) #endif #if CONFIG_FREERTOS_ASSERT_ON_UNTESTED_FUNCTION