Skip to content

Commit

Permalink
Fix #1449, POSIX implementation honors stack pointer
Browse files Browse the repository at this point in the history
When the OS_TaskCreate() function is called with a non-NULL stack pointer,
the POSIX implementation now should use this pointer as intended.  Previously
it was always allocating a new stack buffer, ignoring the passed-in pointer.

NOTE: When using this feature, the caller must use caution to make sure the
buffer being passed in is large enough to use as a task stack.  Particularly
when running under a desktop Linux/Glibc environment, the infrastructure
puts a considerably large object on the stack.  For example, in the
osal-core-tests, the stack size was too small.
  • Loading branch information
jphickey committed Feb 14, 2024
1 parent 403ebba commit 74503ec
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/os/posix/inc/os-impl-tasks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
5 changes: 3 additions & 2 deletions src/os/posix/src/os-impl-console.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
38 changes: 24 additions & 14 deletions src/os/posix/src/os-impl-tasks.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Check notice

Code scanning / CodeQL-coding-standard

Function too long Note

OS_Posix_InternalTaskCreate_Impl has too many lines (129, while 60 are allowed).

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
size_t stacksz, PthreadFuncPtr_t entry, void *entry_arg)
{
int return_code = 0;
pthread_attr_t custom_attr;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/os/posix/src/os-impl-timebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
34 changes: 34 additions & 0 deletions src/tests/osal-core-test/osal-core-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
}

/* ************************************************************************************ */
Expand Down
8 changes: 4 additions & 4 deletions src/tests/osal-core-test/osal-core-test.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit 74503ec

Please sign in to comment.