From 85f734d70ac821607bfee75ebb29c4d99acf4079 Mon Sep 17 00:00:00 2001
From: Joseph Hickey <joseph.p.hickey@nasa.gov>
Date: Mon, 29 Jun 2020 13:42:00 -0400
Subject: [PATCH] Fix #525, ensure POSIX stack size meets requirements

The pthread_attr_setstacksize() function stipulates that it
may fail if the user-supplied stack size is not at least
PTHREAD_STACK_MIN and also possibly a multiple of page size.

This partially reverts previous PR #508 and adds back rounding
up to PTHREAD_STACK_MIN and also adds rounding up to a system
page size.  However the check for zero stack still remains
at the shared level so attempts to create a task with zero
stack will still fail.  This allows internal helper threads
to be created with a default minimum stack size, however.
---
 src/os/posix/inc/os-posix.h      |  1 +
 src/os/posix/src/os-impl-tasks.c | 78 +++++++++++++++++++++++++-------
 2 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/src/os/posix/inc/os-posix.h b/src/os/posix/inc/os-posix.h
index 48a43c202..6b549a565 100644
--- a/src/os/posix/inc/os-posix.h
+++ b/src/os/posix/inc/os-posix.h
@@ -80,6 +80,7 @@ typedef struct
    pthread_key_t ThreadKey;
    sigset_t      MaximumSigMask;
    sigset_t      NormalSigMask;
+   size_t        PageMask;
    POSIX_PriorityLimits_t PriLimits;
    int           SelectedRtScheduler;
 } POSIX_GlobalVars_t;
diff --git a/src/os/posix/src/os-impl-tasks.c b/src/os/posix/src/os-impl-tasks.c
index a318861a0..0d6f4fbb1 100644
--- a/src/os/posix/src/os-impl-tasks.c
+++ b/src/os/posix/src/os-impl-tasks.c
@@ -38,6 +38,13 @@
 #include "os-shared-task.h"
 #include "os-shared-idmap.h"
 
+/*
+ * Defines
+ */
+#ifndef PTHREAD_STACK_MIN
+#define PTHREAD_STACK_MIN   (8*1024)
+#endif
+
 /* Tables where the OS object information is stored */
 OS_impl_task_internal_record_t      OS_impl_task_table          [OS_MAX_TASKS];
 
@@ -212,6 +219,7 @@ int32 OS_Posix_TaskAPI_Impl_Init(void)
    bool                   sched_fifo_valid;
    POSIX_PriorityLimits_t sched_rr_limits;
    bool                   sched_rr_valid;
+   size_t                 pagesz;
 
    /* Initialize Local Tables */
    memset(OS_impl_task_table, 0, sizeof(OS_impl_task_table));
@@ -416,6 +424,24 @@ int32 OS_Posix_TaskAPI_Impl_Init(void)
    }
 #endif
 
+   pagesz = sysconf(_SC_PAGESIZE);
+   if (pagesz > 0)
+   {
+       /*
+        * convert to a mask.  Should already be a power of 2,
+        * but if not, make it so.
+        */
+       --pagesz;
+       pagesz |= (pagesz >> 1);
+       pagesz |= (pagesz >> 2);
+       pagesz |= (pagesz >> 4);
+       pagesz |= (pagesz >> 8);
+       pagesz |= (pagesz >> 16);
+
+       POSIX_GlobalVars.PageMask = pagesz;
+   }
+
+
    return OS_SUCCESS;
 } /* end OS_Posix_TaskAPI_Impl_Init */
 
@@ -446,14 +472,41 @@ int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, uint32 priority, size_t
        return(OS_ERROR);
     }
 
-
-     /*
-     ** Test to see if the original main task scheduling priority worked.
-     ** If so, then also set the attributes for this task.  Otherwise attributes
-     ** are left at default.
+    /*
+     * Adjust the stack size parameter.
+     *
+     * POSIX has additional restrictions/limitations on the stack size of tasks that
+     * other RTOS environments may not have.  Specifically POSIX says that the stack
+     * size must be at least PTHREAD_STACK_MIN and may also need to be a multiple of the
+     * system page size.
+     *
+     * Rounding up means the user might get a bigger stack than they requested, but
+     * that should not break anything aside from consuming extra memory.
      */
-     if (POSIX_GlobalVars.EnableTaskPriorities)
-     {
+    if (stacksz < PTHREAD_STACK_MIN)
+    {
+        stacksz = PTHREAD_STACK_MIN;
+    }
+
+    stacksz = (stacksz + POSIX_GlobalVars.PageMask) & ~POSIX_GlobalVars.PageMask;
+
+    /*
+    ** 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));
+        return(OS_ERROR);
+    }
+
+    /*
+    ** Test to see if the original main task scheduling priority worked.
+    ** If so, then also set the attributes for this task.  Otherwise attributes
+    ** are left at default.
+    */
+    if (POSIX_GlobalVars.EnableTaskPriorities)
+    {
         /*
         ** Set the scheduling inherit attribute to EXPLICIT
         */
@@ -464,15 +517,6 @@ int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, uint32 priority, size_t
             return(OS_ERROR);
         }
 
-       /*
-       ** 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));
-           return(OS_ERROR);
-       }
 
        /*
        ** Set the scheduling policy
@@ -503,7 +547,7 @@ int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, uint32 priority, size_t
           return(OS_ERROR);
        }
 
-     } /* End if user is root */
+    } /* End if user is root */
 
     /*
      ** Create thread