From d6f4d99d937eda1a1a677ea1782852976262ca20 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 12 Feb 2021 16:01:05 +1100 Subject: [PATCH 01/13] core system: Fix warnings in compilation when assertions are disabled Adds a CI config for hello world that sets this, to catch future regressions --- .../bootloader_support/src/idf/bootloader_sha.c | 2 ++ components/bt/CMakeLists.txt | 10 ++++++++++ components/bt/common/osi/hash_map.c | 1 + components/bt/controller/esp32/bt.c | 3 ++- components/bt/controller/esp32c3/bt.c | 3 ++- components/bt/controller/esp32s3/bt.c | 4 ++-- components/bt/host/bluedroid/hci/hci_packet_parser.c | 8 ++++---- components/bt/host/bluedroid/hci/packet_fragmenter.c | 2 +- components/cxx/cxx_guards.cpp | 7 +++++++ components/driver/sdio_slave.c | 10 ++++++---- components/driver/spi_master.c | 1 + components/driver/spi_slave_hd.c | 2 +- components/driver/twai.c | 2 ++ components/efuse/esp32c3/esp_efuse_rtc_calib.c | 10 ++++------ components/efuse/src/esp_efuse_utility.c | 7 +++---- components/esp32/crosscore_int.c | 2 +- components/esp32/dport_access.c | 1 + components/esp32/esp_himem.c | 2 +- components/esp32c3/crosscore_int.c | 3 +-- components/esp32s3/crosscore_int.c | 6 ++---- components/esp_adc_cal/esp_adc_cal_esp32s2.c | 2 +- components/esp_hw_support/port/esp32c3/rtc_init.c | 3 +-- components/esp_ipc/ipc.c | 1 + components/esp_pm/pm_impl.c | 2 +- components/esp_system/port/soc/esp32/clk.c | 4 +++- components/esp_system/port/soc/esp32c3/clk.c | 4 +++- components/esp_system/port/soc/esp32s2/clk.c | 4 +++- components/esp_system/port/soc/esp32s3/clk.c | 4 +++- components/esp_system/sleep_modes.c | 4 ++-- components/esp_system/startup.c | 3 ++- components/freertos/port/port_common.c | 1 + .../port/riscv/include/freertos/FreeRTOSConfig.h | 9 +++++---- components/freertos/port/riscv/port.c | 3 +-- .../port/xtensa/include/freertos/FreeRTOSConfig.h | 11 ++++++----- components/freertos/tasks.c | 1 + components/hal/esp32s2/usbh_hal.c | 2 ++ components/hal/sdio_slave_hal.c | 2 +- components/heap/heap_caps.c | 2 +- components/libsodium/CMakeLists.txt | 5 +++++ components/lwip/port/esp32/freertos/sys_arch.c | 7 +++++++ components/mbedtls/port/sha/esp_sha.c | 2 +- components/nvs_flash/src/nvs_storage.cpp | 3 +-- components/protobuf-c/CMakeLists.txt | 5 +++++ components/spi_flash/cache_utils.c | 4 ++-- .../wpa_supplicant/src/esp_supplicant/esp_wpa_main.c | 1 + 45 files changed, 114 insertions(+), 61 deletions(-) diff --git a/components/bootloader_support/src/idf/bootloader_sha.c b/components/bootloader_support/src/idf/bootloader_sha.c index 40308b667f..1aaa6708ef 100644 --- a/components/bootloader_support/src/idf/bootloader_sha.c +++ b/components/bootloader_support/src/idf/bootloader_sha.c @@ -39,6 +39,7 @@ void bootloader_sha256_data(bootloader_sha256_handle_t handle, const void *data, mbedtls_sha256_context *ctx = (mbedtls_sha256_context *)handle; int ret = mbedtls_sha256_update_ret(ctx, data, data_len); assert(ret == 0); + (void)ret; } void bootloader_sha256_finish(bootloader_sha256_handle_t handle, uint8_t *digest) @@ -48,6 +49,7 @@ void bootloader_sha256_finish(bootloader_sha256_handle_t handle, uint8_t *digest if (digest != NULL) { int ret = mbedtls_sha256_finish_ret(ctx, digest); assert(ret == 0); + (void)ret; } mbedtls_sha256_free(ctx); free(handle); diff --git a/components/bt/CMakeLists.txt b/components/bt/CMakeLists.txt index aa97d41ee8..0b2a2bdc54 100644 --- a/components/bt/CMakeLists.txt +++ b/components/bt/CMakeLists.txt @@ -599,3 +599,13 @@ if(CONFIG_BT_ENABLED) target_link_libraries(${COMPONENT_LIB} PUBLIC btdm_app btbb) endif() endif() + +if(CONFIG_BT_NIMBLE_MESH) + set_source_files_properties("host/nimble/nimble/nimble/host/mesh/src/net.c" + PROPERTIES COMPILE_FLAGS -Wno-type-limits) +endif() + +if(CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE AND CONFIG_BT_NIMBLE_ENABLED) + # some variables in NimBLE are only used by asserts + target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-unused-but-set-variable -Wno-unused-variable) +endif() diff --git a/components/bt/common/osi/hash_map.c b/components/bt/common/osi/hash_map.c index 841e84f6eb..59942bfd41 100644 --- a/components/bt/common/osi/hash_map.c +++ b/components/bt/common/osi/hash_map.c @@ -144,6 +144,7 @@ bool hash_map_set(hash_map_t *hash_map, const void *key, void *data) // Calls hash_map callback to delete the hash_map_entry. bool rc = list_remove(hash_bucket_list, hash_map_entry); assert(rc == true); + (void)rc; } else { hash_map->hash_size++; } diff --git a/components/bt/controller/esp32/bt.c b/components/bt/controller/esp32/bt.c index afe291c607..a829a99743 100644 --- a/components/bt/controller/esp32/bt.c +++ b/components/bt/controller/esp32/bt.c @@ -1309,7 +1309,8 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg) btdm_lpclk_sel = BTDM_LPCLK_SEL_XTAL; // set default value #endif - bool select_src_ret, set_div_ret; + bool select_src_ret __attribute__((unused)); + bool set_div_ret __attribute__((unused)); if (btdm_lpclk_sel == BTDM_LPCLK_SEL_XTAL) { select_src_ret = btdm_lpclk_select_src(BTDM_LPCLK_SEL_XTAL); set_div_ret = btdm_lpclk_set_div(rtc_clk_xtal_freq_get() * 2 - 1); diff --git a/components/bt/controller/esp32c3/bt.c b/components/bt/controller/esp32c3/bt.c index b8632bccac..45c9a95d1d 100644 --- a/components/bt/controller/esp32c3/bt.c +++ b/components/bt/controller/esp32c3/bt.c @@ -1050,7 +1050,8 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg) s_lp_cntl.lpclk_sel = BTDM_LPCLK_SEL_XTAL; // set default value #endif - bool select_src_ret, set_div_ret; + bool select_src_ret __attribute__((unused)); + bool set_div_ret __attribute__((unused)); if (s_lp_cntl.lpclk_sel == BTDM_LPCLK_SEL_XTAL) { select_src_ret = btdm_lpclk_select_src(BTDM_LPCLK_SEL_XTAL); set_div_ret = btdm_lpclk_set_div(rtc_clk_xtal_freq_get() * 2); diff --git a/components/bt/controller/esp32s3/bt.c b/components/bt/controller/esp32s3/bt.c index 7d13d62f21..71a0aacba8 100644 --- a/components/bt/controller/esp32s3/bt.c +++ b/components/bt/controller/esp32s3/bt.c @@ -988,8 +988,8 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg) cfg->sleep_clock = ESP_BT_SLEEP_CLOCK_FPGA_32K; ESP_LOGW(BTDM_LOG_TAG, "%s sleep clock overwrite on FPGA", __func__); #endif - bool select_src_ret = false; - bool set_div_ret = false; + bool select_src_ret __attribute__((unused)); + bool set_div_ret __attribute__((unused)); if (cfg->sleep_clock == ESP_BT_SLEEP_CLOCK_MAIN_XTAL) { select_src_ret = btdm_lpclk_select_src(BTDM_LPCLK_SEL_XTAL); set_div_ret = btdm_lpclk_set_div(rtc_clk_xtal_freq_get() * 2); diff --git a/components/bt/host/bluedroid/hci/hci_packet_parser.c b/components/bt/host/bluedroid/hci/hci_packet_parser.c index 40c736fe98..ba756f0bba 100644 --- a/components/bt/host/bluedroid/hci/hci_packet_parser.c +++ b/components/bt/host/bluedroid/hci/hci_packet_parser.c @@ -227,19 +227,19 @@ static uint8_t *read_command_complete_header( uint8_t *stream = response->data + response->offset; // Read the event header - uint8_t event_code; - uint8_t parameter_length; + uint8_t event_code __attribute__((unused)); + uint8_t parameter_length __attribute__((unused)); STREAM_TO_UINT8(event_code, stream); STREAM_TO_UINT8(parameter_length, stream); - const size_t parameter_bytes_we_read_here = 4; + const size_t parameter_bytes_we_read_here __attribute__((unused)) = 4; // Check the event header values against what we expect assert(event_code == HCI_COMMAND_COMPLETE_EVT); assert(parameter_length >= (parameter_bytes_we_read_here + minimum_bytes_after)); // Read the command complete header - command_opcode_t opcode; + command_opcode_t opcode __attribute__((unused)); uint8_t status; STREAM_SKIP_UINT8(stream); // skip the number of hci command packets field STREAM_TO_UINT16(opcode, stream); diff --git a/components/bt/host/bluedroid/hci/packet_fragmenter.c b/components/bt/host/bluedroid/hci/packet_fragmenter.c index a562e3c45a..78b6151e51 100644 --- a/components/bt/host/bluedroid/hci/packet_fragmenter.c +++ b/components/bt/host/bluedroid/hci/packet_fragmenter.c @@ -141,7 +141,7 @@ static void reassemble_and_dispatch(BT_HDR *packet) uint8_t *stream = packet->data + packet->offset; uint16_t handle; uint16_t l2cap_length; - uint16_t acl_length; + uint16_t acl_length __attribute__((unused)); STREAM_TO_UINT16(handle, stream); STREAM_TO_UINT16(acl_length, stream); diff --git a/components/cxx/cxx_guards.cpp b/components/cxx/cxx_guards.cpp index 640cf17756..8580a26cc9 100644 --- a/components/cxx/cxx_guards.cpp +++ b/components/cxx/cxx_guards.cpp @@ -82,6 +82,7 @@ static void wait_for_guard_obj(guard_t* g) do { auto result = xSemaphoreGive(s_static_init_mutex); assert(result); + static_cast(result); /* Task may be preempted here, but this isn't a problem, * as the semaphore will be given exactly the s_static_init_waiting_count * number of times; eventually the current task will execute next statement, @@ -140,6 +141,7 @@ extern "C" int __cxa_guard_acquire(__guard* pg) */ auto result = xSemaphoreTake(s_static_init_mutex, portMAX_DELAY); assert(result); + static_cast(result); if (g->pending) { /* Another task is doing initialization at the moment; wait until it calls * __cxa_guard_release or __cxa_guard_abort @@ -168,6 +170,7 @@ extern "C" int __cxa_guard_acquire(__guard* pg) if (scheduler_started) { auto result = xSemaphoreGive(s_static_init_mutex); assert(result); + static_cast(result); } return ret; } @@ -179,6 +182,7 @@ extern "C" void __cxa_guard_release(__guard* pg) if (scheduler_started) { auto result = xSemaphoreTake(s_static_init_mutex, portMAX_DELAY); assert(result); + static_cast(result); } assert(g->pending && "tried to release a guard which wasn't acquired"); g->pending = 0; @@ -189,6 +193,7 @@ extern "C" void __cxa_guard_release(__guard* pg) signal_waiting_tasks(); auto result = xSemaphoreGive(s_static_init_mutex); assert(result); + static_cast(result); } } @@ -199,6 +204,7 @@ extern "C" void __cxa_guard_abort(__guard* pg) if (scheduler_started) { auto result = xSemaphoreTake(s_static_init_mutex, portMAX_DELAY); assert(result); + static_cast(result); } assert(!g->ready && "tried to abort a guard which is ready"); assert(g->pending && "tried to release a guard which is not acquired"); @@ -208,6 +214,7 @@ extern "C" void __cxa_guard_abort(__guard* pg) signal_waiting_tasks(); auto result = xSemaphoreGive(s_static_init_mutex); assert(result); + static_cast(result); } } diff --git a/components/driver/sdio_slave.c b/components/driver/sdio_slave.c index 8329d2976f..3f9ffd9fa0 100644 --- a/components/driver/sdio_slave.c +++ b/components/driver/sdio_slave.c @@ -375,6 +375,7 @@ void sdio_slave_deinit(void) } esp_err_t ret = esp_intr_free(context.intr_handle); assert(ret==ESP_OK); + (void)ret; context.intr_handle = NULL; deinit_context(); } @@ -532,7 +533,7 @@ static void sdio_intr_send(void* arg) uint32_t returned_cnt; if (sdio_slave_hal_send_eof_happened(context.hal)) { - portBASE_TYPE ret = pdTRUE; + portBASE_TYPE ret __attribute__((unused)); esp_err_t err; while (1) { @@ -549,7 +550,7 @@ static void sdio_intr_send(void* arg) } //get_next_finished_arg returns the total amount of returned descs. for(size_t i = 0; i < returned_cnt; i++) { - portBASE_TYPE ret = xSemaphoreGiveFromISR(context.remain_cnt, &yield); + ret = xSemaphoreGiveFromISR(context.remain_cnt, &yield); assert(ret == pdTRUE); } } @@ -603,16 +604,17 @@ esp_err_t sdio_slave_transmit(uint8_t* addr, size_t len) static esp_err_t send_flush_data(void) { esp_err_t err; + portBASE_TYPE ret __attribute__((unused)); while (1) { void *finished_arg; uint32_t return_cnt = 0; err = sdio_slave_hal_send_flush_next_buffer(context.hal, &finished_arg, &return_cnt); if (err == ESP_OK) { - portBASE_TYPE ret = xQueueSend(context.ret_queue, &finished_arg, portMAX_DELAY); + ret = xQueueSend(context.ret_queue, &finished_arg, portMAX_DELAY); assert(ret == pdTRUE); for (size_t i = 0; i < return_cnt; i++) { - portBASE_TYPE ret = xSemaphoreGive(context.remain_cnt); + ret = xSemaphoreGive(context.remain_cnt); assert(ret == pdTRUE); } } else { diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index 7eb160cb99..e6e093afb1 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -915,6 +915,7 @@ void SPI_MASTER_ISR_ATTR spi_device_release_bus(spi_device_t *dev) host->device_acquiring_lock = NULL; esp_err_t ret = spi_bus_lock_acquire_end(dev->dev_lock); assert(ret == ESP_OK); + (void) ret; } esp_err_t SPI_MASTER_ISR_ATTR spi_device_polling_start(spi_device_handle_t handle, spi_transaction_t *trans_desc, TickType_t ticks_to_wait) diff --git a/components/driver/spi_slave_hd.c b/components/driver/spi_slave_hd.c index 7e21b4488f..8f0f3cd9cc 100644 --- a/components/driver/spi_slave_hd.c +++ b/components/driver/spi_slave_hd.c @@ -380,7 +380,7 @@ static IRAM_ATTR void spi_slave_hd_intr_append(void *arg) spi_slave_hd_callback_config_t *callback = &host->callback; spi_slave_hd_hal_context_t *hal = &host->hal; BaseType_t awoken = pdFALSE; - BaseType_t ret; + BaseType_t ret __attribute__((unused)); bool tx_done = false; bool rx_done = false; diff --git a/components/driver/twai.c b/components/driver/twai.c index 967082fb5e..bf152111a0 100644 --- a/components/driver/twai.c +++ b/components/driver/twai.c @@ -410,6 +410,7 @@ esp_err_t twai_driver_install(const twai_general_config_t *g_config, const twai_ periph_module_enable(PERIPH_TWAI_MODULE); //Enable APB CLK to TWAI peripheral bool init = twai_hal_init(&twai_context); assert(init); + (void)init; twai_hal_configure(&twai_context, t_config, f_config, DRIVER_DEFAULT_INTERRUPTS, g_config->clkout_divider); TWAI_EXIT_CRITICAL(); @@ -536,6 +537,7 @@ esp_err_t twai_transmit(const twai_message_t *message, TickType_t ticks_to_wait) //Manually start a transmission int res = xQueueReceive(p_twai_obj->tx_queue, &tx_frame, 0); assert(res == pdTRUE); + (void)res; twai_hal_set_tx_buffer_and_transmit(&twai_context, &tx_frame); p_twai_obj->tx_msg_count++; ret = ESP_OK; diff --git a/components/efuse/esp32c3/esp_efuse_rtc_calib.c b/components/efuse/esp32c3/esp_efuse_rtc_calib.c index 844463b6f3..afda42ca26 100644 --- a/components/efuse/esp32c3/esp_efuse_rtc_calib.c +++ b/components/efuse/esp32c3/esp_efuse_rtc_calib.c @@ -42,8 +42,7 @@ uint16_t esp_efuse_rtc_calib_get_init_code(int version, int atten) assert(init_code_size == 10); uint32_t init_code = 0; - esp_err_t err = esp_efuse_read_field_blob(init_code_efuse, &init_code, init_code_size); - assert(err == ESP_OK); + ESP_ERROR_CHECK(esp_efuse_read_field_blob(init_code_efuse, &init_code, init_code_size)); return init_code + 1000; // version 1 logic } @@ -71,12 +70,10 @@ esp_err_t esp_efuse_rtc_calib_get_cal_voltage(int version, int atten, uint32_t* calib_vol_expected_mv = 1370; } - int cal_vol_size = esp_efuse_get_field_size(cal_vol_efuse); - assert(cal_vol_size == 10); + assert(cal_vol_efuse[0]->bit_count == 10); uint32_t cal_vol = 0; - esp_err_t err = esp_efuse_read_field_blob(cal_vol_efuse, &cal_vol, cal_vol_size) & 0x3FF; - assert(err == ESP_OK); + ESP_ERROR_CHECK(esp_efuse_read_field_blob(cal_vol_efuse, &cal_vol, cal_vol_efuse[0]->bit_count)); *out_digi = 2000 + ((cal_vol & BIT(9))? -(cal_vol & ~BIT9): cal_vol); *out_vol_mv = calib_vol_expected_mv; @@ -94,6 +91,7 @@ float esp_efuse_rtc_calib_get_cal_temp(int version) uint32_t cal_temp = 0; esp_err_t err = esp_efuse_read_field_blob(cal_temp_efuse, &cal_temp, cal_temp_size); assert(err == ESP_OK); + (void)err; // BIT(8) stands for sign: 1: negtive, 0: positive return ((cal_temp & BIT(8)) != 0)? -(uint8_t)cal_temp: (uint8_t)cal_temp; } diff --git a/components/efuse/src/esp_efuse_utility.c b/components/efuse/src/esp_efuse_utility.c index a6a303f72e..931b5167f5 100644 --- a/components/efuse/src/esp_efuse_utility.c +++ b/components/efuse/src/esp_efuse_utility.c @@ -228,8 +228,7 @@ esp_err_t esp_efuse_utility_write_reg(esp_efuse_block_t efuse_block, unsigned in uint32_t esp_efuse_utility_read_reg(esp_efuse_block_t blk, unsigned int num_reg) { assert(blk >= 0 && blk < EFUSE_BLK_MAX); - unsigned int max_num_reg = (range_read_addr_blocks[blk].end - range_read_addr_blocks[blk].start) / sizeof(uint32_t); - assert(num_reg <= max_num_reg); + assert(num_reg <= (range_read_addr_blocks[blk].end - range_read_addr_blocks[blk].start) / sizeof(uint32_t)); uint32_t value; #ifdef CONFIG_EFUSE_VIRTUAL value = virt_blocks[blk][num_reg]; @@ -245,8 +244,8 @@ uint32_t esp_efuse_utility_read_reg(esp_efuse_block_t blk, unsigned int num_reg) static void write_reg(esp_efuse_block_t blk, unsigned int num_reg, uint32_t value) { assert(blk >= 0 && blk < EFUSE_BLK_MAX); - unsigned int max_num_reg = (range_read_addr_blocks[blk].end - range_read_addr_blocks[blk].start) / sizeof(uint32_t); - assert(num_reg <= max_num_reg); + assert(num_reg <= (range_read_addr_blocks[blk].end - range_read_addr_blocks[blk].start) / sizeof(uint32_t)); + uint32_t addr_wr_reg = range_write_addr_blocks[blk].start + num_reg * 4; uint32_t reg_to_write = REG_READ(addr_wr_reg) | value; // The register can be written in parts so we combine the new value with the one already available. diff --git a/components/esp32/crosscore_int.c b/components/esp32/crosscore_int.c index af13ad7e3f..309e12058e 100644 --- a/components/esp32/crosscore_int.c +++ b/components/esp32/crosscore_int.c @@ -84,7 +84,7 @@ void esp_crosscore_int_init(void) { portENTER_CRITICAL(&reason_spinlock); reason[xPortGetCoreID()]=0; portEXIT_CRITICAL(&reason_spinlock); - esp_err_t err; + esp_err_t err __attribute__((unused)); if (xPortGetCoreID()==0) { err = esp_intr_alloc(ETS_FROM_CPU_INTR0_SOURCE, ESP_INTR_FLAG_IRAM, esp_crosscore_isr, (void*)&reason[0], NULL); } else { diff --git a/components/esp32/dport_access.c b/components/esp32/dport_access.c index 261527d317..979553abf8 100644 --- a/components/esp32/dport_access.c +++ b/components/esp32/dport_access.c @@ -173,6 +173,7 @@ void esp_dport_access_int_init(void) #ifndef CONFIG_FREERTOS_UNICORE portBASE_TYPE res = xTaskCreatePinnedToCore(&dport_access_init_core, "dport", configMINIMAL_STACK_SIZE, NULL, 5, NULL, xPortGetCoreID()); assert(res == pdTRUE); + (void)res; #endif } diff --git a/components/esp32/esp_himem.c b/components/esp32/esp_himem.c index be6d8d9222..d2f413092d 100644 --- a/components/esp32/esp_himem.c +++ b/components/esp32/esp_himem.c @@ -110,7 +110,7 @@ static inline int rangeblock_idx_valid(int rangeblock_idx) static void set_bank(int virt_bank, int phys_bank, int ct) { - int r; + int r __attribute__((unused)); r = cache_sram_mmu_set( 0, 0, SOC_EXTRAM_DATA_LOW + CACHE_BLOCKSIZE * virt_bank, phys_bank * CACHE_BLOCKSIZE, 32, ct ); assert(r == 0); r = cache_sram_mmu_set( 1, 0, SOC_EXTRAM_DATA_LOW + CACHE_BLOCKSIZE * virt_bank, phys_bank * CACHE_BLOCKSIZE, 32, ct ); diff --git a/components/esp32c3/crosscore_int.c b/components/esp32c3/crosscore_int.c index dad7673969..e704b33914 100644 --- a/components/esp32c3/crosscore_int.c +++ b/components/esp32c3/crosscore_int.c @@ -66,8 +66,7 @@ void esp_crosscore_int_init(void) portENTER_CRITICAL(&reason_spinlock); reason[cpu_hal_get_core_id()] = 0; portEXIT_CRITICAL(&reason_spinlock); - esp_err_t err = esp_intr_alloc(ETS_FROM_CPU_INTR0_SOURCE, ESP_INTR_FLAG_IRAM, esp_crosscore_isr, (void *)&reason[0], NULL); - assert(err == ESP_OK); + ESP_ERROR_CHECK(esp_intr_alloc(ETS_FROM_CPU_INTR0_SOURCE, ESP_INTR_FLAG_IRAM, esp_crosscore_isr, (void *)&reason[0], NULL)); } static void IRAM_ATTR esp_crosscore_int_send(int core_id, uint32_t reason_mask) diff --git a/components/esp32s3/crosscore_int.c b/components/esp32s3/crosscore_int.c index ea8c45414a..ed963992ee 100644 --- a/components/esp32s3/crosscore_int.c +++ b/components/esp32s3/crosscore_int.c @@ -73,13 +73,11 @@ void esp_crosscore_int_init(void) portENTER_CRITICAL(&reason_spinlock); reason[cpu_hal_get_core_id()] = 0; portEXIT_CRITICAL(&reason_spinlock); - esp_err_t err; if (cpu_hal_get_core_id() == 0) { - err = esp_intr_alloc(ETS_FROM_CPU_INTR0_SOURCE, ESP_INTR_FLAG_IRAM, esp_crosscore_isr, (void *)&reason[0], NULL); + ESP_ERROR_CHECK(esp_intr_alloc(ETS_FROM_CPU_INTR0_SOURCE, ESP_INTR_FLAG_IRAM, esp_crosscore_isr, (void *)&reason[0], NULL)); } else { - err = esp_intr_alloc(ETS_FROM_CPU_INTR1_SOURCE, ESP_INTR_FLAG_IRAM, esp_crosscore_isr, (void *)&reason[1], NULL); + ESP_ERROR_CHECK(esp_intr_alloc(ETS_FROM_CPU_INTR1_SOURCE, ESP_INTR_FLAG_IRAM, esp_crosscore_isr, (void *)&reason[1], NULL)); } - assert(err == ESP_OK); } static void IRAM_ATTR esp_crosscore_int_send(int core_id, uint32_t reason_mask) diff --git a/components/esp_adc_cal/esp_adc_cal_esp32s2.c b/components/esp_adc_cal/esp_adc_cal_esp32s2.c index 08532ba451..f3d058ccdf 100644 --- a/components/esp_adc_cal/esp_adc_cal_esp32s2.c +++ b/components/esp_adc_cal/esp_adc_cal_esp32s2.c @@ -177,7 +177,7 @@ esp_adc_cal_value_t esp_adc_cal_characterize(adc_unit_t adc_num, uint32_t default_vref, esp_adc_cal_characteristics_t *chars) { - bool res; + bool res __attribute__((unused)); adc_calib_parsed_info efuse_parsed_data = {0}; // Check parameters assert((adc_num == ADC_UNIT_1) || (adc_num == ADC_UNIT_2)); diff --git a/components/esp_hw_support/port/esp32c3/rtc_init.c b/components/esp_hw_support/port/esp32c3/rtc_init.c index f6f952c04b..cfefb9ea79 100644 --- a/components/esp_hw_support/port/esp32c3/rtc_init.c +++ b/components/esp_hw_support/port/esp32c3/rtc_init.c @@ -193,8 +193,7 @@ static void set_ocode_by_efuse(int calib_version) assert(calib_version == 1); // use efuse ocode. uint32_t ocode; - esp_err_t err = esp_efuse_read_field_blob(ESP_EFUSE_OCODE, &ocode, 8); - assert(err == ESP_OK); + ESP_ERROR_CHECK(esp_efuse_read_field_blob(ESP_EFUSE_OCODE, &ocode, 8)); REGI2C_WRITE_MASK(I2C_ULP, I2C_ULP_EXT_CODE, ocode); REGI2C_WRITE_MASK(I2C_ULP, I2C_ULP_IR_FORCE_CODE, 1); } diff --git a/components/esp_ipc/ipc.c b/components/esp_ipc/ipc.c index 4ec94b16ec..7d63d6f87f 100644 --- a/components/esp_ipc/ipc.c +++ b/components/esp_ipc/ipc.c @@ -95,6 +95,7 @@ static void esp_ipc_init(void) portBASE_TYPE res = xTaskCreatePinnedToCore(ipc_task, task_name, CONFIG_ESP_IPC_TASK_STACK_SIZE, (void*) i, configMAX_PRIORITIES - 1, &s_ipc_task_handle[i], i); assert(res == pdTRUE); + (void)res; } } diff --git a/components/esp_pm/pm_impl.c b/components/esp_pm/pm_impl.c index 3e64133f12..2f9173f34b 100644 --- a/components/esp_pm/pm_impl.c +++ b/components/esp_pm/pm_impl.c @@ -288,7 +288,7 @@ esp_err_t esp_pm_configure(const void* vconfig) portENTER_CRITICAL(&s_switch_lock); - bool res = false; + bool res __attribute__((unused)); res = rtc_clk_cpu_freq_mhz_to_config(max_freq_mhz, &s_cpu_freq_by_mode[PM_MODE_CPU_MAX]); assert(res); res = rtc_clk_cpu_freq_mhz_to_config(apb_max_freq, &s_cpu_freq_by_mode[PM_MODE_APB_MAX]); diff --git a/components/esp_system/port/soc/esp32/clk.c b/components/esp_system/port/soc/esp32/clk.c index 720c0e9b26..ebd4e6de24 100644 --- a/components/esp_system/port/soc/esp32/clk.c +++ b/components/esp_system/port/soc/esp32/clk.c @@ -192,7 +192,9 @@ static void select_rtc_slow_clk(slow_clk_sel_t slow_clk) esp_rom_uart_tx_wait_idle(CONFIG_ESP_CONSOLE_UART_NUM); } - rtc_clk_cpu_freq_set_config(&new_config); + if (res) { + rtc_clk_cpu_freq_set_config(&new_config); + } // Re calculate the ccount to make time calculation correct. cpu_hal_set_cycle_count( (uint64_t)cpu_hal_get_cycle_count() * new_freq_mhz / old_freq_mhz ); diff --git a/components/esp_system/port/soc/esp32c3/clk.c b/components/esp_system/port/soc/esp32c3/clk.c index 2f9d42c589..3b442a2332 100644 --- a/components/esp_system/port/soc/esp32c3/clk.c +++ b/components/esp_system/port/soc/esp32c3/clk.c @@ -133,7 +133,9 @@ static const char *TAG = "clk"; // when switching APB frequency esp_rom_uart_tx_wait_idle(CONFIG_ESP_CONSOLE_UART_NUM); - rtc_clk_cpu_freq_set_config(&new_config); + if (res) { + rtc_clk_cpu_freq_set_config(&new_config); + } // Re calculate the ccount to make time calculation correct. cpu_hal_set_cycle_count( (uint64_t)cpu_hal_get_cycle_count() * new_freq_mhz / old_freq_mhz ); diff --git a/components/esp_system/port/soc/esp32s2/clk.c b/components/esp_system/port/soc/esp32s2/clk.c index 688e0ec076..bf8a12a4cb 100644 --- a/components/esp_system/port/soc/esp32s2/clk.c +++ b/components/esp_system/port/soc/esp32s2/clk.c @@ -136,7 +136,9 @@ static void select_rtc_slow_clk(slow_clk_sel_t slow_clk); esp_rom_uart_tx_wait_idle(CONFIG_ESP_CONSOLE_UART_NUM); } - rtc_clk_cpu_freq_set_config(&new_config); + if (res) { + rtc_clk_cpu_freq_set_config(&new_config); + } // Re calculate the ccount to make time calculation correct. cpu_hal_set_cycle_count( (uint64_t)cpu_hal_get_cycle_count() * new_freq_mhz / old_freq_mhz ); diff --git a/components/esp_system/port/soc/esp32s3/clk.c b/components/esp_system/port/soc/esp32s3/clk.c index 05b9df6bde..0478452f42 100644 --- a/components/esp_system/port/soc/esp32s3/clk.c +++ b/components/esp_system/port/soc/esp32s3/clk.c @@ -131,7 +131,9 @@ static void select_rtc_slow_clk(slow_clk_sel_t slow_clk); esp_rom_uart_tx_wait_idle(CONFIG_ESP_CONSOLE_UART_NUM); } - rtc_clk_cpu_freq_set_config(&new_config); + if (res) { + rtc_clk_cpu_freq_set_config(&new_config); + } // Re calculate the ccount to make time calculation correct. cpu_hal_set_cycle_count( (uint64_t)cpu_hal_get_cycle_count() * new_freq_mhz / old_freq_mhz ); diff --git a/components/esp_system/sleep_modes.c b/components/esp_system/sleep_modes.c index 3ead495f10..1cb5e1e127 100644 --- a/components/esp_system/sleep_modes.c +++ b/components/esp_system/sleep_modes.c @@ -927,7 +927,7 @@ touch_pad_t esp_sleep_get_touchpad_wakeup_status(void) touch_pad_t pad_num; esp_err_t ret = touch_pad_get_wakeup_status(&pad_num); //TODO 723diff commit id:fda9ada1b assert(ret == ESP_OK && "wakeup reason is RTC_TOUCH_TRIG_EN but SENS_TOUCH_MEAS_EN is zero"); - return pad_num; + return (ret == ESP_OK) ? pad_num : TOUCH_PAD_MAX; } #endif // SOC_TOUCH_SENSOR_NUM > 0 @@ -1097,7 +1097,7 @@ esp_err_t esp_deep_sleep_enable_gpio_wakeup(uint64_t gpio_pin_mask, esp_deepslee return ESP_ERR_INVALID_ARG; } err = gpio_deep_sleep_wakeup_enable(gpio_idx, intr_type); - + s_config.gpio_wakeup_mask |= BIT(gpio_idx); if (mode == ESP_GPIO_WAKEUP_GPIO_HIGH) { s_config.gpio_trigger_mode |= (mode << gpio_idx); diff --git a/components/esp_system/startup.c b/components/esp_system/startup.c index 43862691d5..ebe2d985f9 100644 --- a/components/esp_system/startup.c +++ b/components/esp_system/startup.c @@ -284,7 +284,7 @@ static void do_core_init(void) esp_secure_boot_init_checks(); #endif - esp_err_t err; + esp_err_t err __attribute__((unused)); #if CONFIG_SECURE_DISABLE_ROM_DL_MODE err = esp_efuse_disable_rom_download_mode(); @@ -323,6 +323,7 @@ static void do_core_init(void) esp_flash_app_init(); esp_err_t flash_ret = esp_flash_init_default_chip(); assert(flash_ret == ESP_OK); + (void)flash_ret; } static void do_secondary_init(void) diff --git a/components/freertos/port/port_common.c b/components/freertos/port/port_common.c index b80c82a1b0..293f824ed8 100644 --- a/components/freertos/port/port_common.c +++ b/components/freertos/port/port_common.c @@ -83,6 +83,7 @@ void esp_startup_start_app_common(void) ESP_TASK_MAIN_STACK, NULL, ESP_TASK_MAIN_PRIO, NULL, 0); assert(res == pdTRUE); + (void)res; } static void main_task(void* args) diff --git a/components/freertos/port/riscv/include/freertos/FreeRTOSConfig.h b/components/freertos/port/riscv/include/freertos/FreeRTOSConfig.h index 83d8e8bfe8..62ca5ce488 100644 --- a/components/freertos/port/riscv/include/freertos/FreeRTOSConfig.h +++ b/components/freertos/port/riscv/include/freertos/FreeRTOSConfig.h @@ -91,14 +91,15 @@ #include /* for abort() */ #include "esp32c3/rom/ets_sys.h" -#if defined(CONFIG_FREERTOS_ASSERT_DISABLE) -#define configASSERT(a) /* assertions disabled */ -#elif defined(CONFIG_FREERTOS_ASSERT_FAIL_PRINT_CONTINUE) +// If CONFIG_FREERTOS_ASSERT_DISABLE is set then configASSERT is defined empty later in FreeRTOS.h and the macro +// configASSERT_DEFINED remains unset (meaning some warnings are avoided) + +#if defined(CONFIG_FREERTOS_ASSERT_FAIL_PRINT_CONTINUE) #define configASSERT(a) if (unlikely(!(a))) { \ esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ __FUNCTION__); \ } -#else /* CONFIG_FREERTOS_ASSERT_FAIL_ABORT */ +#elif defined(CONFIG_FREERTOS_ASSERT_FAIL_ABORT) #define configASSERT(a) if (unlikely(!(a))) { \ esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ __FUNCTION__); \ diff --git a/components/freertos/port/riscv/port.c b/components/freertos/port/riscv/port.c index f327e0775b..26b07bdee9 100644 --- a/components/freertos/port/riscv/port.c +++ b/components/freertos/port/riscv/port.c @@ -148,8 +148,7 @@ void vPortExitCritical(void) void vPortSetupTimer(void) { /* set system timer interrupt vector */ - esp_err_t err = esp_intr_alloc(ETS_SYSTIMER_TARGET0_EDGE_INTR_SOURCE, ESP_INTR_FLAG_IRAM, vPortSysTickHandler, NULL, NULL); - assert(err == ESP_OK); + ESP_ERROR_CHECK(esp_intr_alloc(ETS_SYSTIMER_TARGET0_EDGE_INTR_SOURCE, ESP_INTR_FLAG_IRAM, vPortSysTickHandler, NULL, NULL)); /* configure the timer */ systimer_hal_init(); diff --git a/components/freertos/port/xtensa/include/freertos/FreeRTOSConfig.h b/components/freertos/port/xtensa/include/freertos/FreeRTOSConfig.h index 7b7d69437f..7aae48374f 100644 --- a/components/freertos/port/xtensa/include/freertos/FreeRTOSConfig.h +++ b/components/freertos/port/xtensa/include/freertos/FreeRTOSConfig.h @@ -131,14 +131,15 @@ int xt_clock_freq(void) __attribute__((deprecated)); #include "esp32c3/rom/ets_sys.h" #endif -#if defined(CONFIG_FREERTOS_ASSERT_DISABLE) -#define configASSERT(a) /* assertions disabled */ -#elif defined(CONFIG_FREERTOS_ASSERT_FAIL_PRINT_CONTINUE) -#define configASSERT(a) if (unlikely(!(a))) { \ +// If CONFIG_FREERTOS_ASSERT_DISABLE is set then configASSERT is defined empty later in FreeRTOS.h and the macro +// configASSERT_DEFINED remains unset (meaning some warnings are avoided) + +#if defined(CONFIG_FREERTOS_ASSERT_FAIL_PRINT_CONTINUE) +#define configASSERT(a) if (unlikely(!(a))) { \ esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ __FUNCTION__); \ } -#else /* CONFIG_FREERTOS_ASSERT_FAIL_ABORT */ +#elif defined(CONFIG_FREERTOS_ASSERT_FAIL_ABORT) #define configASSERT(a) if (unlikely(!(a))) { \ esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ __FUNCTION__); \ diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index 585224ca9a..0d1a6313b6 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -1935,6 +1935,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode taskEXIT_CRITICAL(&xTaskQueueMutex); configASSERT( suspended == 0 ); + (void)suspended; portYIELD_WITHIN_API(); } else diff --git a/components/hal/esp32s2/usbh_hal.c b/components/hal/esp32s2/usbh_hal.c index b00a08ae8e..5dd25b9ca8 100644 --- a/components/hal/esp32s2/usbh_hal.c +++ b/components/hal/esp32s2/usbh_hal.c @@ -141,8 +141,10 @@ void usbh_hal_init(usbh_hal_context_t *hal) { //Check if a peripheral is alive by reading the core ID registers usbh_dev_t *dev = &USBH; +#ifndef NDEBUG uint32_t core_id = usb_ll_get_controller_core_id(dev); assert(core_id == CORE_REG_GSNPSID); +#endif //Initialize HAL context memset(hal, 0, sizeof(usbh_hal_context_t)); hal->dev = dev; diff --git a/components/hal/sdio_slave_hal.c b/components/hal/sdio_slave_hal.c index b6dfed6fc9..43a1bdfb86 100644 --- a/components/hal/sdio_slave_hal.c +++ b/components/hal/sdio_slave_hal.c @@ -178,7 +178,7 @@ void sdio_slave_hal_hw_init(sdio_slave_context_t *hal) static esp_err_t init_send_queue(sdio_slave_context_t *hal) { esp_err_t ret; - esp_err_t rcv_res; + esp_err_t rcv_res __attribute((unused)); sdio_ringbuf_t *buf = &(hal->send_desc_queue); //initialize pointers diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 07cc258e70..546376eedd 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -42,7 +42,7 @@ static esp_alloc_failed_hook_t alloc_failed_callback; IRAM_ATTR static void *dram_alloc_to_iram_addr(void *addr, size_t len) { uintptr_t dstart = (uintptr_t)addr; //First word - uintptr_t dend = dstart + len - 4; //Last word + uintptr_t dend __attribute__((unused)) = dstart + len - 4; //Last word assert(esp_ptr_in_diram_dram((void *)dstart)); assert(esp_ptr_in_diram_dram((void *)dend)); assert((dstart & 3) == 0); diff --git a/components/libsodium/CMakeLists.txt b/components/libsodium/CMakeLists.txt index 2c1f10c313..5be92e7164 100644 --- a/components/libsodium/CMakeLists.txt +++ b/components/libsodium/CMakeLists.txt @@ -173,3 +173,8 @@ set_source_files_properties( PROPERTIES COMPILE_FLAGS -DRANDOMBYTES_DEFAULT_IMPLEMENTATION ) + +if(CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE) + # some libsodium variables are only used for asserts + target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-unused-but-set-variable) +endif() diff --git a/components/lwip/port/esp32/freertos/sys_arch.c b/components/lwip/port/esp32/freertos/sys_arch.c index 9319fa9f2b..1832e2283b 100644 --- a/components/lwip/port/esp32/freertos/sys_arch.c +++ b/components/lwip/port/esp32/freertos/sys_arch.c @@ -82,6 +82,7 @@ sys_mutex_lock(sys_mutex_t *pxMutex) BaseType_t ret = xSemaphoreTake(*pxMutex, portMAX_DELAY); LWIP_ASSERT("failed to take the mutex", ret == pdTRUE); + (void)ret; } /** @@ -95,6 +96,7 @@ sys_mutex_unlock(sys_mutex_t *pxMutex) BaseType_t ret = xSemaphoreGive(*pxMutex); LWIP_ASSERT("failed to give the mutex", ret == pdTRUE); + (void)ret; } /** @@ -134,6 +136,7 @@ sys_sem_new(sys_sem_t *sem, u8_t count) if (count == 1) { BaseType_t ret = xSemaphoreGive(*sem); LWIP_ASSERT("sys_sem_new: initial give failed", ret == pdTRUE); + (void)ret; } return ERR_OK; @@ -151,6 +154,7 @@ sys_sem_signal(sys_sem_t *sem) /* queue full is OK, this is a signal only... */ LWIP_ASSERT("sys_sem_signal: sane return value", (ret == pdTRUE) || (ret == errQUEUE_FULL)); + (void)ret; } /*-----------------------------------------------------------------------------------*/ @@ -247,6 +251,7 @@ sys_mbox_post(sys_mbox_t *mbox, void *msg) { BaseType_t ret = xQueueSendToBack((*mbox)->os_mbox, &msg, portMAX_DELAY); LWIP_ASSERT("mbox post failed", ret == pdTRUE); + (void)ret; } /** @@ -386,6 +391,8 @@ sys_mbox_free(sys_mbox_t *mbox) vQueueDelete((*mbox)->os_mbox); free(*mbox); *mbox = NULL; + + (void)msgs_waiting; } /** diff --git a/components/mbedtls/port/sha/esp_sha.c b/components/mbedtls/port/sha/esp_sha.c index 0940ed1320..82a7ac1d0d 100644 --- a/components/mbedtls/port/sha/esp_sha.c +++ b/components/mbedtls/port/sha/esp_sha.c @@ -45,7 +45,7 @@ void esp_sha(esp_sha_type sha_type, const unsigned char *input, size_t ilen, uns #endif } ctx; - int ret; + int ret __attribute__((unused)); assert(input != NULL && output != NULL); #if SOC_SHA_SUPPORT_SHA1 diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 52952b7a38..ee04d869d3 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -455,10 +455,9 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat uint8_t chunkCount = item.blobIndex.chunkCount; VerOffset chunkStart = item.blobIndex.chunkStart; - size_t readSize = item.blobIndex.dataSize; size_t offset = 0; - assert(dataSize == readSize); + assert(dataSize == item.blobIndex.dataSize); /* Now read corresponding chunks */ for (uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) { diff --git a/components/protobuf-c/CMakeLists.txt b/components/protobuf-c/CMakeLists.txt index 1801093194..0bca78fce1 100644 --- a/components/protobuf-c/CMakeLists.txt +++ b/components/protobuf-c/CMakeLists.txt @@ -1,2 +1,7 @@ idf_component_register(SRCS "protobuf-c/protobuf-c/protobuf-c.c" INCLUDE_DIRS protobuf-c) + +if(CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE) + # some variables are only used by asserts + target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-unused-but-set-variable) +endif() diff --git a/components/spi_flash/cache_utils.c b/components/spi_flash/cache_utils.c index 4418a7273f..a71dd0acac 100644 --- a/components/spi_flash/cache_utils.c +++ b/components/spi_flash/cache_utils.c @@ -152,8 +152,8 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void) // Signal to the spi_flash_op_block_task on the other CPU that we need it to // disable cache there and block other tasks from executing. s_flash_op_can_start = false; - esp_err_t ret = esp_ipc_call(other_cpuid, &spi_flash_op_block_func, (void *) other_cpuid); - assert(ret == ESP_OK); + ESP_ERROR_CHECK(esp_ipc_call(other_cpuid, &spi_flash_op_block_func, (void *) other_cpuid)); + while (!s_flash_op_can_start) { // Busy loop and wait for spi_flash_op_block_func to disable cache // on the other CPU diff --git a/components/wpa_supplicant/src/esp_supplicant/esp_wpa_main.c b/components/wpa_supplicant/src/esp_supplicant/esp_wpa_main.c index f2fad8643f..7e333c20bf 100644 --- a/components/wpa_supplicant/src/esp_supplicant/esp_wpa_main.c +++ b/components/wpa_supplicant/src/esp_supplicant/esp_wpa_main.c @@ -177,6 +177,7 @@ void wpa_sta_connect(uint8_t *bssid) wpa_config_profile(); ret = wpa_config_bss(bssid); WPA_ASSERT(ret == 0); + (void)ret; } int wpa_parse_wpa_ie_wrapper(const u8 *wpa_ie, size_t wpa_ie_len, wifi_wpa_ie_t *data) From 90ec0b03279969968c2a2ba9e3764ab9503efb70 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 10:05:42 +1100 Subject: [PATCH 02/13] 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 222aad9f2d..b647587f6c 100644 --- a/components/bootloader/subproject/CMakeLists.txt +++ b/components/bootloader/subproject/CMakeLists.txt @@ -29,10 +29,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 0f869ed475..924a27212b 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 9c6d4de1e6a0bf9202b0d4670f0e9c99377b457a Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 10:06:36 +1100 Subject: [PATCH 03/13] 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 0539d24a34..39cb3e25ee 100644 --- a/components/lwip/Kconfig +++ b/components/lwip/Kconfig @@ -771,9 +771,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 e74efa967f..78935f3b5f 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 10bde42551b479bd4bfccc9d3c6d983f8abe0b87 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 10:09:47 +1100 Subject: [PATCH 04/13] esp_websocket_client: Don't log the filename when logging "Websocket already stop" Progress towards https://jira.espressif.com:8443/browse/IDFGH-4477 --- components/esp_websocket_client/esp_websocket_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/esp_websocket_client/esp_websocket_client.c b/components/esp_websocket_client/esp_websocket_client.c index 055a237722..5055736d43 100644 --- a/components/esp_websocket_client/esp_websocket_client.c +++ b/components/esp_websocket_client/esp_websocket_client.c @@ -58,7 +58,7 @@ static const char *TAG = "WEBSOCKET_CLIENT"; } #define ESP_WS_CLIENT_STATE_CHECK(TAG, a, action) if ((a->state) < WEBSOCKET_STATE_INIT) { \ - ESP_LOGE(TAG,"%s:%d (%s): %s", __FILE__, __LINE__, __FUNCTION__, "Websocket already stop"); \ + ESP_LOGE(TAG,"%s(%d): %s", __FUNCTION__, __LINE__, "Websocket already stop"); \ action; \ } From a0c73c5f92b86e659e6925bc47af8830eb8a6893 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 10:10:49 +1100 Subject: [PATCH 05/13] bt host: Don't redefine the assert macro Required so that bt asserts obey the same configuration settings as other asserts. Progress towards https://github.com/espressif/esp-idf/issues/6306 --- components/bt/common/include/bt_common.h | 5 +---- components/bt/host/bluedroid/bta/av/bta_av_aact.c | 2 +- components/bt/host/bluedroid/bta/av/bta_av_main.c | 2 +- components/bt/host/bluedroid/bta/dm/bta_dm_pm.c | 2 +- components/bt/host/bluedroid/bta/sys/bta_sys_main.c | 2 +- components/bt/host/bluedroid/stack/avdt/avdt_ad.c | 2 +- components/bt/host/bluedroid/stack/avrc/avrc_api.c | 2 +- components/bt/host/bluedroid/stack/avrc/avrc_opt.c | 2 +- 8 files changed, 8 insertions(+), 11 deletions(-) diff --git a/components/bt/common/include/bt_common.h b/components/bt/common/include/bt_common.h index 15662303e3..54984af6fb 100644 --- a/components/bt/common/include/bt_common.h +++ b/components/bt/common/include/bt_common.h @@ -16,6 +16,7 @@ #ifndef _BT_COMMON_H_ #define _BT_COMMON_H_ +#include #include "bt_user_config.h" #include "esp_log.h" @@ -87,10 +88,6 @@ #define BT_PRINT_D(tag, format, ...) {esp_log_write(ESP_LOG_DEBUG, tag, LOG_FORMAT(D, format), esp_log_timestamp(), tag, ##__VA_ARGS__); } #define BT_PRINT_V(tag, format, ...) {esp_log_write(ESP_LOG_VERBOSE, tag, LOG_FORMAT(V, format), esp_log_timestamp(), tag, ##__VA_ARGS__); } -#ifndef assert -#define assert(x) do { if (!(x)) BT_PRINT_E("BT", "bt host error %s %u\n", __FILE__, __LINE__); } while (0) -#endif - #if !UC_BT_STACK_NO_LOG /* define traces for BTC */ diff --git a/components/bt/host/bluedroid/bta/av/bta_av_aact.c b/components/bt/host/bluedroid/bta/av/bta_av_aact.c index 35863d3217..60f7cdfa1c 100644 --- a/components/bt/host/bluedroid/bta/av/bta_av_aact.c +++ b/components/bt/host/bluedroid/bta/av/bta_av_aact.c @@ -27,7 +27,7 @@ #include "common/bt_target.h" #if defined(BTA_AV_INCLUDED) && (BTA_AV_INCLUDED == TRUE) -// #include +#include #include "common/bt_trace.h" #include diff --git a/components/bt/host/bluedroid/bta/av/bta_av_main.c b/components/bt/host/bluedroid/bta/av/bta_av_main.c index ae223fb635..561f0b60d0 100644 --- a/components/bt/host/bluedroid/bta/av/bta_av_main.c +++ b/components/bt/host/bluedroid/bta/av/bta_av_main.c @@ -22,7 +22,7 @@ * ******************************************************************************/ -// #include +#include #include #include "common/bt_target.h" diff --git a/components/bt/host/bluedroid/bta/dm/bta_dm_pm.c b/components/bt/host/bluedroid/bta/dm/bta_dm_pm.c index 699de89314..812f998908 100644 --- a/components/bt/host/bluedroid/bta/dm/bta_dm_pm.c +++ b/components/bt/host/bluedroid/bta/dm/bta_dm_pm.c @@ -23,7 +23,7 @@ * ******************************************************************************/ -// #include +#include #include #include "bta/bta_sys.h" diff --git a/components/bt/host/bluedroid/bta/sys/bta_sys_main.c b/components/bt/host/bluedroid/bta/sys/bta_sys_main.c index c495e14f09..58983d3a5d 100644 --- a/components/bt/host/bluedroid/bta/sys/bta_sys_main.c +++ b/components/bt/host/bluedroid/bta/sys/bta_sys_main.c @@ -23,7 +23,7 @@ ******************************************************************************/ #define LOG_TAG "bt_bta_sys_main" -// #include +#include #include #include "osi/alarm.h" diff --git a/components/bt/host/bluedroid/stack/avdt/avdt_ad.c b/components/bt/host/bluedroid/stack/avdt/avdt_ad.c index 361c5b276f..b24c952d90 100644 --- a/components/bt/host/bluedroid/stack/avdt/avdt_ad.c +++ b/components/bt/host/bluedroid/stack/avdt/avdt_ad.c @@ -22,7 +22,7 @@ * ******************************************************************************/ -// #include +#include #include "common/bt_trace.h" #include diff --git a/components/bt/host/bluedroid/stack/avrc/avrc_api.c b/components/bt/host/bluedroid/stack/avrc/avrc_api.c index beb823eac9..9972bbfde4 100644 --- a/components/bt/host/bluedroid/stack/avrc/avrc_api.c +++ b/components/bt/host/bluedroid/stack/avrc/avrc_api.c @@ -21,7 +21,7 @@ * Interface to AVRCP mandatory commands * ******************************************************************************/ -// #include +#include #include "common/bt_trace.h" #include #include "common/bt_target.h" diff --git a/components/bt/host/bluedroid/stack/avrc/avrc_opt.c b/components/bt/host/bluedroid/stack/avrc/avrc_opt.c index 9ecf966ee1..6f0663c08a 100644 --- a/components/bt/host/bluedroid/stack/avrc/avrc_opt.c +++ b/components/bt/host/bluedroid/stack/avrc/avrc_opt.c @@ -21,7 +21,7 @@ * Interface to AVRCP optional commands * ******************************************************************************/ -// #include +#include #include "common/bt_target.h" #include #include "stack/avrc_api.h" From 0d26c89b6bdb8327220477bae3b747de71a0c135 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 10:15:56 +1100 Subject: [PATCH 06/13] btc_common: Redefine BTC assert macro to use standard assert Allows assert to be disabled, made silent, etc. Progress towards https://github.com/espressif/esp-idf/issues/6306 --- components/bt/host/bluedroid/btc/core/btc_dm.c | 2 +- components/bt/host/bluedroid/btc/include/btc/btc_common.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/bt/host/bluedroid/btc/core/btc_dm.c b/components/bt/host/bluedroid/btc/core/btc_dm.c index 1f9c761959..754745be95 100644 --- a/components/bt/host/bluedroid/btc/core/btc_dm.c +++ b/components/bt/host/bluedroid/btc/core/btc_dm.c @@ -342,7 +342,7 @@ static void btc_dm_auth_cmpl_evt (tBTA_DM_AUTH_CMPL *p_auth_cmpl) ) #endif if (1) { - bt_status_t ret; + bt_status_t ret __attribute__((unused)); BTC_TRACE_DEBUG("%s: Storing link key. key_type=0x%x", __FUNCTION__, p_auth_cmpl->key_type); ret = btc_storage_add_bonded_device(&bd_addr, diff --git a/components/bt/host/bluedroid/btc/include/btc/btc_common.h b/components/bt/host/bluedroid/btc/include/btc/btc_common.h index ae4501b5a8..ed225db758 100644 --- a/components/bt/host/bluedroid/btc/include/btc/btc_common.h +++ b/components/bt/host/bluedroid/btc/include/btc/btc_common.h @@ -16,12 +16,12 @@ #ifndef __BTC_COMMON_H__ #define __BTC_COMMON_H__ +#include #include "common/bt_trace.h" #include "stack/bt_types.h" #include "osi/osi.h" -#define BTC_ASSERTC(cond, msg, val) if (!(cond)) { LOG_ERROR( \ - "### ASSERT : %s line %d %s (%d) ###", __FILE__, __LINE__, msg, val);} +#define BTC_ASSERTC(cond, msg, val) assert(cond && msg) #define BTC_HAL_CBACK(P_CB, P_CBACK, ...)\ if (P_CB && P_CB->P_CBACK) { \ From cfde7adb7ff474b7db986d516ee5116f7065db2d Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 11:18:32 +1100 Subject: [PATCH 07/13] esp_ble_mesh: Use standard ESP-IDF asserts Smaller binary size, means that config options for silent assert or disable assert will apply --- .../bt/esp_ble_mesh/btc/btc_ble_mesh_prov.c | 2 +- .../esp_ble_mesh/mesh_common/include/mesh_trace.h | 15 +++------------ components/bt/esp_ble_mesh/mesh_core/adv.c | 1 + 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/components/bt/esp_ble_mesh/btc/btc_ble_mesh_prov.c b/components/bt/esp_ble_mesh/btc/btc_ble_mesh_prov.c index 6fe3da8b7e..ac2fabb07a 100644 --- a/components/bt/esp_ble_mesh/btc/btc_ble_mesh_prov.c +++ b/components/bt/esp_ble_mesh/btc/btc_ble_mesh_prov.c @@ -1017,7 +1017,7 @@ int btc_ble_mesh_client_model_init(esp_ble_mesh_model_t *model) return -EINVAL; } - __ASSERT(model && model->op, "%s, Invalid parameter", __func__); + __ASSERT(model && model->op, "Invalid parameter"); esp_ble_mesh_model_op_t *op = model->op; while (op != NULL && op->opcode != 0) { op->param_cb = (esp_ble_mesh_cb_t)btc_ble_mesh_client_model_op_cb; diff --git a/components/bt/esp_ble_mesh/mesh_common/include/mesh_trace.h b/components/bt/esp_ble_mesh/mesh_common/include/mesh_trace.h index aa46790f12..3d9a94b363 100644 --- a/components/bt/esp_ble_mesh/mesh_common/include/mesh_trace.h +++ b/components/bt/esp_ble_mesh/mesh_common/include/mesh_trace.h @@ -9,6 +9,7 @@ #ifndef _BLE_MESH_TRACE_H_ #define _BLE_MESH_TRACE_H_ +#include #include "esp_log.h" #include "mesh_util.h" #include "esp_rom_sys.h" @@ -72,21 +73,11 @@ extern "C" { #define STRINGIFY(s) _STRINGIFY(s) #ifndef __ASSERT -#define __ASSERT(test, fmt, ...) \ - do { \ - if (!(test)) { \ - printk("ASSERTION FAIL [%s] @ %s:%d:\n\t", \ - _STRINGIFY(test), \ - __FILE__, \ - __LINE__); \ - printk(fmt, ##__VA_ARGS__); \ - for (;;); \ - } \ - } while ((0)) +#define __ASSERT(test, str) assert(test) #endif #ifndef __ASSERT_NO_MSG -#define __ASSERT_NO_MSG(x) do { if (!(x)) BLE_MESH_PRINT_E(BLE_MESH_TRACE_TAG, "error %s %u", __FILE__, __LINE__); } while (0) +#define __ASSERT_NO_MSG(x) assert(x) #endif #if !CONFIG_BLE_MESH_NO_LOG diff --git a/components/bt/esp_ble_mesh/mesh_core/adv.c b/components/bt/esp_ble_mesh/mesh_core/adv.c index a697c5e860..b098fb70ca 100644 --- a/components/bt/esp_ble_mesh/mesh_core/adv.c +++ b/components/bt/esp_ble_mesh/mesh_core/adv.c @@ -652,6 +652,7 @@ void bt_mesh_adv_init(void) int ret = xTaskCreatePinnedToCore(adv_thread, BLE_MESH_ADV_TASK_NAME, BLE_MESH_ADV_TASK_STACK_SIZE, NULL, BLE_MESH_ADV_TASK_PRIO, &adv_task.handle, BLE_MESH_ADV_TASK_CORE); __ASSERT(ret == pdTRUE, "Failed to create adv thread"); + (void)ret; #endif /* CONFIG_BLE_MESH_FREERTOS_STATIC_ALLOC_EXTERNAL && CONFIG_SPIRAM_CACHE_WORKAROUND && CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY */ } From 61b70c50a4ca11061fabcd68ad943a22492dd446 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 15:13:50 +1100 Subject: [PATCH 08/13] 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 | 8 ++------ .../port/xtensa/include/freertos/FreeRTOSConfig.h | 8 ++------ 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/components/freertos/Kconfig b/components/freertos/Kconfig index ad975c6510..25de7e73c7 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 62ca5ce488..63d5aa44cc 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 CONFIG_FREERTOS_ASSERT_DISABLE is set then configASSERT is defined empty later in FreeRTOS.h and the macro @@ -100,11 +100,7 @@ __FUNCTION__); \ } #elif defined(CONFIG_FREERTOS_ASSERT_FAIL_ABORT) -#define configASSERT(a) if (unlikely(!(a))) { \ - esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ - __FUNCTION__); \ - 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 7aae48374f..93c7f81c98 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 @@ -140,11 +140,7 @@ int xt_clock_freq(void) __attribute__((deprecated)); __FUNCTION__); \ } #elif defined(CONFIG_FREERTOS_ASSERT_FAIL_ABORT) -#define configASSERT(a) if (unlikely(!(a))) { \ - esp_rom_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ - __FUNCTION__); \ - abort(); \ - } +#define configASSERT(a) assert(a) #endif #if CONFIG_FREERTOS_ASSERT_ON_UNTESTED_FUNCTION From c68f8694118a966e54921abf1193b9f6e648aa95 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 15:31:52 +1100 Subject: [PATCH 09/13] driver: Remove some stray __FILE__ macros Related to https://github.com/espressif/esp-idf/issues/6306 --- components/driver/esp32c3/adc.c | 6 +++--- components/driver/esp32c3/rtc_tempsensor.c | 2 +- components/driver/esp32s2/dac.c | 2 +- .../include/touch_element/touch_element_private.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/driver/esp32c3/adc.c b/components/driver/esp32c3/adc.c index e4c495d4cf..81b30860c9 100644 --- a/components/driver/esp32c3/adc.c +++ b/components/driver/esp32c3/adc.c @@ -34,9 +34,9 @@ #include "esp_efuse_rtc_calib.h" #include "esp_private/gdma.h" -#define ADC_CHECK_RET(fun_ret) ({ \ +#define ADC_CHECK_RET(fun_ret) ({ \ if (fun_ret != ESP_OK) { \ - ESP_LOGE(ADC_TAG,"%s:%d\n",__FUNCTION__,__LINE__); \ + ESP_LOGE(ADC_TAG,"%s(%d)",__FUNCTION__,__LINE__); \ return ESP_FAIL; \ } \ }) @@ -45,7 +45,7 @@ static const char *ADC_TAG = "ADC"; #define ADC_CHECK(a, str, ret_val) ({ \ if (!(a)) { \ - ESP_LOGE(ADC_TAG,"%s:%d (%s):%s", __FILE__, __LINE__, __FUNCTION__, str); \ + ESP_LOGE(ADC_TAG,"%s(%d) :%s", __FUNCTION__, __LINE__, str); \ return (ret_val); \ } \ }) diff --git a/components/driver/esp32c3/rtc_tempsensor.c b/components/driver/esp32c3/rtc_tempsensor.c index 1ada5f4c0c..189b3418f5 100644 --- a/components/driver/esp32c3/rtc_tempsensor.c +++ b/components/driver/esp32c3/rtc_tempsensor.c @@ -33,7 +33,7 @@ static const char *TAG = "tsens"; #define TSENS_CHECK(res, ret_val) ({ \ if (!(res)) { \ - ESP_LOGE(TAG, "%s:%d (%s)", __FILE__, __LINE__, __FUNCTION__); \ + ESP_LOGE(TAG, "%s(%d)", __FUNCTION__, __LINE__); \ return (ret_val); \ } \ }) diff --git a/components/driver/esp32s2/dac.c b/components/driver/esp32s2/dac.c index bbba688d23..1501a293d5 100644 --- a/components/driver/esp32s2/dac.c +++ b/components/driver/esp32s2/dac.c @@ -29,7 +29,7 @@ static const char *DAC_TAG = "DAC"; #define DAC_CHECK(a, str, ret_val) ({ \ if (!(a)) { \ - ESP_LOGE(DAC_TAG,"%s:%d (%s):%s", __FILE__, __LINE__, __FUNCTION__, str); \ + ESP_LOGE(DAC_TAG,"%s(%d): %s", __FUNCTION__, __LINE__, str); \ return (ret_val); \ } \ }) diff --git a/components/touch_element/include/touch_element/touch_element_private.h b/components/touch_element/include/touch_element/touch_element_private.h index 1d5f837e58..85143157fe 100644 --- a/components/touch_element/include/touch_element/touch_element_private.h +++ b/components/touch_element/include/touch_element/touch_element_private.h @@ -29,7 +29,7 @@ extern "C" { #define TE_CHECK(cond, ret_val) ({ \ if (!(cond)) { \ - ESP_LOGE(TE_TAG, "%s:%d (%s)", __FILE__, __LINE__, __FUNCTION__); \ + ESP_LOGE(TE_TAG, "%s(%d)", __FUNCTION__, __LINE__); \ return (ret_val); \ } \ }) From 74fa52668c326bbafc3163810ff4ea61ed39f754 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 15:36:16 +1100 Subject: [PATCH 10/13] esp-mqtt: Remove __FILE__ macro from error logs --- components/mqtt/esp-mqtt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/mqtt/esp-mqtt b/components/mqtt/esp-mqtt index 9ea804e0ab..9fdf7b6138 160000 --- a/components/mqtt/esp-mqtt +++ b/components/mqtt/esp-mqtt @@ -1 +1 @@ -Subproject commit 9ea804e0ab5368d5ab53ae2301a5fec9d1f12f1a +Subproject commit 9fdf7b61385633075d5c3b84803f2dd0578d7869 From 9ae01e40b55e7509adadbfeb40b8309060128609 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 11:08:49 +1100 Subject: [PATCH 11/13] ci: Add a test app for not placing embedded file paths into binaries Doubles as a test app that building with assertions off doesn't produce warnings. Closes https://github.com/espressif/esp-idf/issues/6306 --- tools/ci/executable-list.txt | 1 + .../system/no_embedded_paths/CMakeLists.txt | 20 ++++ .../system/no_embedded_paths/README.md | 14 +++ .../no_embedded_paths/check_for_file_paths.py | 94 +++++++++++++++++++ .../no_embedded_paths/main/CMakeLists.txt | 2 + .../main/test_no_embedded_paths_main.c | 4 + .../no_embedded_paths/sdkconfig.ci.noasserts | 8 ++ .../sdkconfig.ci.noasserts.nimble | 13 +++ .../sdkconfig.ci.silentasserts | 7 ++ .../sdkconfig.ci.silentasserts.nimble | 13 +++ 10 files changed, 176 insertions(+) create mode 100644 tools/test_apps/system/no_embedded_paths/CMakeLists.txt create mode 100644 tools/test_apps/system/no_embedded_paths/README.md create mode 100755 tools/test_apps/system/no_embedded_paths/check_for_file_paths.py create mode 100644 tools/test_apps/system/no_embedded_paths/main/CMakeLists.txt create mode 100644 tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble diff --git a/tools/ci/executable-list.txt b/tools/ci/executable-list.txt index ba39889e81..6a6ec198a9 100644 --- a/tools/ci/executable-list.txt +++ b/tools/ci/executable-list.txt @@ -99,6 +99,7 @@ tools/mass_mfg/mfg_gen.py tools/mkdfu.py tools/mkuf2.py tools/set-submodules-to-github.sh +tools/test_apps/system/no_embedded_paths/check_for_file_paths.py tools/test_idf_monitor/run_test_idf_monitor.py tools/test_idf_py/test_idf_py.py tools/test_idf_size/test.sh diff --git a/tools/test_apps/system/no_embedded_paths/CMakeLists.txt b/tools/test_apps/system/no_embedded_paths/CMakeLists.txt new file mode 100644 index 0000000000..e6fdc3cb6b --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/CMakeLists.txt @@ -0,0 +1,20 @@ +# The following lines of boilerplate have to be in your project's +# CMakeLists in this exact order for cmake to work correctly +cmake_minimum_required(VERSION 3.5) + +include($ENV{IDF_PATH}/tools/cmake/project.cmake) +project(no_embedded_paths) + +idf_build_get_property(idf_path IDF_PATH) +idf_build_get_property(python PYTHON) +idf_build_get_property(elf EXECUTABLE) + +# If the configuration is one that doesn't expect any paths to be found then run this build step +# after building the ELF, will fail if it finds any file paths in binary files +if(CONFIG_OPTIMIZATION_ASSERTIONS_SILENT OR CONFIG_OPTIMIZATION_ASSERTIONS_DISABLED) + add_custom_command( + TARGET ${elf} + POST_BUILD + COMMAND ${python} "${CMAKE_CURRENT_LIST_DIR}/check_for_file_paths.py" "${idf_path}" "${CMAKE_BINARY_DIR}" + ) +endif() diff --git a/tools/test_apps/system/no_embedded_paths/README.md b/tools/test_apps/system/no_embedded_paths/README.md new file mode 100644 index 0000000000..32a9312a32 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/README.md @@ -0,0 +1,14 @@ +# No Embedded Paths + +This test app exists to verify that paths (like __FILE__) are not compiled into +any object files in configurations where this should be avoided. + +It doubles up as a build-time check that disabling assertions doesn't lead to +any warnings. + +(These configurations include: assertions disabled, 'silent' asserts, any reproducible +builds configuration.) + +Not embedding paths reduces the binary size, avoids leaking information about +the compilation environment, and is a necessary step to supporet reproducible +builds across projects built in different directories. diff --git a/tools/test_apps/system/no_embedded_paths/check_for_file_paths.py b/tools/test_apps/system/no_embedded_paths/check_for_file_paths.py new file mode 100755 index 0000000000..ca0bd21b9c --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/check_for_file_paths.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python +# +# 'check_for_file_paths.py' is a CI tool that checks all the unlinked object files +# in a CMake build directory for embedded copies of IDF_PATH. +# +# Designed to be run in CI as a check that __FILE__ macros haven't snuck into any source code. +# +# Checking the unlinked object files means we don't rely on anything being actually linked into the binary, +# just anything which could potentially be linked. +# +# Usage: +# ./check_for_file_paths.py +# +# +# +# Copyright 2019 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. +# +import os +import re +import sys + +from elftools.elf.elffile import ELFFile + +# If an ESP-IDF source file has no option but to include __FILE__ macros, name it here (as re expression). +# +# IMPORTANT: This should only be used for upstream code where there is no other +# option. ESP-IDF code should avoid embedding file names as much as possible to +# limit the binary size and support reproducible builds +# +# Note: once ESP-IDF moves to Python >=3.6 then this can be simplified to use 'glob' and '**' +EXCEPTIONS = [ + r'openssl/.+/ssl_pm.c.obj$', # openssl API requires __FILE__ in error reporting functions, as per upstream API + r'openssl/.+/ssl_bio.c.obj$', + r'unity/.+/unity_runner.c.obj$', # unity is not for production use, has __FILE__ for test information +] + + +def main(): # type: () -> None + idf_path = sys.argv[1] + build_dir = sys.argv[2] + + assert os.path.exists(idf_path) + assert os.path.exists(build_dir) + + print('Checking object files in {} for mentions of {}...'.format(build_dir, idf_path)) + + # note: once ESP-IDF moves to Python >=3.6 then this can be simplified to use 'glob' and f'{build_dir}**/*.obj' + files = [] + for (dirpath, _, filepaths) in os.walk(build_dir): + files += [os.path.join(dirpath, filepath) for filepath in filepaths if filepath.endswith('.obj')] + + print('Found {} object files...'.format(len(files))) + + idf_path_binary = idf_path.encode() # we're going to be checking binary streams (note: probably non-ascii IDF_PATH will not match OK) + + failures = 0 + for obj_file in files: + if not any(re.search(exception, obj_file) for exception in EXCEPTIONS): + failures += check_file(obj_file, idf_path_binary) + if failures > 0: + raise SystemExit('{} source files are embedding file paths, see list above.'.format(failures)) + print('No embedded file paths found') + + +def check_file(obj_file, idf_path): # type: (str, bytes) -> int + failures = 0 + with open(obj_file, 'rb') as f: + elf = ELFFile(f) + for sec in elf.iter_sections(): + # can't find a better way to filter out only sections likely to contain strings, + # and exclude debug sections. .dram matches DRAM_STR, which links to .dram1 + if '.rodata' in sec.name or '.dram' in sec.name: + contents = sec.data() + if idf_path in contents: + print('error: {} contains an unwanted __FILE__ macro'.format(obj_file)) + failures += 1 + break + return failures + + +if __name__ == '__main__': + main() diff --git a/tools/test_apps/system/no_embedded_paths/main/CMakeLists.txt b/tools/test_apps/system/no_embedded_paths/main/CMakeLists.txt new file mode 100644 index 0000000000..5121b33045 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/main/CMakeLists.txt @@ -0,0 +1,2 @@ +idf_component_register(SRCS "test_no_embedded_paths_main.c" + INCLUDE_DIRS ".") diff --git a/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c b/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c new file mode 100644 index 0000000000..c6d6340883 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c @@ -0,0 +1,4 @@ +/* This test app only exists for the build stage, so doesn't need to do anything at runtime */ +void app_main(void) +{ +} diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts new file mode 100644 index 0000000000..446a04da0b --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts @@ -0,0 +1,8 @@ +CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE=y +CONFIG_FREERTOS_ASSERT_DISABLE=y + +# compiling as many files as possible here (we don't have 100% coverage of course, due to config options, but +# try to maximize what we can check +CONFIG_BT_ENABLED=y +CONFIG_BT_BLUEDROID_ENABLED=y +CONFIG_BLE_MESH=y diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble new file mode 100644 index 0000000000..b91ad2022d --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble @@ -0,0 +1,13 @@ +CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE=y +CONFIG_FREERTOS_ASSERT_DISABLE=y + +# the other sdkconfig builds Bluedroid, build NimBLE here +# +# (Note: ESP32-S2 will build both these configs as well, but they're identical. This is simpler than +# needing to specify per-target configs for both Bluedroid and Nimble on ESP32, ESP32-C3.) +CONFIG_BT_ENABLED=y +CONFIG_BT_NIMBLE_ENABLED=y +CONFIG_BT_NIMBLE_CRYPTO_STACK_MBEDTLS=n +CONFIG_BT_NIMBLE_MESH=y +CONFIG_BLE_MESH=y +CONFIG_BT_NIMBLE_MAX_CONNECTIONS=1 diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts new file mode 100644 index 0000000000..9b54479ee9 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts @@ -0,0 +1,7 @@ +CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y + +# compiling as many files as possible here (we don't have 100% coverage of course, due to config options, but +# try to maximize what we can check +CONFIG_BT_ENABLED=y +CONFIG_BT_BLUEDROID_ENABLED=y +CONFIG_BLE_MESH=y diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble new file mode 100644 index 0000000000..ab0cd5aa6d --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble @@ -0,0 +1,13 @@ +CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y +CONFIG_FREERTOS_ASSERT_DISABLE=y + +# the other sdkconfig builds Bluedroid, build NimBLE here +# +# (Note: ESP32-S2 will build both these configs as well, but they're identical. This is simpler than +# needing to specify per-target configs for both Bluedroid and Nimble on ESP32, ESP32-C3.) +CONFIG_BT_ENABLED=y +CONFIG_BT_NIMBLE_ENABLED=y +CONFIG_BT_NIMBLE_CRYPTO_STACK_MBEDTLS=n +CONFIG_BT_NIMBLE_MESH=y +CONFIG_BLE_MESH=y +CONFIG_BT_NIMBLE_MAX_CONNECTIONS=1 From 9b988ca097bb1152de655fdf16904561e2426ba5 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 16:02:23 +1100 Subject: [PATCH 12/13] config: Add new option to replace IDF_PATH and project path with placeholders in macros Allows building with asserts on and still not finding any actual file paths in the final binary file. Alternative fix for https://github.com/espressif/esp-idf/issues/6306 Progress towards https://github.com/espressif/esp-idf/issues/5873 --- CMakeLists.txt | 5 +++++ Kconfig | 16 ++++++++++++++++ .../system/no_embedded_paths/CMakeLists.txt | 6 ++++-- .../test_apps/system/no_embedded_paths/README.md | 10 +++++----- .../main/test_no_embedded_paths_main.c | 12 +++++++++++- .../no_embedded_paths/sdkconfig.ci.noasserts | 1 + .../sdkconfig.ci.noasserts.nimble | 1 + .../no_embedded_paths/sdkconfig.ci.replacepaths | 8 ++++++++ .../no_embedded_paths/sdkconfig.ci.silentasserts | 1 + .../sdkconfig.ci.silentasserts.nimble | 1 + 10 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.replacepaths diff --git a/CMakeLists.txt b/CMakeLists.txt index c7e499304b..d684bbd9c9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -90,6 +90,11 @@ if(CONFIG_COMPILER_DUMP_RTL_FILES) list(APPEND compile_options "-fdump-rtl-expand") endif() +if(CONFIG_COMPILER_HIDE_PATHS_MACROS) + list(APPEND compile_options "-fmacro-prefix-map=${CMAKE_SOURCE_DIR}=.") + list(APPEND compile_options "-fmacro-prefix-map=${IDF_PATH}=IDF") +endif() + # GCC-specific options if(CMAKE_C_COMPILER_ID STREQUAL "GNU") list(APPEND compile_options "-fstrict-volatile-bitfields" diff --git a/Kconfig b/Kconfig index 1d498baf59..6ea56d39ac 100644 --- a/Kconfig +++ b/Kconfig @@ -279,6 +279,22 @@ mainmenu "Espressif IoT Development Framework Configuration" endchoice # assertions + menuconfig COMPILER_HIDE_PATHS_MACROS + bool "Replace ESP-IDF and project paths in binaries" + default y + depends on IDF_CMAKE + help + When expanding the __FILE__ and __BASE_FILE__ macros, replace paths inside ESP-IDF + with paths relative to the placeholder string "IDF", and convert paths inside the + project directory to relative paths. + + This allows building the project with assertions or other code that embeds file paths, + without the binary containing the exact path to the IDF or project directories. + + This option passes -fmacro-prefix-map options to the GCC command line. To replace additional + paths in your binaries, modify the project CMakeLists.txt file to pass custom -fmacro-prefix-map or + -ffile-prefix-map arguments. + menuconfig COMPILER_CXX_EXCEPTIONS bool "Enable C++ exceptions" default n diff --git a/tools/test_apps/system/no_embedded_paths/CMakeLists.txt b/tools/test_apps/system/no_embedded_paths/CMakeLists.txt index e6fdc3cb6b..4edc089d8e 100644 --- a/tools/test_apps/system/no_embedded_paths/CMakeLists.txt +++ b/tools/test_apps/system/no_embedded_paths/CMakeLists.txt @@ -9,9 +9,11 @@ idf_build_get_property(idf_path IDF_PATH) idf_build_get_property(python PYTHON) idf_build_get_property(elf EXECUTABLE) -# If the configuration is one that doesn't expect any paths to be found then run this build step +# If the configuration is one that doesn't expect the IDF_PATH to be found in binaries then run this build step # after building the ELF, will fail if it finds any file paths in binary files -if(CONFIG_OPTIMIZATION_ASSERTIONS_SILENT OR CONFIG_OPTIMIZATION_ASSERTIONS_DISABLED) +if(CONFIG_OPTIMIZATION_ASSERTIONS_SILENT OR + CONFIG_OPTIMIZATION_ASSERTIONS_DISABLED OR + CONFIG_COMPILER_HIDE_PATHS_MACROS) add_custom_command( TARGET ${elf} POST_BUILD diff --git a/tools/test_apps/system/no_embedded_paths/README.md b/tools/test_apps/system/no_embedded_paths/README.md index 32a9312a32..5206c2df8e 100644 --- a/tools/test_apps/system/no_embedded_paths/README.md +++ b/tools/test_apps/system/no_embedded_paths/README.md @@ -3,12 +3,12 @@ This test app exists to verify that paths (like __FILE__) are not compiled into any object files in configurations where this should be avoided. -It doubles up as a build-time check that disabling assertions doesn't lead to -any warnings. +Configurations where this is relevant include: -(These configurations include: assertions disabled, 'silent' asserts, any reproducible -builds configuration.) +* Assertions disabled (doubles up as a build-time check that disabling assertions doesn't lead to any warnings) +* Silent assertions +* CONFIG_COMPILER_HIDE_PATHS_MACROS is set to replace IDF_PATH and project dir with placeholders when expanding `__FILE__` Not embedding paths reduces the binary size, avoids leaking information about -the compilation environment, and is a necessary step to supporet reproducible +the compilation environment, and is a necessary step to support reproducible builds across projects built in different directories. diff --git a/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c b/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c index c6d6340883..7fce3f7efc 100644 --- a/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c +++ b/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c @@ -1,4 +1,14 @@ -/* This test app only exists for the build stage, so doesn't need to do anything at runtime */ +/* This test app only exists for the build stage, so doesn't need to do anything at runtime + apart from embedding an assert to check asserts inside the project dir */ +#include + +// Declared non-static to avoid the assert being optimized out +int other_function(void) +{ + return 3; +} + void app_main(void) { + assert(other_function() == 3); } diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts index 446a04da0b..22cb4d70ea 100644 --- a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts @@ -1,5 +1,6 @@ CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE=y CONFIG_FREERTOS_ASSERT_DISABLE=y +CONFIG_COMPILER_HIDE_PATHS_MACROS=n # compiling as many files as possible here (we don't have 100% coverage of course, due to config options, but # try to maximize what we can check diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble index b91ad2022d..7d8ccfe182 100644 --- a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble @@ -1,5 +1,6 @@ CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE=y CONFIG_FREERTOS_ASSERT_DISABLE=y +CONFIG_COMPILER_HIDE_PATHS_MACROS=n # the other sdkconfig builds Bluedroid, build NimBLE here # diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.replacepaths b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.replacepaths new file mode 100644 index 0000000000..966b86d989 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.replacepaths @@ -0,0 +1,8 @@ +# this is the default value, actually +CONFIG_COMPILER_HIDE_PATHS_MACROS=y + +# compiling as many files as possible here (we don't have 100% coverage of course, due to config options, but +# try to maximize what we can check +CONFIG_BT_ENABLED=y +CONFIG_BT_BLUEDROID_ENABLED=y +CONFIG_BLE_MESH=y diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts index 9b54479ee9..f75fef7686 100644 --- a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts @@ -1,4 +1,5 @@ CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y +CONFIG_COMPILER_HIDE_PATHS_MACROS=n # compiling as many files as possible here (we don't have 100% coverage of course, due to config options, but # try to maximize what we can check diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble index ab0cd5aa6d..640f0fd2e1 100644 --- a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble @@ -1,5 +1,6 @@ CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y CONFIG_FREERTOS_ASSERT_DISABLE=y +CONFIG_COMPILER_HIDE_PATHS_MACROS=n # the other sdkconfig builds Bluedroid, build NimBLE here # From 772b218c38e6b9247b0414f440a8222394ca8899 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 23 Feb 2021 13:27:07 +1100 Subject: [PATCH 13/13] docs: Add section about assertions to the style guide --- docs/en/api-guides/error-handling.rst | 8 ++++-- docs/en/contribute/style-guide.rst | 40 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/docs/en/api-guides/error-handling.rst b/docs/en/api-guides/error-handling.rst index 86277266b5..42c5e7985e 100644 --- a/docs/en/api-guides/error-handling.rst +++ b/docs/en/api-guides/error-handling.rst @@ -10,13 +10,13 @@ Overview Identifying and handling run-time errors is important for developing robust applications. There can be multiple kinds of run-time errors: - Recoverable errors: - + - Errors indicated by functions through return values (error codes) - C++ exceptions, thrown using ``throw`` keyword - Unrecoverable (fatal) errors: - - - Failed assertions (using ``assert`` macro and equivalent methods) and ``abort()`` calls. + + - Failed assertions (using ``assert`` macro and equivalent methods, see :ref:`assertions`) and ``abort()`` calls. - CPU exceptions: access to protected regions of memory, illegal instruction, etc. - System level checks: watchdog timeout, cache access error, stack overflow, stack smashing, heap corruption, etc. @@ -43,6 +43,8 @@ Additionally, :cpp:func:`esp_err_to_name_r` function will attempt to interpret t This feature is enabled by default, but can be disabled to reduce application binary size. See :ref:`CONFIG_ESP_ERR_TO_NAME_LOOKUP`. When this feature is disabled, :cpp:func:`esp_err_to_name` and :cpp:func:`esp_err_to_name_r` are still defined and can be called. In this case, :cpp:func:`esp_err_to_name` will return ``UNKNOWN ERROR``, and :cpp:func:`esp_err_to_name_r` will return ``Unknown error 0xXXXX(YYYYY)``, where ``0xXXXX`` and ``YYYYY`` are the hexadecimal and decimal representations of the error code, respectively. +.. _esp-error-check-macro: + ``ESP_ERROR_CHECK`` macro ------------------------- diff --git a/docs/en/contribute/style-guide.rst b/docs/en/contribute/style-guide.rst index f7dc397b86..8e2376f1b8 100644 --- a/docs/en/contribute/style-guide.rst +++ b/docs/en/contribute/style-guide.rst @@ -217,6 +217,46 @@ Enums should be defined through the `typedef` and be namespaced:: MODULE_FOO_THREE } module_foo_t; + +.. _assertions: + +Assertions +^^^^^^^^^^ + +The standard C ``assert()`` function, defined in ``assert.h`` should be used to check conditions that should be true in source code. In the default configuration, an assert condition that returns ``false`` or 0 will call ``abort()`` and trigger a :doc:`Fatal Error`. + +``assert()`` should only be used to detect unrecoverable errors due to a serious internal logic bug or corruption, where it's not possible for the program to continue. For recoverable errors, including errors that are possible due to invalid external input, an :doc:`error value should be returned `. + +.. note:: + + When asserting a value of type ``esp_err_t``is equal to ``ESP_OK``, use the :ref:`esp-error-check-macro` instead of an ``assert()``. + +It's possible to configure ESP-IDF projects with assertions disabled (see :ref:`CONFIG_COMPILER_OPTIMIZATION_ASSERTION_LEVEL`). Therefore, functions called in an ``assert()`` statement should not have side-effects. + +It's also necessary to use particular techniques to avoid "variable set but not used" warnings when assertions are disabled, due to code patterns such as:: + + int res = do_something(); + assert(res == 0); + +Once the ``assert`` is optimized out, the ``res`` value is unused and the compiler will warn about this. However the function ``do_something()`` must still be called, even if assertions are disabled. + +When the variable is declared and initialized in a single statement, a good strategy is to cast it to ``void`` on a new line. The compiler will not produce a warning, and the variable can still be optimized out of the final binary:: + + int res = do_something(); + assert(res == 0); + (void)res; + +If the variable is declared separately, for example if it is used for multiple assertions, then it can be declared with the GCC attribute ``__attribute__((unused))``. The compiler will not produce any unused variable warnings, but the variable can still be optimized out:: + + int res __attribute__((unused)); + + res = do_something(); + assert(res == 0); + + res = do_something_else(); + assert(res != 0); + + C++ Code Formatting -------------------