Merge branch 'contrib/github_pr_14384' into 'master'

fix(pthread): Add esp_pthread function implementations for linux target (GitHub PR)

Closes IDFGH-13485

See merge request espressif/esp-idf!33278

components/pthread/include/esp_pthread.h

@@ -63,6 +63,7 @@ esp_pthread_cfg_t esp_pthread_get_default_config(void);
  * @return
  *      - ESP_OK if configuration was successfully set
  *      - ESP_ERR_NO_MEM if out of memory
+ *      - ESP_ERR_INVALID_ARG if cfg is NULL
  *      - ESP_ERR_INVALID_ARG if stack_size is less than PTHREAD_STACK_MIN
  *      - ESP_ERR_INVALID_ARG if stack_alloc_caps does not include MALLOC_CAP_8BIT
  */
@@ -79,6 +80,7 @@ esp_err_t esp_pthread_set_cfg(const esp_pthread_cfg_t *cfg);
  *
  * @return
  *      - ESP_OK if the configuration was available
+ *      - ESP_ERR_INVALID_ARG if p is NULL
  *      - ESP_ERR_NOT_FOUND if a configuration wasn't previously set
  */
 esp_err_t esp_pthread_get_cfg(esp_pthread_cfg_t *p);

components/pthread/port/linux/pthread.c

@@ -7,8 +7,25 @@
  * pthread port for Linux build
  */
 
+#include <pthread.h>
+#include <sys/param.h>
+#include "sdkconfig.h"
 #include "esp_pthread.h"
+#include "esp_heap_caps.h"
+
 #include <string.h>
+#include "freertos/FreeRTOS.h"
+
+static pthread_key_t s_pthread_cfg_key;
+static void esp_pthread_cfg_key_destructor(void *value)
+{
+    free(value);
+}
+
+static int get_default_pthread_core(void)
+{
+    return CONFIG_PTHREAD_TASK_CORE_DEFAULT == -1 ? tskNO_AFFINITY : CONFIG_PTHREAD_TASK_CORE_DEFAULT;
+}
 
 /**
  * @brief Creates a default pthread configuration based
@@ -24,20 +41,72 @@ esp_pthread_cfg_t esp_pthread_get_default_config(void)
         .prio = CONFIG_PTHREAD_TASK_PRIO_DEFAULT,
         .inherit_cfg = false,
         .thread_name = NULL,
-        .pin_to_core = 0,
-        .stack_alloc_caps = 0,
+        .pin_to_core = get_default_pthread_core(),
+        .stack_alloc_caps = MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT,
     };
 
     return cfg;
 }
 
 esp_err_t esp_pthread_set_cfg(const esp_pthread_cfg_t *cfg)
 {
+    // Not checking the stack size here since PTHREAD_STACK_MIN has two conflicting declarations on Linux
+
+    if (cfg == NULL) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
+    // 0 is treated as default value, hence change caps to MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL in that case
+    int heap_caps;
+    if (cfg->stack_alloc_caps == 0) {
+        heap_caps = MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL;
+    } else {
+        // Check that memory is 8-bit capable
+        if (!(cfg->stack_alloc_caps & MALLOC_CAP_8BIT)) {
+            return ESP_ERR_INVALID_ARG;
+        }
+
+        heap_caps = cfg->stack_alloc_caps;
+    }
+
+    /* If a value is already set, update that value */
+    esp_pthread_cfg_t *p = pthread_getspecific(s_pthread_cfg_key);
+    if (!p) {
+        p = malloc(sizeof(esp_pthread_cfg_t));
+        if (!p) {
+            return ESP_ERR_NO_MEM;
+        }
+    }
+    *p = *cfg;
+    p->stack_alloc_caps = heap_caps;
+    p->stack_size = MAX(p->stack_size, 0x4000); // make sure Linux minimal stack size is respected
+
+    int __attribute((unused)) res = pthread_setspecific(s_pthread_cfg_key, p);
+
+    assert(res == 0);
+
     return ESP_OK;
 }
 
 esp_err_t esp_pthread_get_cfg(esp_pthread_cfg_t *p)
 {
+    if (p == NULL) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
+    esp_pthread_cfg_t *cfg = pthread_getspecific(s_pthread_cfg_key);
+    if (cfg) {
+        *p = *cfg;
+        return ESP_OK;
+    }
     memset(p, 0, sizeof(*p));
     return ESP_ERR_NOT_FOUND;
 }
+
+__attribute__((constructor)) esp_err_t esp_pthread_init(void)
+{
+    if (pthread_key_create(&s_pthread_cfg_key, esp_pthread_cfg_key_destructor) != 0) {
+        return ESP_ERR_NO_MEM;
+    }
+    return ESP_OK;
+}

components/pthread/pthread.c

@@ -142,6 +142,10 @@ static void pthread_delete(esp_pthread_t *pthread)
 /* Call this function to configure pthread stacks in Pthreads */
 esp_err_t esp_pthread_set_cfg(const esp_pthread_cfg_t *cfg)
 {
+    if (cfg == NULL) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
     if (cfg->stack_size < PTHREAD_STACK_MIN) {
         return ESP_ERR_INVALID_ARG;
     }
@@ -174,12 +178,16 @@ esp_err_t esp_pthread_set_cfg(const esp_pthread_cfg_t *cfg)
     p->stack_alloc_caps = heap_caps;
     pthread_setspecific(s_pthread_cfg_key, p);
 
-    return 0;
+    return ESP_OK;
     ESP_COMPILER_DIAGNOSTIC_POP("-Wanalyzer-malloc-leak")
 }
 
 esp_err_t esp_pthread_get_cfg(esp_pthread_cfg_t *p)
 {
+    if (p == NULL) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
     ESP_RETURN_ON_ERROR(lazy_init_pthread_cfg_key(), TAG, "Failed to initialize pthread key");
 
     esp_pthread_cfg_t *cfg = pthread_getspecific(s_pthread_cfg_key);

components/pthread/test_apps/.build-test-rules.yml

@@ -4,3 +4,7 @@ components/pthread/test_apps/pthread_psram_tests:
   enable:
     - if: IDF_TARGET in ["esp32"]
       reason: PSRAM only available on ESP32, S2, S3; code is fairly generic
+
+components/pthread/test_apps/pthread_unity_tests:
+  enable:
+    - if: IDF_TARGET in ["esp32", "esp32c2", "esp32c3", "esp32c5", "esp32c6", "esp32c61", "esp32h2", "esp32p4", "esp32s2", "esp32s3", "linux"]

components/pthread/test_apps/pthread_unity_tests/README.md

@@ -1,2 +1,2 @@
-| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C5 | ESP32-C6 | ESP32-C61 | ESP32-H2 | ESP32-P4 | ESP32-S2 | ESP32-S3 |
-| ----------------- | ----- | -------- | -------- | -------- | -------- | --------- | -------- | -------- | -------- | -------- |
+| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C5 | ESP32-C6 | ESP32-C61 | ESP32-H2 | ESP32-P4 | ESP32-S2 | ESP32-S3 | Linux |
+| ----------------- | ----- | -------- | -------- | -------- | -------- | --------- | -------- | -------- | -------- | -------- | ----- |

components/pthread/test_apps/pthread_unity_tests/main/CMakeLists.txt

@@ -1,12 +1,19 @@
-set(sources "test_app_main.c"
-            "test_pthread.c"
-            "test_pthread_cond_var.c"
-            "test_pthread_local_storage.c"
-            "test_pthread_cxx.cpp"
-            "test_pthread_rwlock.c"
-            "test_pthread_semaphore.c")
+idf_build_get_property(target IDF_TARGET)
+
+set(sources "test_app_main.c" "test_esp_pthread.c")
+set(priv_requires "pthread" "unity")
+
+if(NOT ${target} STREQUAL "linux")
+    list(APPEND sources "test_pthread.c"
+                        "test_pthread_cond_var.c"
+                        "test_pthread_local_storage.c"
+                        "test_pthread_cxx.cpp"
+                        "test_pthread_rwlock.c"
+                        "test_pthread_semaphore.c")
+    list(APPEND priv_requires "esp_timer" "test_utils")
+endif()
 
 idf_component_register(SRCS ${sources}
                        INCLUDE_DIRS "."
-                       REQUIRES pthread esp_timer test_utils
+                       REQUIRES ${priv_requires}
                        WHOLE_ARCHIVE)

components/pthread/test_apps/pthread_unity_tests/main/test_app_main.c

@@ -1,44 +1,35 @@
 /*
- * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#include <stdio.h>
 #include <errno.h>
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
 #include "unity.h"
 #include "unity_test_runner.h"
-#include "esp_heap_caps.h"
+#include "unity_test_utils_memory.h"
 
 // Some resources are lazy allocated (e.g. newlib locks), the threshold is left for that case
 #define TEST_MEMORY_LEAK_THRESHOLD (-200)
 
-static size_t before_free_8bit;
-static size_t before_free_32bit;
-
-static void check_leak(size_t before_free, size_t after_free, const char *type)
-{
-    ssize_t delta = after_free - before_free;
-    printf("MALLOC_CAP_%s: Before %u bytes free, After %u bytes free (delta %d)\n", type, before_free, after_free, delta);
-    TEST_ASSERT_MESSAGE(delta >= TEST_MEMORY_LEAK_THRESHOLD, "memory leak");
-}
-
 void setUp(void)
 {
-    before_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT);
-    before_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT);
+    unity_utils_set_leak_level(TEST_MEMORY_LEAK_THRESHOLD);
+    unity_utils_record_free_mem();
     errno = 0;
 }
 
 void tearDown(void)
 {
+#ifndef CONFIG_IDF_TARGET_LINUX // on Linux, we don't check for memory leaks with memory utils
     // Add a short delay of 200ms to allow the idle task to free remaining memory
     vTaskDelay(pdMS_TO_TICKS(200));
-    size_t after_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT);
-    size_t after_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT);
-    check_leak(before_free_8bit, after_free_8bit, "8BIT");
-    check_leak(before_free_32bit, after_free_32bit, "32BIT");
+#endif // CONFIG_IDF_TARGET_LINUX
+
+    unity_utils_evaluate_leaks();
 }
 
 void app_main(void)

components/pthread/test_apps/pthread_unity_tests/main/test_esp_pthread.c

@@ -0,0 +1,95 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Unlicense OR CC0-1.0
+ */
+
+#include <pthread.h>
+#include "sdkconfig.h"
+#include "esp_pthread.h"
+#include "esp_heap_caps.h"
+#include "unity.h"
+
+TEST_CASE("esp_pthread_get_default_config creates correct stack memory capabilities", "[cfg]")
+{
+    esp_pthread_cfg_t default_config = esp_pthread_get_default_config();
+
+    // The default must always be internal, 8-bit accessible RAM
+    TEST_ASSERT_EQUAL_HEX(MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL, default_config.stack_alloc_caps);
+}
+
+TEST_CASE("null pointers are rejected", "[cfg]")
+{
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, esp_pthread_set_cfg(NULL));
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, esp_pthread_get_cfg(NULL));
+}
+
+TEST_CASE("wrong heap caps are rejected", "[cfg]")
+{
+    esp_pthread_cfg_t default_config = esp_pthread_get_default_config();
+    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_set_cfg(&default_config)); // make sure we have saved a known value
+
+    // Test the rejection of wrong values
+    default_config.stack_alloc_caps = MALLOC_CAP_32BIT;
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, esp_pthread_set_cfg(&default_config));
+
+    default_config.stack_alloc_caps = MALLOC_CAP_32BIT | MALLOC_CAP_INTERNAL;
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, esp_pthread_set_cfg(&default_config));
+
+    // check that saved values are unaltered
+    esp_pthread_cfg_t retrieved_config;
+    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_get_cfg(&retrieved_config));
+    TEST_ASSERT_EQUAL_HEX(MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL, retrieved_config.stack_alloc_caps);
+}
+
+// On Linux, we silently adjust the stack size since pthread on Linux
+// requires a minimum stack size of 0x4000.
+#if !CONFIG_IDF_TARGET_LINUX
+TEST_CASE("invalid stack size is rejected", "[cfg]")
+{
+    esp_pthread_cfg_t default_config = esp_pthread_get_default_config();
+    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_set_cfg(&default_config)); // make sure we have saved a known value
+
+    // Test the rejection of wrong values
+    default_config.stack_size = PTHREAD_STACK_MIN - 1;
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, esp_pthread_set_cfg(&default_config));
+
+    // check that saved values are unaltered
+    esp_pthread_cfg_t retrieved_config;
+    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_get_cfg(&retrieved_config));
+    TEST_ASSERT_EQUAL(CONFIG_PTHREAD_TASK_STACK_SIZE_DEFAULT, retrieved_config.stack_size);
+}
+#endif // !CONFIG_IDF_TARGET_LINUX
+
+TEST_CASE("correct memory is accepted", "[cfg]")
+{
+    esp_pthread_cfg_t default_config = esp_pthread_get_default_config();
+
+    default_config.stack_alloc_caps = MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL;
+    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_set_cfg(&default_config));
+}
+
+TEST_CASE("configuration is preserved inside pthread", "[cfg]")
+{
+    esp_pthread_cfg_t saved_config;
+    esp_pthread_cfg_t retrieved_config;
+    saved_config.stack_size = PTHREAD_STACK_MIN;
+    saved_config.prio = 5;
+    saved_config.inherit_cfg = true;
+    saved_config.thread_name = "test_esp_pthread";
+    saved_config.pin_to_core = 0;
+    saved_config.stack_alloc_caps = MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL;
+
+    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_set_cfg(&saved_config));
+    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_get_cfg(&retrieved_config));
+
+    TEST_ASSERT_EQUAL(saved_config.stack_size, retrieved_config.stack_size);
+    TEST_ASSERT_EQUAL(saved_config.prio, retrieved_config.prio);
+    TEST_ASSERT_EQUAL(saved_config.inherit_cfg, retrieved_config.inherit_cfg);
+    TEST_ASSERT_EQUAL(saved_config.thread_name, retrieved_config.thread_name);
+    TEST_ASSERT_EQUAL(saved_config.pin_to_core, retrieved_config.pin_to_core);
+    TEST_ASSERT_EQUAL(saved_config.stack_alloc_caps, retrieved_config.stack_alloc_caps);
+
+    esp_pthread_cfg_t cfg = esp_pthread_get_default_config();
+    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_set_cfg(&cfg));
+}

components/pthread/test_apps/pthread_unity_tests/main/test_pthread.c

@@ -13,33 +13,6 @@
 
 #include "unity.h"
 
-TEST_CASE("esp_pthread_get_default_config creates correct stack memory capabilities", "[set_cfg]")
-{
-    esp_pthread_cfg_t default_config = esp_pthread_get_default_config();
-
-    // The default must always be internal, 8-bit accessible RAM
-    TEST_ASSERT_EQUAL_HEX(MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL, default_config.stack_alloc_caps);
-}
-
-TEST_CASE("wrong heap caps are rejected", "[set_cfg]")
-{
-    esp_pthread_cfg_t default_config = esp_pthread_get_default_config();
-
-    default_config.stack_alloc_caps = MALLOC_CAP_32BIT;
-    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, esp_pthread_set_cfg(&default_config));
-
-    default_config.stack_alloc_caps = MALLOC_CAP_32BIT | MALLOC_CAP_INTERNAL;
-    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, esp_pthread_set_cfg(&default_config));
-}
-
-TEST_CASE("correct memory is accepted", "[set_cfg]")
-{
-    esp_pthread_cfg_t default_config = esp_pthread_get_default_config();
-
-    default_config.stack_alloc_caps = MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL;
-    TEST_ASSERT_EQUAL(ESP_OK, esp_pthread_set_cfg(&default_config));
-}
-
 static void *compute_square(void *arg)
 {
     int *num = (int *) arg;

components/pthread/test_apps/pthread_unity_tests/pytest_pthread_unity_tests.py

@@ -62,3 +62,9 @@ def test_pthread_qemu(dut: Dut) -> None:
     for case in dut.test_menu:
         if 'qemu-ignore' not in case.groups and case.type == 'normal':
             dut._run_normal_case(case, timeout=75)
+
+
+@pytest.mark.linux
+@pytest.mark.host_test
+def test_pthread_linux(dut: Dut) -> None:
+    dut.run_all_single_board_cases(timeout=120)