diff --git a/components/pthread/include/esp_pthread.h b/components/pthread/include/esp_pthread.h index 45a58da34d..4eda567fcd 100644 --- a/components/pthread/include/esp_pthread.h +++ b/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); diff --git a/components/pthread/port/linux/pthread.c b/components/pthread/port/linux/pthread.c index e5646da4d5..9f1d2b40a7 100644 --- a/components/pthread/port/linux/pthread.c +++ b/components/pthread/port/linux/pthread.c @@ -7,8 +7,25 @@ * pthread port for Linux build */ +#include +#include +#include "sdkconfig.h" #include "esp_pthread.h" +#include "esp_heap_caps.h" + #include +#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,8 +41,8 @@ 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; @@ -33,11 +50,63 @@ esp_pthread_cfg_t esp_pthread_get_default_config(void) 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; +} diff --git a/components/pthread/pthread.c b/components/pthread/pthread.c index 51931c0d02..0e73fe86bf 100644 --- a/components/pthread/pthread.c +++ b/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); diff --git a/components/pthread/test_apps/.build-test-rules.yml b/components/pthread/test_apps/.build-test-rules.yml index 2856cec83b..2c28a098c3 100644 --- a/components/pthread/test_apps/.build-test-rules.yml +++ b/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"] diff --git a/components/pthread/test_apps/pthread_unity_tests/README.md b/components/pthread/test_apps/pthread_unity_tests/README.md index 7b96141437..d2290ca669 100644 --- a/components/pthread/test_apps/pthread_unity_tests/README.md +++ b/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 | +| ----------------- | ----- | -------- | -------- | -------- | -------- | --------- | -------- | -------- | -------- | -------- | ----- | diff --git a/components/pthread/test_apps/pthread_unity_tests/main/CMakeLists.txt b/components/pthread/test_apps/pthread_unity_tests/main/CMakeLists.txt index f7a98a39ae..312997658e 100644 --- a/components/pthread/test_apps/pthread_unity_tests/main/CMakeLists.txt +++ b/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) diff --git a/components/pthread/test_apps/pthread_unity_tests/main/test_app_main.c b/components/pthread/test_apps/pthread_unity_tests/main/test_app_main.c index 0f773d0fed..7f98a26b29 100644 --- a/components/pthread/test_apps/pthread_unity_tests/main/test_app_main.c +++ b/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 #include #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) diff --git a/components/pthread/test_apps/pthread_unity_tests/main/test_esp_pthread.c b/components/pthread/test_apps/pthread_unity_tests/main/test_esp_pthread.c new file mode 100644 index 0000000000..08904349fc --- /dev/null +++ b/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 +#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)); +} diff --git a/components/pthread/test_apps/pthread_unity_tests/main/test_pthread.c b/components/pthread/test_apps/pthread_unity_tests/main/test_pthread.c index 2ebc220b13..551158bda8 100644 --- a/components/pthread/test_apps/pthread_unity_tests/main/test_pthread.c +++ b/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; diff --git a/components/pthread/test_apps/pthread_unity_tests/pytest_pthread_unity_tests.py b/components/pthread/test_apps/pthread_unity_tests/pytest_pthread_unity_tests.py index a2a231214e..a9472aa73d 100644 --- a/components/pthread/test_apps/pthread_unity_tests/pytest_pthread_unity_tests.py +++ b/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)