From e32831033ae449b83b11da166294c09b79f71aee Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Fri, 11 Jun 2021 11:56:58 +0800 Subject: [PATCH] [esp_rom]: fixed S3 longjmp patch * On S3, the placement of ROM functions is ECO-dependent. Hence, we don't jump into the middle of the longjmp function in ROM on S3 anymore. Instead, the whole longjump function is used in the patch. * Also properly excluded the patch from bootloader build with Makefiles Closes IDF-3391 --- components/esp_rom/CMakeLists.txt | 6 +- components/esp_rom/component.mk | 10 ++- components/esp_rom/patches/esp_rom_longjmp.S | 86 ++++++++++++++++++- .../system/longjmp_test/main/component.mk | 4 + .../longjmp_test/main/hello_world_main.c | 2 +- 5 files changed, 97 insertions(+), 11 deletions(-) create mode 100644 tools/test_apps/system/longjmp_test/main/component.mk diff --git a/components/esp_rom/CMakeLists.txt b/components/esp_rom/CMakeLists.txt index e05a3d4e48..60e0122101 100644 --- a/components/esp_rom/CMakeLists.txt +++ b/components/esp_rom/CMakeLists.txt @@ -4,8 +4,7 @@ set(sources "patches/esp_rom_crc.c" "patches/esp_rom_sys.c" "patches/esp_rom_uart.c") -if(CONFIG_IDF_TARGET_ARCH_XTENSA AND NOT (target STREQUAL "esp32s3")) - # Temporarily disabled on S3 due to it breaking longjmp TODO ESP32S3 IDF-3391 +if(CONFIG_IDF_TARGET_ARCH_XTENSA) list(APPEND sources "patches/esp_rom_longjmp.S") endif() @@ -103,8 +102,7 @@ else() # Regular app build endif() endif() - if(CONFIG_IDF_TARGET_ARCH_XTENSA AND NOT (target STREQUAL "esp32s3")) - # Temporarily disabled on S3 due to it breaking longjmp TODO ESP32S3 IDF-3391 + if(CONFIG_IDF_TARGET_ARCH_XTENSA) target_link_libraries(${COMPONENT_LIB} INTERFACE "-Wl,--wrap=longjmp") endif() endif() diff --git a/components/esp_rom/component.mk b/components/esp_rom/component.mk index 4f53003366..c68101573e 100644 --- a/components/esp_rom/component.mk +++ b/components/esp_rom/component.mk @@ -1,6 +1,10 @@ COMPONENT_ADD_INCLUDEDIRS := include esp32 include/esp32 COMPONENT_SRCDIRS := patches . +ifdef IS_BOOTLOADER_BUILD +COMPONENT_OBJEXCLUDE := patches/esp_rom_longjmp.o +endif + #Linker scripts used to link the final application. #Warning: These linker scripts are only used when the normal app is compiled; the bootloader #specifies its own scripts. @@ -38,7 +42,9 @@ LINKER_SCRIPTS += esp32.rom.newlib-time.ld endif COMPONENT_ADD_LDFLAGS += -L $(COMPONENT_PATH)/esp32/ld \ - $(addprefix -T ,$(LINKER_SCRIPTS)) \ - -l$(COMPONENT_NAME) -Wl,--wrap=longjmp \ + $(addprefix -T ,$(LINKER_SCRIPTS)) +ifndef IS_BOOTLOADER_BUILD +COMPONENT_ADD_LDFLAGS += -l$(COMPONENT_NAME) -Wl,--wrap=longjmp +endif COMPONENT_ADD_LINKER_DEPS += $(addprefix esp32/ld/, $(LINKER_SCRIPTS)) diff --git a/components/esp_rom/patches/esp_rom_longjmp.S b/components/esp_rom/patches/esp_rom_longjmp.S index 7f2de71737..d76ac4f7e6 100644 --- a/components/esp_rom/patches/esp_rom_longjmp.S +++ b/components/esp_rom/patches/esp_rom_longjmp.S @@ -29,10 +29,9 @@ */ #include +#include -/* - Replacement of the first instructions of void longjmp (jmp_buf env, int val) -*/ +/* void longjmp (jmp_buf env, int val) */ .align 4 .literal_position @@ -59,7 +58,11 @@ __wrap_longjmp: /* Activate interrupts again after modifying WINDOWBASE and WINDOWSTART. */ wsr a7, PS - /* Jump back to original longjmp implementation. +#if !CONFIG_IDF_TARGET_ESP32S3 + + /* + If not on S3, replacement of only the first instructions, + then jump back to original longjmp implementation. The jump target is the instrucion l32i a0, a2, 64 of the original code. Hence, the original code's entry instruction and windowstart modification are left @@ -69,3 +72,78 @@ __wrap_longjmp: jx a0 .size __wrap_longjmp, . - __wrap_longjmp + +#else /* CONFIG_IDF_TARGET_ESP32S3 */ + /* + If on S3, we replace the whole function for simplicity. The placement of longjmp in ROM is ECO-dependent + on S3. + */ + + /* + Return to the return address of the setjmp, using the + window size bits from the setjmp call so that the caller + will be able to find the return value that we put in a2. */ + + l32i a0, a2, 64 + + /* Copy the first 4 saved registers from jmp_buf into the save area + at the current sp so that the values will be restored to registers + when longjmp returns. */ + + addi a7, a1, -16 + l32i a4, a2, 0 + l32i a5, a2, 4 + s32i a4, a7, 0 + s32i a5, a7, 4 + l32i a4, a2, 8 + l32i a5, a2, 12 + s32i a4, a7, 8 + s32i a5, a7, 12 + + /* Copy the remaining 0-8 saved registers. */ + extui a7, a0, 30, 2 + blti a7, 2, .Lendlj + l32i a8, a2, 52 + slli a4, a7, 4 + sub a6, a8, a4 + addi a5, a2, 16 + addi a8, a8, -16 // a8 = end of register overflow area +.Lljloop: + l32i a7, a5, 0 + l32i a4, a5, 4 + s32i a7, a6, 0 + s32i a4, a6, 4 + l32i a7, a5, 8 + l32i a4, a5, 12 + s32i a7, a6, 8 + s32i a4, a6, 12 + addi a5, a5, 16 + addi a6, a6, 16 + blt a6, a8, .Lljloop +.Lendlj: + + /* The 4 words saved from the register save area at the target's + sp are copied back to the target procedure's save area. The + only point of this is to prevent a catastrophic failure in + case the contents were moved by an alloca after calling + setjmp. This is a bit paranoid but it doesn't cost much. */ + + l32i a7, a2, 4 // load the target stack pointer + addi a7, a7, -16 // find the destination save area + l32i a4, a2, 48 + l32i a5, a2, 52 + s32i a4, a7, 0 + s32i a5, a7, 4 + l32i a4, a2, 56 + l32i a5, a2, 60 + s32i a4, a7, 8 + s32i a5, a7, 12 + + /* Return val ? val : 1. */ + movi a2, 1 + movnez a2, a3, a3 + + retw + .size __wrap_longjmp, . - __wrap_longjmp + +#endif /* CONFIG_IDF_TARGET_ESP32S3 */ diff --git a/tools/test_apps/system/longjmp_test/main/component.mk b/tools/test_apps/system/longjmp_test/main/component.mk new file mode 100644 index 0000000000..a98f634eae --- /dev/null +++ b/tools/test_apps/system/longjmp_test/main/component.mk @@ -0,0 +1,4 @@ +# +# "main" pseudo-component makefile. +# +# (Uses default behaviour of compiling all source files in directory, adding 'include' to include path.) diff --git a/tools/test_apps/system/longjmp_test/main/hello_world_main.c b/tools/test_apps/system/longjmp_test/main/hello_world_main.c index c358efa334..490644cc34 100644 --- a/tools/test_apps/system/longjmp_test/main/hello_world_main.c +++ b/tools/test_apps/system/longjmp_test/main/hello_world_main.c @@ -110,7 +110,7 @@ void app_main(void) } vTaskDelay(10000); - printf("stopping timers...\n"); + printf("stopping timer...\n"); esp_timer_stop(crash_timer); esp_timer_delete(crash_timer);