From b868fd2a958585e1eab89f10502b1a381ad12699 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Wed, 29 Dec 2021 10:40:22 +0800 Subject: [PATCH 1/2] espcoredump: fix a bug where tracked DRAM data where not dumped Variables marked as COREDUMP_DRAM_ATTR will now be part of the core dump. * Closes https://github.com/espressif/esp-idf/issues/8151 --- components/esp_common/include/esp_attr.h | 35 ++++++++++--------- .../esp_system/ld/esp32s2/sections.ld.in | 3 +- components/espcoredump/linker.lf | 21 ++++++----- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/components/esp_common/include/esp_attr.h b/components/esp_common/include/esp_attr.h index 7b73af7663..106a686b5e 100644 --- a/components/esp_common/include/esp_attr.h +++ b/components/esp_common/include/esp_attr.h @@ -1,17 +1,9 @@ +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ -// Copyright 2015-2016 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. #ifndef __ESP_ATTR_H__ #define __ESP_ATTR_H__ @@ -33,17 +25,24 @@ extern "C" { // Forces data into DRAM instead of flash #define DRAM_ATTR _SECTION_ATTR_IMPL(".dram1", __COUNTER__) -#ifdef CONFIG_ESP32_IRAM_AS_8BIT_ACCESSIBLE_MEMORY +// IRAM can only be accessed as an 8-bit memory on ESP32, when CONFIG_ESP32_IRAM_AS_8BIT_ACCESSIBLE_MEMORY is set +#define IRAM_8BIT_ACCESSIBLE (CONFIG_IDF_TARGET_ESP32 && CONFIG_ESP32_IRAM_AS_8BIT_ACCESSIBLE_MEMORY) + +// Make sure that IRAM is accessible as an 8-bit memory on ESP32. +// If that's not the case, coredump cannot dump data from IRAM. +#if IRAM_8BIT_ACCESSIBLE // Forces data into IRAM instead of DRAM #define IRAM_DATA_ATTR __attribute__((section(".iram.data"))) // Forces data into IRAM instead of DRAM and map it to coredump -#define COREDUMP_IRAM_DATA_ATTR _SECTION_ATTR_IMPL(".iram.data.coredump", __COUNTER__) +#define COREDUMP_IRAM_DATA_ATTR _SECTION_ATTR_IMPL(".iram2.coredump", __COUNTER__) // Forces bss into IRAM instead of DRAM #define IRAM_BSS_ATTR __attribute__((section(".iram.bss"))) #else -#define COREDUMP_IRAM_DATA_ATTR + +// IRAM is not accessible as an 8-bit memory, put IRAM coredump variables in DRAM +#define COREDUMP_IRAM_DATA_ATTR COREDUMP_DRAM_ATTR #define IRAM_DATA_ATTR #define IRAM_BSS_ATTR @@ -103,7 +102,9 @@ extern "C" { #define RTC_NOINIT_ATTR _SECTION_ATTR_IMPL(".rtc_noinit", __COUNTER__) // Forces code into DRAM instead of flash and map it to coredump -#define COREDUMP_DRAM_ATTR _SECTION_ATTR_IMPL(".dram1.coredump", __COUNTER__) +// Use dram2 instead of dram1 to make sure this section will not be included +// by dram1 section in the linker script +#define COREDUMP_DRAM_ATTR _SECTION_ATTR_IMPL(".dram2.coredump", __COUNTER__) // Forces data into RTC memory and map it to coredump #define COREDUMP_RTC_DATA_ATTR _SECTION_ATTR_IMPL(".rtc.coredump", __COUNTER__) diff --git a/components/esp_system/ld/esp32s2/sections.ld.in b/components/esp_system/ld/esp32s2/sections.ld.in index 2bbfdad5ce..ff716c49cf 100644 --- a/components/esp_system/ld/esp32s2/sections.ld.in +++ b/components/esp_system/ld/esp32s2/sections.ld.in @@ -178,7 +178,8 @@ SECTIONS mapping[iram0_text] - /* added to maintain compability */ + /* Added to maintain compability, there are no iram0 data section to put + * sections:iram_coredump entry defined in espcoredump's linker.lf file */ _coredump_iram_start = 0; _coredump_iram_end = 0; diff --git a/components/espcoredump/linker.lf b/components/espcoredump/linker.lf index f4af6d7c64..fe978a7b6a 100644 --- a/components/espcoredump/linker.lf +++ b/components/espcoredump/linker.lf @@ -8,20 +8,21 @@ entries: [sections:dram_coredump] entries: - .dram1.coredump+ + .dram2.coredump+ -if IDF_TARGET_ESP32S2 = n: - [sections:iram_coredump] - entries: - .iram.data.coredump+ +# Always include .iram2.coredump section in the final linker script file, +# even though it may be empty. The coredump component will ignore empty +# sections when generating the ELF dump. +[sections:iram_coredump] +entries: + .iram2.coredump+ [scheme:coredump_default] entries: dram_coredump -> dram0_data rtc_coredump -> rtc_data rtc_fast_coredump -> rtc_force_fast - if IDF_TARGET_ESP32S2 = n: - iram_coredump -> iram0_data + iram_coredump -> iram0_data [mapping:coredump_default] archive: * @@ -29,10 +30,8 @@ entries: * (coredump_default); rtc_fast_coredump -> rtc_force_fast SURROUND(coredump_rtc_fast), rtc_coredump -> rtc_data SURROUND(coredump_rtc), - dram_coredump -> dram0_data SURROUND(coredump_dram) - if IDF_TARGET_ESP32S2 = n: - * (coredump_default); - iram_coredump -> iram0_data SURROUND(coredump_iram) + dram_coredump -> dram0_data SURROUND(coredump_dram), + iram_coredump -> iram0_data SURROUND(coredump_iram) [mapping:espcoredump] archive: libespcoredump.a From 9a36ced2944deacc729aa9d627716a1a71fdb1ba Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Thu, 30 Dec 2021 13:11:11 +0800 Subject: [PATCH 2/2] espcoredump: add a test for coredump dumped sections --- components/espcoredump/test/test_sections.c | 60 +++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 components/espcoredump/test/test_sections.c diff --git a/components/espcoredump/test/test_sections.c b/components/espcoredump/test/test_sections.c new file mode 100644 index 0000000000..9b27669413 --- /dev/null +++ b/components/espcoredump/test/test_sections.c @@ -0,0 +1,60 @@ +/* + * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include +#include "unity.h" +#include "esp_attr.h" + +/* Global variables that should be part of the coredump */ +COREDUMP_IRAM_DATA_ATTR uint32_t var_iram = 0x42; +COREDUMP_DRAM_ATTR uint32_t var_dram = 0x43; +COREDUMP_RTC_DATA_ATTR uint32_t var_rtc = 0x44; +COREDUMP_RTC_FAST_ATTR uint32_t var_rtcfast = 0x45; + +/* Memory regions to dump, defined at compile time. */ +extern int _coredump_dram_start; +extern int _coredump_dram_end; +extern int _coredump_iram_start; +extern int _coredump_iram_end; +extern int _coredump_rtc_start; +extern int _coredump_rtc_end; +extern int _coredump_rtc_fast_start; +extern int _coredump_rtc_fast_end; + +static inline bool is_addr_in_region(void* addr, uint8_t* region, int region_size) +{ + const void* start = (void*) region; + const void* end = (void*) (region + region_size); + return addr >= start && addr < end; +} + +TEST_CASE("test variables presence in core dump sections", "[espcoredump]") +{ + uint32_t section_start = 0; + uint32_t section_size = 0; + + /* Check DRAM coredump section */ + section_start = (uint32_t)&_coredump_dram_start; + section_size = (uint8_t *)&_coredump_dram_end - (uint8_t *)&_coredump_dram_start; + TEST_ASSERT(section_size > 0); + TEST_ASSERT(is_addr_in_region(&var_dram, (uint8_t*) section_start, section_size)); +#if IRAM_8BIT_ACCESSIBLE + /* Check IRAM coredump section */ + section_start = (uint32_t)&_coredump_iram_start; + section_size = (uint8_t *)&_coredump_iram_end - (uint8_t *)&_coredump_iram_start; + TEST_ASSERT(section_size > 0); + TEST_ASSERT(is_addr_in_region(&var_iram, (uint8_t*) section_start, section_size)); +#endif + /* Check RTC coredump section */ + section_start = (uint32_t)&_coredump_rtc_start; + section_size = (uint8_t *)&_coredump_rtc_end - (uint8_t *)&_coredump_rtc_start; + TEST_ASSERT(section_size > 0); + TEST_ASSERT(is_addr_in_region(&var_rtc, (uint8_t*) section_start, section_size)); + /* Check RTC Fast coredump section */ + section_start = (uint32_t)&_coredump_rtc_fast_start; + section_size = (uint8_t *)&_coredump_rtc_fast_end - (uint8_t *)&_coredump_rtc_fast_start; + TEST_ASSERT(section_size > 0); + TEST_ASSERT(is_addr_in_region(&var_rtcfast, (uint8_t*) section_start, section_size)); +}