StackSize and ConstrainedDevice incompatible; Additional unit tests f…

…or the thread create API; Add new CI job and script to check CMake flag compatibility

.github/workflows/ci.yml

@@ -215,6 +215,17 @@ jobs:
           cmake .. ${{ matrix.config.cmake_flags }}
           make -j$(nproc)
 
+  cmake-flags-test:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Run CMake Flag Combination Test
+        run: |
+          ./tst/scripts/test_cmake_flags.sh >> $GITHUB_STEP_SUMMARY
+
   windows-msvc:
     runs-on: windows-2022
     env:

CMakeLists.txt

@@ -58,9 +58,14 @@ function(enableSanitizer SANITIZER)
     set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${SANITIZER}" PARENT_SCOPE)
 endfunction()
 
-if(KVS_DEFAULT_STACK_SIZE)
-  message(STATUS "Building with default stack size")
-  add_compile_definitions(KVS_DEFAULT_STACK_SIZE_BYTES=${KVS_DEFAULT_STACK_SIZE})
+if(KVS_DEFAULT_STACK_SIZE AND CONSTRAINED_DEVICE)
+  message(FATAL_ERROR "Conflicting parameters: KVS_DEFAULT_STACK_SIZE and CONSTRAINED_DEVICE cannot both be set.")
+elseif(KVS_DEFAULT_STACK_SIZE)
+  message(STATUS "Building with default stack size: ${KVS_DEFAULT_STACK_SIZE} bytes")
+  add_definitions(-DKVS_DEFAULT_STACK_SIZE_BYTES=${KVS_DEFAULT_STACK_SIZE})
+elseif(CONSTRAINED_DEVICE)
+  message(STATUS "Building for constrained device with stack size set to 512 KiB (0.5 MiB)")
+  add_definitions(-DCONSTRAINED_DEVICE)
 endif()
 
 if(ADDRESS_SANITIZER)

README.md

@@ -53,9 +53,10 @@ You can pass the following options to `cmake ..`
 * `-DMEMORY_SANITIZER` --  Build with MemorySanitizer
 * `-DTHREAD_SANITIZER` -- Build with ThreadSanitizer
 * `-DUNDEFINED_BEHAVIOR_SANITIZER` Build with UndefinedBehaviorSanitizer
-* `-DBUILD_DEBUG_HEAP` Build debug heap with guard bands and validation. This is ONLY intended for low-level debugging purposes. Default is OFF
+* `-DBUILD_DEBUG_HEAP` Build debug heap with guard bands and validation. This is ONLY intended for low-level debugging purposes. Default is OFF.
 * `-DALIGNED_MEMORY_MODEL` Build for aligned memory model only devices. Default is OFF.
-* `-DKVS_DEFAULT_STACK_SIZE` -- Value in bytes for default stack size of threads. Pthread minimum for Unix applications is 16 Kib.
+* `-DCONSTRAINED_DEVICE` -- Sets the default stack size of threads to 512 KiB (0.5 MiB). Incompatible with `-DKVS_DEFAULT_STACK_SIZE`. Not available in Windows. Default is OFF.
+* `-DKVS_DEFAULT_STACK_SIZE` -- Value in bytes for default stack size of threads. Incompatible with `-DCONSTRAINED_DEVICE`. Default is system default.
 
 ### Build
 To build the library run make in the build directory you executed CMake.

tst/scripts/test_cmake_flags.sh

@@ -0,0 +1,65 @@
+#!/bin/bash
+
+# Define flag combinations as tuples with expected results
+# Format: "flags expected_result"
+# - expected_result is either OK or FAIL
+COMBINATIONS=(
+  "-DKVS_DEFAULT_STACK_SIZE=65536 -DCONSTRAINED_DEVICE=ON FAIL"
+  "-DKVS_DEFAULT_STACK_SIZE=65536 OK"
+  "-DCONSTRAINED_DEVICE=ON OK"
+  "-DKVS_DEFAULT_STACK_SIZE=65536 -DCONSTRAINED_DEVICE=OFF OK"
+  "-DKVS_DEFAULT_STACK_SIZE=0 -DCONSTRAINED_DEVICE=ON OK"
+)
+
+# Initialize result counters
+TOTAL_TESTS=${#COMBINATIONS[@]}
+PASSED=0
+FAILED=0
+
+# Markdown summary header
+SUMMARY="# CMake Flag Test Summary\n"
+
+# Test each combination
+for combination in "${COMBINATIONS[@]}"; do
+  # Extract flags and expected result from the tuple
+  FLAGS="${combination% *}"      # Everything except the last word
+  EXPECTED_RESULT="${combination##* }"  # The last word
+
+  # Clean up the build directory before each test
+  rm -rf build
+  mkdir build
+
+  # Run only the CMake configuration with the current combination
+  cmake -B build $FLAGS > /dev/null 2>&1
+  EXIT_CODE=$?
+
+  # Check if the exit code matches the expected result
+  if [[ "$EXPECTED_RESULT" == "FAIL" ]]; then
+    if [[ $EXIT_CODE -ne 0 ]]; then
+      PASSED=$((PASSED + 1))
+      SUMMARY+="* ✅ \`$FLAGS\`: Correctly failed.\n"
+    else
+      FAILED=$((FAILED + 1))
+      SUMMARY+="* ❌ \`$FLAGS\`: Expected failure but succeeded.\n"
+    fi
+  else
+    if [[ $EXIT_CODE -ne 0 ]]; then
+      FAILED=$((FAILED + 1))
+      SUMMARY+="* ❌ \`$FLAGS\`: Unexpected failure.\n"
+    else
+      PASSED=$((PASSED + 1))
+      SUMMARY+="* ✅ \`$FLAGS\`: Success as expected.\n"
+    fi
+  fi
+done
+
+# Generate a Markdown summary
+SUMMARY+="\n### Total Tests: $TOTAL_TESTS\n"
+SUMMARY+="### Passed: $PASSED\n"
+SUMMARY+="### Failed: $FAILED\n"
+
+echo -e "$SUMMARY"
+
+if [[ $FAILED -gt 0 ]]; then
+  exit 1
+fi

tst/utils/Thread.cpp

@@ -15,6 +15,10 @@ struct sleep_times {
 
 PVOID testThreadRoutine(PVOID arg)
 {
+    if (arg == NULL) {
+        FAIL() << "TestThreadRoutine requires args!";
+    }
+
     MUTEX_LOCK(gThreadMutex);
     gThreadCount++;
     struct sleep_times* st = (struct sleep_times*) arg;
@@ -37,6 +41,11 @@ PVOID testThreadRoutine(PVOID arg)
     return NULL;
 }
 
+PVOID emptyRoutine(PVOID arg)
+{
+    return NULL;
+}
+
 TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheck)
 {
     UINT64 index;
@@ -130,7 +139,7 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheckWithStack)
         st[index].threadVisited = FALSE;
         st[index].threadCleared = FALSE;
         st[index].threadSleepTime = index * HUNDREDS_OF_NANOS_IN_A_MILLISECOND;
-        EXPECT_EQ(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(&threads[index], testThreadRoutine, threadStack, (PVOID)&st[index]));
+        EXPECT_EQ(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(&threads[index], testThreadRoutine, threadStack, (PVOID) &st[index]));
     }
 
     // Await for the threads to finish
@@ -150,21 +159,85 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheckWithStack)
     MUTEX_FREE(gThreadMutex);
 }
 
+TEST_F(ThreadFunctionalityTest, ThreadCreateUseDefaultsTest)
+{
+    TID threadId = 0;
+
+    // 0 passed into the size parameter means to use the defaults.
+    EXPECT_EQ(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(&threadId, emptyRoutine, 0, NULL));
+    EXPECT_NE(0, threadId);
+    EXPECT_EQ(STATUS_SUCESSS, THREAD_JOIN(threadId, NULL))
+}
+
 TEST_F(ThreadFunctionalityTest, NegativeTest)
 {
-    UINT64 index;
-    TID threads[TEST_THREAD_COUNT];
-    gThreadMutex = MUTEX_CREATE(FALSE);
-    SIZE_T threadStack = 16 * 1024;
-    struct sleep_times st[TEST_THREAD_COUNT];
+    TID threadId = 0;
+    SIZE_T threadStack = 512 * 1024; // 0.5 MiB
 
-    gThreadCount = 0;
-    EXPECT_NE(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(NULL, testThreadRoutine, threadStack, NULL));
+    // No out value case
+    EXPECT_NE(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(NULL, emptyRoutine, threadStack, NULL));
+
+    // Request too large stack size case
+    EXPECT_NE(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(&threadId, emptyRoutine, SIZE_MAX, NULL));
+    EXPECT_EQ(0, threadId);
+
+    // No out value case
     EXPECT_NE(STATUS_SUCCESS, THREAD_CREATE(NULL, testThreadRoutine, NULL));
+}
 
-    MUTEX_LOCK(gThreadMutex);
-    EXPECT_EQ(0, gThreadCount);
-    MUTEX_UNLOCK(gThreadMutex);
+// Linux-only test
+#if !defined _WIN32 && !defined _WIN64 && !defined __CYGWIN__ && !defined __APPLE__ && !defined __MACH__
 
-    MUTEX_FREE(gThreadMutex);
+// Struct to hold the stack size information for passing back to main
+typedef struct {
+    SIZE_T stackSize;
+    INT32 failure;
+} TestThreadInfo;
+
+// Function to retrieve and print the stack size from within the thread
+PVOID fetchStackSizeThreadRoutine(PVOID arg)
+{
+    pthread_attr_t attr;
+    SIZE_T stackSize;
+    INT32 result = 0;
+    TestThreadInfo* pThreadInfo = (TestThreadInfo*) arg;
+
+    // Initialize the thread attributes for the running thread
+    result = pthread_getattr_np(pthread_self(), &attr); // Linux-specific function
+    if (result != 0) {
+        goto CleanUp;
+    }
+
+    // Retrieve the stack size from the thread attributes
+    result = pthread_attr_getstacksize(&attr, &stackSize);
+    if (result != 0) {
+        goto CleanUp;
+    }
+
+    pThreadInfo->stackSize = stackSize;
+
+CleanUp:
+    pThreadInfo->failure = result;
+    pthread_attr_destroy(&attr);
+
+    return NULL;
+}
+
+// Create a thread with the min stack size + 512 KiB
+// Then check that the thread has the requested size.
+TEST_F(ThreadFunctionalityTest, VerifyStackSize)
+{
+    TID threadId;
+    SIZE_T threadStack = 512 * 1024; // 0.5 MiB
+
+    TestThreadInfo threadInfo = {.stackSize = 0, .failure = 0};
+
+    EXPECT_EQ(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(&threadId, fetchStackSizeThreadRoutine, threadStack, &threadInfo));
+
+    EXPECT_EQ(STATUS_SUCCESS, THREAD_JOIN(threadId, NULL));
+
+    EXPECT_EQ(0, threadInfo.failure);
+    EXPECT_EQ(threadStack, threadInfo.stackSize);
 }
+
+#endif