Merge pull request #1450 from jphickey:fix-1449-stackptr

Fix #1449, POSIX implementation honors stack pointer
nasa · Feb 22, 2024 · dc106bf · dc106bf
2 parents 04149d8 + e0d3d5a
commit dc106bf
Show file tree

Hide file tree

Showing 11 changed files with 78 additions and 33 deletions.
diff --git a/src/examples/tasking-example/tasking-example.c b/src/examples/tasking-example/tasking-example.c
@@ -30,7 +30,7 @@
 /* Task 1 */
 
 #define TASK_1_ID         1
-#define TASK_1_STACK_SIZE 1024
+#define TASK_1_STACK_SIZE 4096
 #define TASK_1_PRIORITY   101
 
 uint32 task_1_stack[TASK_1_STACK_SIZE];
@@ -40,7 +40,7 @@ void task_1(void);
 /* Task 2 */
 
 #define TASK_2_ID         2
-#define TASK_2_STACK_SIZE 1024
+#define TASK_2_STACK_SIZE 4096
 #define TASK_2_PRIORITY   102
 
 uint32 task_2_stack[TASK_2_STACK_SIZE];
@@ -50,7 +50,7 @@ void task_2(void);
 /* Task 3 */
 
 #define TASK_3_ID         3
-#define TASK_3_STACK_SIZE 1024
+#define TASK_3_STACK_SIZE 4096
 #define TASK_3_PRIORITY   103
 
 uint32 task_3_stack[TASK_3_STACK_SIZE];

diff --git a/src/os/posix/inc/os-impl-tasks.h b/src/os/posix/inc/os-impl-tasks.h
@@ -40,7 +40,7 @@ typedef struct
 /* Tables where the OS object information is stored */
 extern OS_impl_task_internal_record_t OS_impl_task_table[OS_MAX_TASKS];
 
-int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, osal_priority_t priority, size_t stacksz,
-                                       PthreadFuncPtr_t entry, void *entry_arg);
+int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, osal_priority_t priority, osal_stackptr_t stackptr,
+                                       size_t stacksz, PthreadFuncPtr_t entry, void *entry_arg);
 
 #endif /* OS_IMPL_TASKS_H */
diff --git a/src/os/posix/src/os-impl-console.c b/src/os/posix/src/os-impl-console.c
@@ -128,8 +128,9 @@ int32 OS_ConsoleCreate_Impl(const OS_object_token_t *token)
             {
                 /* cppcheck-suppress unreadVariable // intentional use of other union member */
                 local_arg.id = OS_ObjectIdFromToken(token);
-                return_code  = OS_Posix_InternalTaskCreate_Impl(&consoletask, OS_CONSOLE_TASK_PRIORITY, 0,
-                                                               OS_ConsoleTask_Entry, local_arg.opaque_arg);
+                return_code =
+                    OS_Posix_InternalTaskCreate_Impl(&consoletask, OS_CONSOLE_TASK_PRIORITY, OSAL_TASK_STACK_ALLOCATE,
+                                                     PTHREAD_STACK_MIN, OS_ConsoleTask_Entry, local_arg.opaque_arg);
 
                 if (return_code != OS_SUCCESS)
                 {

diff --git a/src/os/posix/src/os-impl-tasks.c b/src/os/posix/src/os-impl-tasks.c
@@ -447,8 +447,8 @@ int32 OS_Posix_TaskAPI_Impl_Init(void)
  *  Purpose: Local helper routine, not part of OSAL API.
  *
  *-----------------------------------------------------------------*/
-int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, osal_priority_t priority, size_t stacksz,
-                                       PthreadFuncPtr_t entry, void *entry_arg)
+int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, osal_priority_t priority, osal_stackptr_t stackptr,
+                                       size_t stacksz, PthreadFuncPtr_t entry, void *entry_arg)
 {
     int                return_code = 0;
     pthread_attr_t     custom_attr;
@@ -467,20 +467,30 @@ int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, osal_priority_t priority
     }
 
     /*
-     * Adjust the stack size parameter, add budget for TCB/TLS overhead.
-     */
-    stacksz += OS_IMPL_STACK_EXTRA;
+    ** Set the Stack Pointer and/or Size
+    */
+    if (stackptr != OSAL_TASK_STACK_ALLOCATE)
+    {
+        return_code = pthread_attr_setstack(&custom_attr, stackptr, stacksz);
+    }
+    else
+    {
+        /*
+         * Adjust the stack size parameter, add budget for TCB/TLS overhead.
+         * Note that this budget can only be added when allocating the stack here,
+         * if the caller passed in a stack, they take responsibility for adding this.
+         */
+        stacksz += OS_IMPL_STACK_EXTRA;
 
-    stacksz += POSIX_GlobalVars.PageSize - 1;
-    stacksz -= stacksz % POSIX_GlobalVars.PageSize;
+        stacksz += POSIX_GlobalVars.PageSize - 1;
+        stacksz -= stacksz % POSIX_GlobalVars.PageSize;
+
+        return_code = pthread_attr_setstacksize(&custom_attr, stacksz);
+    }
 
-    /*
-    ** Set the Stack Size
-    */
-    return_code = pthread_attr_setstacksize(&custom_attr, stacksz);
     if (return_code != 0)
     {
-        OS_DEBUG("pthread_attr_setstacksize error in OS_TaskCreate: %s\n", strerror(return_code));
+        OS_DEBUG("Error configuring stack in OS_TaskCreate: %s\n", strerror(return_code));
         return OS_ERROR;
     }
 
@@ -588,8 +598,8 @@ int32 OS_TaskCreate_Impl(const OS_object_token_t *token, uint32 flags)
     task = OS_OBJECT_TABLE_GET(OS_task_table, *token);
     impl = OS_OBJECT_TABLE_GET(OS_impl_task_table, *token);
 
-    return_code = OS_Posix_InternalTaskCreate_Impl(&impl->id, task->priority, task->stack_size, OS_PthreadTaskEntry,
-                                                   arg.opaque_arg);
+    return_code = OS_Posix_InternalTaskCreate_Impl(&impl->id, task->priority, task->stack_pointer, task->stack_size,
+                                                   OS_PthreadTaskEntry, arg.opaque_arg);
 
     return return_code;
 }

diff --git a/src/os/posix/src/os-impl-timebase.c b/src/os/posix/src/os-impl-timebase.c
@@ -347,8 +347,8 @@ int32 OS_TimeBaseCreate_Impl(const OS_object_token_t *token)
 
     /* cppcheck-suppress unreadVariable // intentional use of other union member */
     arg.id      = OS_ObjectIdFromToken(token);
-    return_code = OS_Posix_InternalTaskCreate_Impl(&local->handler_thread, OSAL_PRIORITY_C(0), 0,
-                                                   OS_TimeBasePthreadEntry, arg.opaque_arg);
+    return_code = OS_Posix_InternalTaskCreate_Impl(&local->handler_thread, OSAL_PRIORITY_C(0), OSAL_TASK_STACK_ALLOCATE,
+                                                   PTHREAD_STACK_MIN, OS_TimeBasePthreadEntry, arg.opaque_arg);
     if (return_code != OS_SUCCESS)
     {
         return return_code;

diff --git a/src/tests/bin-sem-test/bin-sem-test.c b/src/tests/bin-sem-test/bin-sem-test.c
@@ -23,7 +23,7 @@
 #include "utassert.h"
 #include "uttest.h"
 
-#define TASK_STACK_SIZE 1024
+#define TASK_STACK_SIZE 16384
 
 uint32 task_counter[3];
 

diff --git a/src/tests/bin-sem-timeout-test/bin-sem-timeout-test.c b/src/tests/bin-sem-timeout-test/bin-sem-timeout-test.c
@@ -32,9 +32,9 @@ void BinSemTimeoutSetup(void);
 void BinSemTimeoutCheck(void);
 
 /* Task 1 */
-#define TASK_1_STACK_SIZE 1024
+#define TASK_1_STACK_SIZE 4096
 #define TASK_1_PRIORITY   101
-#define TASK_2_STACK_SIZE 1024
+#define TASK_2_STACK_SIZE 4096
 #define TASK_2_PRIORITY   50
 
 uint32    task_1_stack[TASK_1_STACK_SIZE];

diff --git a/src/tests/count-sem-test/count-sem-test.c b/src/tests/count-sem-test/count-sem-test.c
@@ -23,7 +23,7 @@
 #include "utassert.h"
 #include "uttest.h"
 
-#define TASK_STACK_SIZE 1024
+#define TASK_STACK_SIZE 4096
 
 uint32 task_counter[3];
 

diff --git a/src/tests/osal-core-test/osal-core-test.c b/src/tests/osal-core-test/osal-core-test.c
@@ -119,11 +119,27 @@ void task_generic_no_exit(void)
 
 void task_generic_with_exit(void) {}
 
+void task_test_stackptr_0(void)
+{
+    int32   LocalVar;
+    cpuaddr VarAddress;
+    cpuaddr StackAddress;
+
+    OS_TaskDelay(10);
+
+    VarAddress   = (cpuaddr)&LocalVar;
+    StackAddress = (cpuaddr)OSAL_STACKPTR_C(task_0_stack);
+
+    UtAssert_GT(cpuaddr, VarAddress, StackAddress);
+    UtAssert_LT(cpuaddr, VarAddress, StackAddress + sizeof(task_0_stack));
+}
+
 typedef struct
 {
     osal_id_t task_id;
     uint32    task_stack[TASK_0_STACK_SIZE];
 } TestTaskData;
+
 /* ********************************************** TASKS******************************* */
 void TestTasks(void)
 {
@@ -252,6 +268,24 @@ void TestTasks(void)
     UtAssert_True(OS_TaskDelete(task_1_id) != OS_SUCCESS, "OS_TaskDelete, Task 1");
     UtAssert_True(OS_TaskDelete(task_2_id) == OS_SUCCESS, "OS_TaskDelete, Task 2");
     UtAssert_True(OS_TaskDelete(task_3_id) == OS_SUCCESS, "OS_TaskDelete, Task 3");
+
+    /*
+     * Validate that the user-specified stack pointer parameter is implemented correctly.
+     * Addresses of local variables within the task should be within the given stack range
+     */
+    UtAssert_INT32_EQ(OS_TaskCreate(&task_0_id, "Task 0", task_test_stackptr_0, OSAL_STACKPTR_C(task_0_stack),
+                                    sizeof(task_0_stack), OSAL_PRIORITY_C(TASK_0_PRIORITY), 0),
+                      OS_SUCCESS);
+
+    /* Looping delay in parent task to wait for child task to exit */
+    loopcnt = 0;
+    while ((OS_TaskGetInfo(task_0_id, &taskprop) == OS_SUCCESS) && (loopcnt < UT_EXIT_LOOP_MAX))
+    {
+        OS_TaskDelay(25);
+        loopcnt++;
+    }
+
+    UtAssert_INT32_LT(loopcnt, UT_EXIT_LOOP_MAX);
 }
 
 /* ************************************************************************************ */

diff --git a/src/tests/osal-core-test/osal-core-test.h b/src/tests/osal-core-test/osal-core-test.h
@@ -29,19 +29,19 @@
 #define OSAL_CORE_TEST_H
 
 /* Task 0 */
-#define TASK_0_STACK_SIZE 1024
+#define TASK_0_STACK_SIZE 4096
 #define TASK_0_PRIORITY   230
 
 /* Task 1 */
-#define TASK_1_STACK_SIZE 1024
+#define TASK_1_STACK_SIZE 4096
 #define TASK_1_PRIORITY   231
 
 /* Task 2 */
-#define TASK_2_STACK_SIZE 1024
+#define TASK_2_STACK_SIZE 4096
 #define TASK_2_PRIORITY   232
 
 /* Task 3 */
-#define TASK_3_STACK_SIZE 1024
+#define TASK_3_STACK_SIZE 4096
 #define TASK_3_PRIORITY   233
 
 /* Global Data */

diff --git a/src/tests/queue-test/queue-test.c b/src/tests/queue-test/queue-test.c
@@ -36,9 +36,9 @@ void QueueTimeoutCheck(void);
 #define MSGQ_BURST 3
 
 /* Task 1 */
-#define TASK_1_STACK_SIZE 1024
+#define TASK_1_STACK_SIZE 4096
 #define TASK_1_PRIORITY   101
-#define TASK_2_STACK_SIZE 1024
+#define TASK_2_STACK_SIZE 4096
 #define TASK_2_PRIORITY   50
 
 uint32    task_1_stack[TASK_1_STACK_SIZE];