Merge branch 'fix/freertos-clang-riscv-port' into 'master'

freertos: riscv: implement vPortTaskWrapper with asm only Closes IDF-6347 See merge request espressif/esp-idf!22199
espressif · Feb 21, 2023 · 25cb9b3 · 25cb9b3
2 parents b26800e + d1214ae
commit 25cb9b3
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 44 deletions.
diff --git a/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c b/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c
@@ -469,23 +469,15 @@ FORCE_INLINE_ATTR UBaseType_t uxInitialiseStackTLS(UBaseType_t uxStackPointer, u
 }
 
 #if CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER
-/**
- * Wrapper to allow task functions to return. Force the optimization option -O1 on that function to make sure there
- * is no tail-call. Indeed, we need the compiler to keep the return address to this function when calling `panic_abort`.
- *
- * Thanks to `naked` attribute, the compiler won't generate a prologue and epilogue for the function, which saves time
- * and stack space.
- */
-static void __attribute__((optimize("O1"), naked)) vPortTaskWrapper(TaskFunction_t pxCode, void *pvParameters)
+static void vPortTaskWrapper(TaskFunction_t pxCode, void *pvParameters)
 {
-    asm volatile(".cfi_undefined ra\n");
+    __asm__ volatile(".cfi_undefined ra");  // tell to debugger that it's outermost (inital) frame
     extern void __attribute__((noreturn)) panic_abort(const char *details);
     static char DRAM_ATTR msg[80] = "FreeRTOS: FreeRTOS Task \"\0";
     pxCode(pvParameters);
-    //FreeRTOS tasks should not return. Log the task name and abort.
-    char *pcTaskName = pcTaskGetName(NULL);
+    /* FreeRTOS tasks should not return. Log the task name and abort. */
     /* We cannot use s(n)printf because it is in flash */
-    strcat(msg, pcTaskName);
+    strcat(msg, pcTaskGetName(NULL));
     strcat(msg, "\" should not return, Aborting now!");
     panic_abort(msg);
 }

diff --git a/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c b/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c
@@ -194,37 +194,17 @@ FORCE_INLINE_ATTR UBaseType_t uxInitialiseStackTLS(UBaseType_t uxStackPointer, u
 }
 
 #if CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER
-/**
- * Wrapper to allow task functions to return. Force the optimization option -O1 on that function to make sure there
- * is no tail-call. Indeed, we need the compiler to keep the return address to this function when calling `panic_abort`.
- *
- * Thanks to `naked` attribute, the compiler won't generate a prologue and epilogue for the function, which saves time
- * and stack space.
- */
-static void __attribute__((optimize("O1"), naked)) vPortTaskWrapper(TaskFunction_t pxCode, void *pvParameters)
+static void vPortTaskWrapper(TaskFunction_t pxCode, void *pvParameters)
 {
-#ifdef __clang__
-    // clang generates error "error: non-ASM statement in naked function is not supported"
-    // The reason for it is described at
-    // https://stackoverflow.com/questions/47316611/clang-error-non-asm-statement-in-naked-function-is-not-supported.
-    // GCC docs say that there is no guarantee that non-ASM statement in naked function will work:
-    // "Only basic asm statements can safely be included in naked functions (see Basic Asm).
-    //  While using extended asm or a mixture of basic asm and C code may appear to work, they cannot be
-    //  depended upon to work reliably and are not supported."
-    // TODO: IDF-6347
-    #error CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER not supported yet when building with Clang!
-#else
-    asm volatile(".cfi_undefined ra\n");
+    __asm__ volatile(".cfi_undefined ra");  // tell to debugger that it's outermost (inital) frame
     extern void __attribute__((noreturn)) panic_abort(const char *details);
     static char DRAM_ATTR msg[80] = "FreeRTOS: FreeRTOS Task \"\0";
     pxCode(pvParameters);
-    //FreeRTOS tasks should not return. Log the task name and abort.
-    char *pcTaskName = pcTaskGetName(NULL);
+    /* FreeRTOS tasks should not return. Log the task name and abort. */
     /* We cannot use s(n)printf because it is in flash */
-    strcat(msg, pcTaskName);
+    strcat(msg, pcTaskGetName(NULL));
     strcat(msg, "\" should not return, Aborting now!");
     panic_abort(msg);
-#endif
 }
 #endif // CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER
 

diff --git a/components/freertos/linker.lf b/components/freertos/linker.lf
@@ -232,8 +232,6 @@ entries:
         if IDF_TARGET_ARCH_XTENSA = y:
             port:xPortStartScheduler (default)
             port:vPortEndScheduler (default)
-            if FREERTOS_TASK_FUNCTION_WRAPPER = y:
-                port:vPortTaskWrapper (default)
             port:pxPortInitialiseStack (default)
             if IDF_TARGET_ESP32 = y || IDF_TARGET_ESP32S3 = y :
                 port:vPortCleanUpCoprocArea (default)
@@ -245,7 +243,5 @@ entries:
         if IDF_TARGET_ARCH_RISCV = y:
             port:xPortStartScheduler (default)
             port:vPortEndScheduler (default)
-            if FREERTOS_TASK_FUNCTION_WRAPPER = y:
-                port:vPortTaskWrapper (default)
             port:pxPortInitialiseStack (default)
             port:xPortGetTickRateHz (default)
diff --git a/components/freertos/linker_smp.lf b/components/freertos/linker_smp.lf
@@ -215,8 +215,6 @@ entries:
             port:vPortFree (default)
             port:vPortInitialiseBlocks (default)
             port:xPortGetFreeHeapSize (default)
-            if FREERTOS_TASK_FUNCTION_WRAPPER = y:
-                port:vPortTaskWrapper (default)
             port:pxPortInitialiseStack (default)
             if FREERTOS_UNICORE = n:
                 port:vPortCleanUpCoprocArea (default)
@@ -235,8 +233,6 @@ entries:
             port:vPortFree (default)
             port:vPortInitialiseBlocks (default)
             port:xPortGetFreeHeapSize (default)
-            if FREERTOS_TASK_FUNCTION_WRAPPER = y:
-                port:vPortTaskWrapper (default)
             port:pxPortInitialiseStack (default)
             if FREERTOS_TLSP_DELETION_CALLBACKS = y:
                 port:vPortTLSPointersDelCb (default)