Selaa lähdekoodia

[system]: Made longjmp save for context switch

* Patched longjmp to be context-switch safe
  longjmp modifies the windowbase and windowstart
  registers, which isn't safe if a context switch
  occurs during the modification. After a context
  switch, windowstart and windowbase will be
  different, leading to a wrongly set windowstart
  bit due to longjmp writing it based on the
  windowbase before the context switch. This
  corrupts the registers at the next window
  overflow reaching that wrongly set bit.

  The solution is to disable interrupts during
  this code. It is only 6 instructions long,
  the impact shouldn't be significant.

  The fix is implemented as a wrapper which
  replaces the original first instructions of
  longjmp which are buggy. Then, it jumps back
  to execute the rest of the original longjmp
  function.

  Added a comparably reliable test to the
  test apps.
Jakob Hasse 4 vuotta sitten
vanhempi
sitoutus
aabdb2c7a1

+ 2 - 0
components/bootloader/subproject/Makefile

@@ -32,4 +32,6 @@ CFLAGS += -D BOOTLOADER_BUILD=1
 # include the top-level "project" include directory, for sdkconfig.h
 CFLAGS += -I$(BUILD_DIR_BASE)/../include
 
+COMPONENT_ADD_LDFLAGS += -l$(COMPONENT_NAME) -Wl,--wrap=longjmp \
+
 include $(IDF_PATH)/make/project.mk

+ 4 - 1
components/esp_rom/CMakeLists.txt

@@ -1,6 +1,8 @@
 idf_build_get_property(target IDF_TARGET)
 
-idf_component_register(INCLUDE_DIRS include)
+idf_component_register(SRCS "patches/esp_rom_longjmp.S"
+                       INCLUDE_DIRS include "${target}"
+                       PRIV_REQUIRES soc)
 
 if(BOOTLOADER_BUILD)
     set(scripts
@@ -64,4 +66,5 @@ else() # Regular app build
 
     target_linker_script(${COMPONENT_LIB} INTERFACE "${scripts}")
 
+    target_link_libraries(${COMPONENT_LIB} INTERFACE "-Wl,--wrap=longjmp")
 endif()

+ 3 - 0
components/esp_rom/component.mk

@@ -33,7 +33,10 @@ ifndef CONFIG_SDK_TOOLCHAIN_SUPPORTS_TIME_WIDE_64_BITS
 LINKER_SCRIPTS += esp32.rom.newlib-time.ld
 endif
 
+COMPONENT_SRCDIRS := patches
+
 COMPONENT_ADD_LDFLAGS += -L $(COMPONENT_PATH)/esp32/ld \
                          $(addprefix -T ,$(LINKER_SCRIPTS)) \
+                         -l$(COMPONENT_NAME) -Wl,--wrap=longjmp \
 
 COMPONENT_ADD_LINKER_DEPS += $(addprefix esp32/ld/, $(LINKER_SCRIPTS))

+ 71 - 0
components/esp_rom/patches/esp_rom_longjmp.S

@@ -0,0 +1,71 @@
+/*
+    Copyright (c) 2001-2006 by Tensilica Inc.
+
+    Permission is hereby granted, free of charge, to any person obtaining
+    a copy of this software and associated documentation files (the
+    "Software"), to deal in the Software without restriction, including
+    without limitation the rights to use, copy, modify, merge, publish,
+    distribute, sublicense, and/or sell copies of the Software, and to
+    permit persons to whom the Software is furnished to do so, subject to
+    the following conditions:
+
+    The above copyright notice and this permission notice shall be included
+    in all copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+    EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+    MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+    IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+    CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+    TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+    SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+/*
+  This file contains a modified version of the original Xtensa longjmp implementation.
+  In this modified version, setting WINDOWSTART = 1 << WINDOWBASE is done inside a critical section.
+  This is necessary because after a FreeRTOS context switch in IDF, the values of WINDOWBASE and WINDOWSTART
+  are not guaranteed to be the same as before the context switch.
+*/
+
+#include <xtensa/corebits.h>
+
+/*
+    Replacement of the first instructions of void longjmp (jmp_buf env, int val)
+*/
+
+    .align  4
+    .literal_position
+    .global __wrap_longjmp
+    .type   __wrap_longjmp, @function
+__wrap_longjmp:
+    entry   sp, 16
+
+    /* Deactivate interrupts in order to modify WINDOWBASE and WINDOWSTART. */
+    rsr     a7, PS                     /* to be restored after SPILL_ALL_WINDOWS */
+    movi    a5, PS_EXCM                /* PS_INTLEVEL_MASK */
+    or      a5, a7, a5                 /* get the current INTLEVEL */
+    wsr     a5, PS
+
+    /* Invalidate all but the current window;
+       set WindowStart to (1 << WindowBase).  */
+    rsr a5, WINDOWBASE
+    movi    a4, 1
+    ssl a5
+    sll a4, a4
+    wsr a4, WINDOWSTART
+    rsync
+
+    /* Activate interrupts again after modifying WINDOWBASE and WINDOWSTART. */
+    wsr     a7, PS
+
+    /* 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
+        out.
+     */
+    movi a0, __real_longjmp + 20
+    jx a0
+
+    .size   __wrap_longjmp, . - __wrap_longjmp

+ 4 - 0
components/xtensa/component.mk

@@ -5,3 +5,7 @@ COMPONENT_ADD_LDFLAGS += $(COMPONENT_PATH)/esp32/libhal.a
 COMPONENT_ADD_LDFRAGMENTS += linker.lf
 
 COMPONENT_SRCDIRS := . esp32
+
+ifdef IS_BOOTLOADER_BUILD
+	COMPONENT_OBJEXCLUDE := xtensa_intr.o xtensa_intr_asm.o
+endif

+ 8 - 0
tools/ci/config/target-test.yml

@@ -343,6 +343,14 @@ example_test_015:
     - ESP32
     - Example_PPP
 
+test_app_test_esp32_generic:
+  extends: .test_app_template
+  tags:
+    - ESP32
+    - Example_GENERIC
+  variables:
+    SETUP_TOOLS: "1"
+
 test_app_test_001:
   extends: .test_app_template
   tags:

+ 6 - 0
tools/test_apps/system/longjmp_test/CMakeLists.txt

@@ -0,0 +1,6 @@
+# The following lines of boilerplate have to be in your project's
+# CMakeLists in this exact order for cmake to work correctly
+cmake_minimum_required(VERSION 3.5)
+
+include($ENV{IDF_PATH}/tools/cmake/project.cmake)
+project(hello-world)

+ 8 - 0
tools/test_apps/system/longjmp_test/Makefile

@@ -0,0 +1,8 @@
+#
+# This is a project Makefile. It is assumed the directory this Makefile resides in is a
+# project subdirectory.
+#
+
+PROJECT_NAME := longjmp_test
+
+include $(IDF_PATH)/make/project.mk

+ 13 - 0
tools/test_apps/system/longjmp_test/README.md

@@ -0,0 +1,13 @@
+| Supported Targets | ESP32 | ESP32-S2 |
+| ----------------- | ----- | -------- |
+
+# Building
+Example building for ESP32:
+```
+idf.py set-target esp32
+cp sdkconfig.defaults sdkconfig
+idf.py build
+```
+
+# Running
+All the setup needs to be done as described in the [test apps README](../../README.md).

+ 18 - 0
tools/test_apps/system/longjmp_test/app_test.py

@@ -0,0 +1,18 @@
+#!/usr/bin/env python
+
+import ttfw_idf
+from tiny_test_fw import Utility
+
+
+@ttfw_idf.idf_custom_test(env_tag='Example_GENERIC', target=['esp32', 'esp32s2'], group='test-apps')
+def test_longjmp(env, _):
+
+    dut = env.get_dut('longjmp_test', 'tools/test_apps/system/longjmp_test')
+    dut.start_app()
+    dut.expect('Test successful', 15)
+
+    Utility.console_log('longjmp test done.')
+
+
+if __name__ == '__main__':
+    test_longjmp()

+ 2 - 0
tools/test_apps/system/longjmp_test/main/CMakeLists.txt

@@ -0,0 +1,2 @@
+idf_component_register(SRCS "hello_world_main.c"
+                    INCLUDE_DIRS "")

+ 5 - 0
tools/test_apps/system/longjmp_test/main/component.mk

@@ -0,0 +1,5 @@
+#
+# "main" pseudo-component makefile.
+#
+# (Uses default behaviour of compiling all source files in directory, adding 'include' to include path.)
+

+ 117 - 0
tools/test_apps/system/longjmp_test/main/hello_world_main.c

@@ -0,0 +1,117 @@
+/* test longjmp
+
+   This example code is in the Public Domain (or CC0 licensed, at your option.)
+
+   Unless required by applicable law or agreed to in writing, this
+   software is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
+   CONDITIONS OF ANY KIND, either express or implied.
+*/
+#include <stdio.h>
+#include "sdkconfig.h"
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "esp_system.h"
+#include "esp_spi_flash.h"
+#include <esp_task.h>
+
+#include <setjmp.h>
+
+#define LUAI_NOIPA __attribute__((__noipa__))
+#define LUAI_THROW(c)		longjmp((c)->b, 1)
+#define LUAI_TRY(c,a)		if (setjmp((c)->b) == 0) { a }
+
+#define TIMEOUT 50
+
+#define RECURSION 19
+
+static esp_timer_handle_t crash_timer;
+
+static uint32_t result = 0;
+
+uint32_t calc_fac(uint32_t n) {
+    if (n == 1 || n == 0) {
+        return 1;
+    } else {
+        return n * calc_fac(n - 1);
+    }
+}
+
+static void timer_cb(void *arg) {
+    result = calc_fac(RECURSION);
+}
+
+typedef struct {
+    jmp_buf b;
+} jmp_ctx;
+
+LUAI_NOIPA
+static void pret(jmp_ctx *jc) {
+    LUAI_THROW(jc);
+}
+
+LUAI_NOIPA
+static void precurse(jmp_ctx *jc, int n) {
+    if (n) precurse(jc, n - 1);
+    else pret(jc);
+}
+
+LUAI_NOIPA
+static void ptest(jmp_ctx *jc) {
+    precurse(jc, 64);
+}
+
+LUAI_NOIPA
+void pcall(void (*func)(jmp_ctx *ctx)) {
+    jmp_ctx jc;
+    LUAI_TRY(&jc,
+        ptest(&jc);
+    );
+}
+
+static void sjlj_task(void *ctx) {
+    uint32_t start = xTaskGetTickCount();
+    for (;;) {
+        pcall(ptest);
+        uint32_t end = xTaskGetTickCount();
+
+        uint32_t dt = end - start;
+        if (dt >= 1000) {
+            start = end;
+
+            printf("[%u] sjlj tick %d\n", end, (int)ctx);
+        }
+
+        if (end > 9800) {
+            break;
+        }
+    }
+
+    vTaskDelete(NULL);
+}
+
+void app_main(void)
+{
+    const esp_timer_create_args_t timer_args = {
+        timer_cb,
+        NULL,
+        ESP_TIMER_TASK,
+        "crash_timer",
+    };
+
+    esp_timer_create(&timer_args, &crash_timer);
+    esp_timer_start_periodic(crash_timer, TIMEOUT);
+
+    printf("Hello world!\n");
+    printf("Free heap: %d\n", esp_get_free_heap_size());
+
+    for (size_t i = 0; i < 16; i++) {
+        xTaskCreate(sjlj_task, "sjlj_task", 4096, (void *) i, tskIDLE_PRIORITY + 0, NULL);
+    }
+
+    vTaskDelay(10000);
+    printf("stopping timers...\n");
+    esp_timer_stop(crash_timer);
+    esp_timer_delete(crash_timer);
+
+    printf("Test successful\n");
+}

+ 11 - 0
tools/test_apps/system/longjmp_test/sdkconfig.defaults

@@ -0,0 +1,11 @@
+CONFIG_COMPILER_OPTIMIZATION_PERF=y
+
+CONFIG_ESP_INT_WDT=n
+CONFIG_ESP_TASK_WDT=n
+
+CONFIG_ESP_SYSTEM_PANIC_PRINT_REBOOT=y
+
+CONFIG_ESP_TIMER_TASK_STACK_SIZE=3584
+
+CONFIG_FREERTOS_UNICORE=y
+CONFIG_FREERTOS_HZ=1000