fix(ulp): Added unit tests for ulp binary embed with prefix and misc fixes

This commit:
- Removes the link time symbol name clash detection.
- Extracts symbols of type NOTYPE for global identifiers defined in
  assembly files.
- Makes the prefix argument optional for ulp_add_build_binary_targets().
- Adds a unit test for the ulp binary embed with a prefix feature.

components/ulp/cmake/CMakeLists.txt

@@ -8,4 +8,4 @@ include(IDFULPProject)
 
 ulp_apply_default_options(${ULP_APP_NAME})
 ulp_apply_default_sources(${ULP_APP_NAME})
-ulp_add_build_binary_targets(${ULP_APP_NAME} ${ULP_VAR_PREFIX})
+ulp_add_build_binary_targets(${ULP_APP_NAME} PREFIX ${ULP_VAR_PREFIX})

components/ulp/cmake/IDFULPProject.cmake

@@ -80,6 +80,7 @@ function(ulp_apply_default_sources ulp_app_name)
 
     # To avoid warning "Manually-specified variables were not used by the project"
     set(bypassWarning "${IDF_TARGET}")
+    set(bypassWarning "${ULP_VAR_PREFIX}")
 
     if(CONFIG_ULP_COPROC_TYPE_RISCV)
         #risc-v ulp uses extra files for building:
@@ -178,7 +179,11 @@ function(ulp_apply_default_sources ulp_app_name)
     endif()
 endfunction()
 
-function(ulp_add_build_binary_targets ulp_app_name prefix)
+function(ulp_add_build_binary_targets ulp_app_name)
+    cmake_parse_arguments(ULP "" "PREFIX" "" ${ARGN})
+    if(NOT ULP_PREFIX)
+        set(ULP_PREFIX "ulp_")
+    endif()
 
     if(ADD_PICOLIBC_SPECS)
         target_compile_options(${ulp_app_name} PRIVATE $<$<COMPILE_LANG_AND_ID:C,GNU>:-specs=picolibc.specs>)
@@ -207,7 +212,7 @@ function(ulp_add_build_binary_targets ulp_app_name prefix)
 
     add_custom_command(OUTPUT ${ulp_app_name}.ld ${ulp_app_name}.h
                     COMMAND ${ULP_MAP_GEN} -s ${ulp_app_name}.sym -o ${ulp_app_name}
-                            --base ${ULP_BASE_ADDR} --prefix ${prefix}
+                            --base ${ULP_BASE_ADDR} --prefix ${ULP_PREFIX}
                     DEPENDS ${ulp_app_name}.sym
                     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 

components/ulp/esp32ulp_mapgen.py

@@ -43,7 +43,7 @@ def gen_ld_h_from_sym(f_sym: typing.TextIO, f_ld: typing.TextIO, f_h: typing.Tex
         tmp = prefix.split('::')
         namespace = tmp[0]
         var_prefix = '_'.join(tmp[1:])  # Limit to a single namespace here to avoid complex mangling rules
-        f_h.write('namespace {0} {{\n'.format(namespace))
+        f_h.write(f'namespace {namespace} {{\n')
         cpp_mode = True
     else:
         f_h.write(textwrap.dedent(
@@ -54,25 +54,38 @@ def gen_ld_h_from_sym(f_sym: typing.TextIO, f_ld: typing.TextIO, f_h: typing.Tex
             #pragma once
             #ifdef __cplusplus
             extern "C" {{
-            #endif
+            #endif\n
             """  # noqa: E222
         ))
 
-    expr = re.compile('^\\s*\\d+: ([a-f0-9]{8})\\s+(\\d+) OBJECT\\s+GLOBAL\\s+DEFAULT\\s+[^ ]+ (.*)$')
+    # Format the regular expression to match the readelf output
-    already_defined = 'this_symbol_is_already_defined_please_use_prefix_in_ulp_embed_binary'
+    expr = re.compile(r'^.*(?P<address>[a-f0-9]{8})\s+(?P<size>\d+) (OBJECT|NOTYPE)\s+GLOBAL\s+DEFAULT\s+[^ ]+ (?P<name>.*)$')
     for line in f_sym:
         # readelf format output has the following structure:
-        #   index: addr_hex     size TYPE  SCOPE DEFAULT    junk symbol_name
+        #   Num:    Value  Size Type    Bind   Vis      Ndx Name
         # So match the line with a regular expression to parse it first
         groups = expr.match(line)
-        if groups is None:  # Ignore non global or non object
+
+        # Ignore lines that do not match the expected format such as non global symbols
+        if groups is None:
             continue
-        addr = int(groups.group(1), 16) + base_addr
+
-        size = int(groups.group(2))
+        # Extract the symbol information
-        name = var_prefix + groups.group(3)
+        addr = int(groups.group('address'), 16) + base_addr
-        f_h.write('extern uint32_t {0}{1};\n'.format(name, '[{0}]'.format(int(size / 4)) if size > 4 else ''))
+        size = int(groups.group('size'))
-        f_ld.write('{0} = DEFINED({0}) ? {2} : 0x{1:08x};\n'.format(
+        sym_name = groups.group('name')
-            name_mangling(namespace + '::' + name) if cpp_mode else name, addr, already_defined))
+
+        # Add the variable prefix if provided
+        var_name = var_prefix + sym_name
+
+        # Get the dimension of the variable if it is an array
+        var_dimension = f'[{size // 4}]' if size > 4 else ''  # Use // to automatically perform an integer division
+
+        # Write the variable definition to the header file and the address to the linker script
+        f_h.write(f'extern uint32_t {var_name}{var_dimension};\n')
+
+        name_in_ld = name_mangling(namespace + '::' + var_name) if cpp_mode else var_name
+        f_ld.write(f'{name_in_ld} = 0x{addr:08x};\n')
 
     if cpp_mode:
         f_h.write('}\n')

components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt

@@ -24,6 +24,8 @@ if(CONFIG_SOC_LP_VAD_SUPPORTED)
     list(APPEND app_sources "test_lp_core_vad.c")
 endif()
 
+list(APPEND app_sources "test_lp_core_prefix.c")
+
 set(lp_core_sources         "lp_core/test_main.c")
 set(lp_core_sources_counter "lp_core/test_main_counter.c")
 
@@ -92,3 +94,6 @@ endif()
 if(CONFIG_SOC_LP_VAD_SUPPORTED)
     ulp_embed_binary(lp_core_test_app_vad "${lp_core_sources_vad}" "${lp_core_exp_dep_srcs}")
 endif()
+
+ulp_embed_binary(lp_core_test_app_prefix1 "lp_core/test_main_prefix1.c"  "${lp_core_exp_dep_srcs}" PREFIX "ulp1_")
+ulp_embed_binary(lp_core_test_app_prefix2 "lp_core/test_main_prefix2.c"  "${lp_core_exp_dep_srcs}" PREFIX "ulp2_")

components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix1.c

@@ -0,0 +1,18 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/* Global variable with the same name across different ULP binaries */
+volatile int g_var;
+
+int main(void)
+{
+    g_var = 1;
+
+    while (1)
+        ;
+
+    return 0;
+}

components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix2.c

@@ -0,0 +1,25 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/* A global variable to make sure that g_var doesn't have the same address across different ULP binaries */
+volatile int g_array[10];
+
+/* Global variable with the same name across different ULP binaries */
+volatile int g_var;
+
+int main(void)
+{
+    int i;
+    for (i = 0; i < 10; i++) {
+        g_array[i] = i;
+    }
+    g_var = 2;
+
+    while (1)
+        ;
+
+    return 0;
+}

components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_prefix.c

@@ -0,0 +1,58 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <stdint.h>
+#include "lp_core_test_app_prefix1.h"
+#include "lp_core_test_app_prefix2.h"
+#include "ulp_lp_core.h"
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "test_shared.h"
+#include "unity.h"
+#include "test_utils.h"
+
+extern const uint8_t lp_core_main_prefix1_bin_start[] asm("_binary_lp_core_test_app_prefix1_bin_start");
+extern const uint8_t lp_core_main_prefix1_bin_end[]   asm("_binary_lp_core_test_app_prefix1_bin_end");
+extern const uint8_t lp_core_main_prefix2_bin_start[] asm("_binary_lp_core_test_app_prefix2_bin_start");
+extern const uint8_t lp_core_main_prefix2_bin_end[]   asm("_binary_lp_core_test_app_prefix2_bin_end");
+
+static void load_and_start_lp_core_firmware(ulp_lp_core_cfg_t* cfg, const uint8_t* firmware_start, const uint8_t* firmware_end)
+{
+    TEST_ASSERT(ulp_lp_core_load_binary(firmware_start,
+                                        (firmware_end - firmware_start)) == ESP_OK);
+
+    TEST_ASSERT(ulp_lp_core_run(cfg) == ESP_OK);
+}
+
+TEST_CASE("LP-Core header prefix test", "[lp_core]")
+{
+    /* Load ULP firmware and start the coprocessor */
+    ulp_lp_core_cfg_t cfg = {
+        .wakeup_source = ULP_LP_CORE_WAKEUP_SOURCE_HP_CPU,
+#if ESP_ROM_HAS_LP_ROM
+        .skip_lp_rom_boot = true,
+#endif //ESP_ROM_HAS_LP_ROM
+    };
+
+    /* Load and run the first LP core firmware */
+    load_and_start_lp_core_firmware(&cfg, lp_core_main_prefix1_bin_start, lp_core_main_prefix1_bin_end);
+    vTaskDelay(10);
+
+    /* Verify the shared variable value */
+    TEST_ASSERT_EQUAL(1, ulp1_g_var);
+
+    /* Load and run the second LP core firmware */
+    load_and_start_lp_core_firmware(&cfg, lp_core_main_prefix2_bin_start, lp_core_main_prefix2_bin_end);
+    vTaskDelay(10);
+
+    /* Verify that a global array can be accessed as an array on the HP CPU */
+    for (int i = 0; i < 10; i++) {
+        TEST_ASSERT_EQUAL(i, ulp2_g_array[i]);
+    }
+
+    /* Verify that the shared variable with the same name is updated once the second LP core binary runs */
+    TEST_ASSERT_EQUAL(2, ulp2_g_var);
+}

docs/en/api-reference/system/ulp-lp-core.rst

@@ -50,7 +50,7 @@ If you need to embed multiple ULP programs, you may add a custom prefix in order
 
     ulp_embed_binary(${ulp_app_name} "${ulp_sources}" "${ulp_exp_dep_srcs}" PREFIX "ULP::")
 
-The additional argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``).
+The additional PREFIX argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``).
 
 Using a Custom CMake Project
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

docs/en/api-reference/system/ulp-risc-v.rst

@@ -53,7 +53,7 @@ If you need to embed multiple ULP programs, you may add a custom prefix in order
 
     ulp_embed_binary(${ulp_app_name} "${ulp_sources}" "${ulp_exp_dep_srcs}" PREFIX "ULP::")
 
-The additional argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``).
+The additional PREFIX argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``).
 
 Using a Custom CMake Project
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

docs/en/api-reference/system/ulp.rst

@@ -64,7 +64,7 @@ If you need to embed multiple ULP programs, you may add a custom prefix in order
 
     ulp_embed_binary(${ulp_app_name} "${ulp_sources}" "${ulp_exp_dep_srcs}" PREFIX "ULP::")
 
-The additional argument can be a C style prefix (like ``ulp2_``).
+The additional PREFIX argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``).
 
 3. Build the application as usual (e.g., ``idf.py app``).