Merge branch 'feature/iram_optimization_app_elf_sha256_2' into 'master'

esp_app_format: IRAM space optimization

Closes IDF-7131

See merge request espressif/esp-idf!23572

components/app_update/esp_ota_app_desc.c

@@ -15,7 +15,7 @@ const esp_app_desc_t *esp_ota_get_app_description(void)
     return esp_app_get_description();
 }
 
-int IRAM_ATTR esp_ota_get_app_elf_sha256(char* dst, size_t size)
+int esp_ota_get_app_elf_sha256(char* dst, size_t size)
 {
    return esp_app_get_elf_sha256(dst, size);
 }

components/esp_app_format/Kconfig.projbuild

@@ -39,12 +39,14 @@ menu "Application manager"
 
     config APP_RETRIEVE_LEN_ELF_SHA
         int "The length of APP ELF SHA is stored in RAM(chars)"
-        default 16
+        default 9
         range 8 64
         help
-            At startup, the app will read this many hex characters from the embedded APP ELF SHA-256 hash value
-            and store it in static RAM. This ensures the app ELF SHA-256 value is always available
-            if it needs to be printed by the panic handler code.
-            Changing this value will change the size of a static buffer, in bytes.
+            At startup, the app will read the embedded APP ELF SHA-256 hash value from flash
+            and convert it into a string and store it in a RAM buffer.
+            This ensures the panic handler and core dump will be able to print this string
+            even when cache is disabled.
+            The size of the buffer is APP_RETRIEVE_LEN_ELF_SHA plus the null terminator.
+            Changing this value will change the size of this buffer, in bytes.
 
 endmenu # "Application manager"

components/esp_app_format/esp_app_desc.c

@@ -6,8 +6,8 @@
 
 #include <assert.h>
 #include <sys/param.h>
+#include <string.h>
 #include "esp_app_desc.h"
-#include "esp_attr.h"
 #include "sdkconfig.h"
 
 
@@ -56,48 +56,43 @@ const esp_app_desc_t *esp_app_get_description(void)
     return &esp_app_desc;
 }
 
-/* The following two functions may be called from the panic handler
- * or core dump, hence IRAM_ATTR.
- */
-
-static inline char IRAM_ATTR to_hex_digit(unsigned val)
-{
-    return (val < 10) ? ('0' + val) : ('a' + val - 10);
-}
-
-__attribute__((constructor)) void esp_init_app_elf_sha256(void)
-{
-    esp_app_get_elf_sha256(NULL, 0);
-}
+char app_elf_sha256_str[CONFIG_APP_RETRIEVE_LEN_ELF_SHA + 1] = { 0 };
 
-/* The esp_app_desc.app_elf_sha256 should be possible to print in panic handler during cache is disabled.
+/* The esp_app_desc.app_elf_sha256 should be possible to print in panic handler or core dump during cache is disabled.
  * But because the cache is disabled the reading esp_app_desc.app_elf_sha256 is not right and
  * can lead to a complete lock-up of the CPU.
- * For this reason we do a reading of esp_app_desc.app_elf_sha256 while start up in esp_init_app_elf_sha256()
- * and keep it in the static s_app_elf_sha256 value.
+ * For this reason we do a reading of esp_app_desc.app_elf_sha256 and convert to string while start up in esp_system_init_app_elf_sha256()
+ * and keep it in the static app_elf_sha256_str variable.
  */
-int IRAM_ATTR esp_app_get_elf_sha256(char* dst, size_t size)
+__attribute__((constructor)) void esp_app_format_init_elf_sha256(void)
 {
-    static char s_app_elf_sha256[CONFIG_APP_RETRIEVE_LEN_ELF_SHA / 2];
-    static bool first_call = true;
-    if (first_call) {
-        first_call = false;
-        // At -O2 optimization level, GCC optimizes out the copying of the first byte of the app_elf_sha256,
-        // because it is zero at compile time, and only modified afterwards by esptool.
-        // Casting to volatile disables the optimization.
-        const volatile uint8_t* src = (const volatile uint8_t*)esp_app_desc.app_elf_sha256;
-        for (size_t i = 0; i < sizeof(s_app_elf_sha256); ++i) {
-            s_app_elf_sha256[i] = src[i];
+    if (*((int *)&app_elf_sha256_str) != 0) {
+        // app_elf_sha256_str is already set
+        return;
+    }
+    // At -O2 optimization level, GCC optimizes out the copying of the first byte of the app_elf_sha256,
+    // because it is zero at compile time, and only modified afterwards by esptool.
+    // Casting to volatile disables the optimization.
+    const volatile char* src = (const volatile char*)esp_app_desc.app_elf_sha256;
+    for (size_t i = 0; i < sizeof(app_elf_sha256_str) / 2; ++i) {
+        char c = src[i];
+        for (size_t s = 0; s < 2; ++s) {
+            char val = (c >> 4) & 0xF;
+            app_elf_sha256_str[2 * i + s] = (val < 10) ? ('0' + val) : ('a' + val - 10);
+            c <<= 4;
         }
     }
-    if (dst == NULL || size == 0) {
+    app_elf_sha256_str[sizeof(app_elf_sha256_str) - 1] = 0;
+}
+
+int esp_app_get_elf_sha256(char* dst, size_t size)
+{
+    if (dst == NULL || size < 2) {
         return 0;
     }
-    size_t n = MIN((size - 1) / 2, sizeof(s_app_elf_sha256));
-    for (size_t i = 0; i < n; ++i) {
-        dst[2*i] = to_hex_digit(s_app_elf_sha256[i] >> 4);
-        dst[2*i + 1] = to_hex_digit(s_app_elf_sha256[i] & 0xf);
-    }
-    dst[2*n] = 0;
-    return 2*n + 1;
+    esp_app_format_init_elf_sha256();
+    size_t n = MIN(size, sizeof(app_elf_sha256_str));
+    memcpy(dst, app_elf_sha256_str, n);
+    dst[n - 1] = 0;
+    return n;
 }

components/esp_app_format/include/esp_app_desc.h

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -11,6 +11,7 @@
 #include <stddef.h>
 #include "esp_err.h"
 #include "esp_assert.h"
+#include "esp_attr.h"
 
 #ifdef __cplusplus
 extern "C"
@@ -57,6 +58,21 @@ const esp_app_desc_t *esp_app_get_description(void);
  */
 int esp_app_get_elf_sha256(char* dst, size_t size);
 
+/** @cond */
+extern char app_elf_sha256_str[];
+/** @endcond */
+
+/**
+ * @brief   Return SHA256 of the ELF file which is already formatted as hexadecimal, null-terminated included.
+ * Can be used in panic handler or core dump during when cache is disabled.
+ * The length is defined by CONFIG_APP_RETRIEVE_LEN_ELF_SHA option.
+ * @return  Hexadecimal SHA256 string
+ */
+FORCE_INLINE_ATTR char *esp_app_get_elf_sha256_str(void)
+{
+    return app_elf_sha256_str;
+}
+
 #ifdef __cplusplus
 }
 #endif

components/esp_app_format/test_apps/main/test_app_desc.c

@@ -55,16 +55,32 @@ TEST(esp_app_format, esp_app_get_elf_sha256_test)
     TEST_ASSERT_EQUAL_HEX(0, dst[8]);
     TEST_ASSERT_EQUAL_HEX(fill, dst[9]);
 
-    memset(dst, fill, sizeof(dst));
-    len = 8;
-    res = esp_app_get_elf_sha256(dst, len);
-    printf("%d: %s (%d)\n", len, dst, res);
-    // should output even number of characters plus '\0'
-    TEST_ASSERT_EQUAL(7, res);
-    TEST_ASSERT_EQUAL(0, memcmp(dst, ref_sha256, res - 1));
-    TEST_ASSERT_EQUAL_HEX(0, dst[6]);
-    TEST_ASSERT_EQUAL_HEX(fill, dst[7]);
-    TEST_ASSERT_EQUAL_HEX(fill, dst[8]);
+    memset(ref_sha256, 0xCC, sizeof(ref_sha256));
+    strncpy(ref_sha256, esp_app_get_elf_sha256_str(), sizeof(ref_sha256));
+    len = strlen(ref_sha256);
+    TEST_ASSERT_EQUAL(CONFIG_APP_RETRIEVE_LEN_ELF_SHA, len);
+    printf("\n_Ref: %s (len=%d with null)\n", ref_sha256, len);
+
+    TEST_ASSERT_EQUAL(0, esp_app_get_elf_sha256(dst, 0));
+    TEST_ASSERT_EQUAL(0, esp_app_get_elf_sha256(dst, 1));
+    TEST_ASSERT_EQUAL(0, esp_app_get_elf_sha256(NULL, 99));
+
+    for (size_t req_len = 2; req_len <= CONFIG_APP_RETRIEVE_LEN_ELF_SHA + 1; req_len++) {
+        memset(dst, 0xCC, sizeof(dst));
+        TEST_ASSERT_EQUAL(req_len, esp_app_get_elf_sha256(dst, req_len));
+        len = strlen(dst) + 1; // + 1 for the null terminator
+        printf("_%02d_: %-15s (len=%d with null)\n", req_len, dst, len);
+        TEST_ASSERT_EQUAL(req_len, len);
+        TEST_ASSERT_EQUAL_STRING_LEN(ref_sha256, dst, len - 1); // -1 without null terminator
+    }
+
+    memset(dst, 0xCC, sizeof(dst));
+    size_t max_len = CONFIG_APP_RETRIEVE_LEN_ELF_SHA + 1; // + 1 for the null terminator
+    TEST_ASSERT_EQUAL(max_len, esp_app_get_elf_sha256(dst, 99));
+    len = strlen(dst) + 1; // + 1 for the null terminator
+    printf("_99_: %-15s (len=%d with null)\n", dst, len);
+    TEST_ASSERT_EQUAL(max_len, len);
+    TEST_ASSERT_EQUAL_STRING_LEN(ref_sha256, dst, len - 1); // -1 without null terminator
 }
 
 TEST_GROUP_RUNNER(esp_app_format)

components/esp_system/CMakeLists.txt

@@ -110,7 +110,9 @@ endif()
 # need to introduce panic "event" concept to remove this dependency (IDF-2194)
 idf_component_optional_requires(PRIVATE esp_gdbstub)
 
-idf_component_optional_requires(PRIVATE esp_app_format)
+if(NOT CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT)
+    idf_component_optional_requires(PRIVATE esp_app_format)
+endif()
 
 if(CONFIG_PM_ENABLE)
     idf_component_optional_requires(PRIVATE pm)

components/esp_system/panic.c

@@ -26,10 +26,12 @@
 
 #include "sdkconfig.h"
 
+#if !CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT
 #if __has_include("esp_app_desc.h")
 #define WITH_ELF_SHA256
 #include "esp_app_desc.h"
 #endif
+#endif // CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT
 
 #if CONFIG_ESP_COREDUMP_ENABLE
 #include "esp_core_dump.h"
@@ -332,11 +334,9 @@ void esp_panic_handler(panic_info_t *info)
 
 #ifdef WITH_ELF_SHA256
     panic_print_str("\r\nELF file SHA256: ");
-    char sha256_buf[65];
-    esp_app_get_elf_sha256(sha256_buf, sizeof(sha256_buf));
-    panic_print_str(sha256_buf);
+    panic_print_str(esp_app_get_elf_sha256_str());
     panic_print_str("\r\n");
-#endif
+#endif // WITH_ELF_SHA256
 
     panic_print_str("\r\n");
 

components/espcoredump/src/core_dump_elf.c

@@ -498,7 +498,11 @@ static int elf_write_core_dump_info(core_dump_elf_t *self)
 
     ESP_COREDUMP_LOG_PROCESS("================ Processing coredump info ================");
     int data_len = (int)sizeof(self->elf_version_info.app_elf_sha256);
-    data_len = esp_app_get_elf_sha256((char*)self->elf_version_info.app_elf_sha256, (size_t)data_len);
+    // This dump function can be called when the cache is diable which requires putting
+    // esp_app_get_elf_sha256() into IRAM. But we want to reduce IRAM usage,
+    // so we use a function that returns an already formatted string.
+    strncpy((char*)self->elf_version_info.app_elf_sha256, esp_app_get_elf_sha256_str(), (size_t)data_len);
+    data_len = strlen((char*)self->elf_version_info.app_elf_sha256) + 1; // + 1 for the null terminator
     ESP_COREDUMP_LOG_PROCESS("Application SHA256='%s', length=%d.",
                                 self->elf_version_info.app_elf_sha256, data_len);
     self->elf_version_info.version = esp_core_dump_elf_version();

examples/peripherals/timer_group/gptimer/pytest_gptimer_example.py

@@ -30,7 +30,7 @@ def test_gptimer_example(dut: Dut) -> None:
     for i in range(1,5):
         res = dut.expect(r'Timer alarmed, count=(\d+)', timeout=5)
         alarm_count = res.group(1).decode('utf8')
-        assert (i * 1000000 - 10) < int(alarm_count) < (i * 1000000 + 10)
+        assert (i * 1000000 - 20) < int(alarm_count) < (i * 1000000 + 20)
 
     dut.expect_exact('Stop timer')
     dut.expect_exact('Delete timer')

tools/test_apps/system/panic/test_panic_util/panic_dut.py

@@ -92,7 +92,7 @@ def expect_elf_sha256(self) -> None:
         """Expect method for ELF SHA256 line"""
         elf_sha256 = sha256(self.app.elf_file)
         elf_sha256_len = int(
-            self.app.sdkconfig.get('CONFIG_APP_RETRIEVE_LEN_ELF_SHA', '16')
+            self.app.sdkconfig.get('CONFIG_APP_RETRIEVE_LEN_ELF_SHA', '9')
         )
         self.expect_exact('ELF file SHA256: ' + elf_sha256[0:elf_sha256_len])