Merge branch 'bugfix/coredump_data_section' into 'master'

espcoredump: fix a bug where tracked DRAM data were not dumped

Closes IDFGH-6498

See merge request espressif/esp-idf!16573

components/esp_common/include/esp_attr.h

@@ -1,6 +1,6 @@
 
 /*
- * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -25,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
@@ -95,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__)

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;
 

components/espcoredump/linker.lf

@@ -8,20 +8,21 @@ entries:
 
 [sections:dram_coredump]
 entries:
-    .dram1.coredump+
+    .dram2.coredump+
 
-if IDF_TARGET_ESP32S2 = n:
+# Always include .iram2.coredump section in the final linker script file,
-    [sections:iram_coredump]
+# even though it may be empty. The coredump component will ignore empty
-    entries:
+# sections when generating the ELF dump.
-        .iram.data.coredump+
+[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)
+        dram_coredump -> dram0_data SURROUND(coredump_dram),
-    if IDF_TARGET_ESP32S2 = n:
+        iram_coredump -> iram0_data SURROUND(coredump_iram)
-        * (coredump_default);
-            iram_coredump -> iram0_data SURROUND(coredump_iram)
 
 [mapping:espcoredump]
 archive: libespcoredump.a

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 <string.h>
+#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));
+}