thread_local not supported; unsafe to use c++ exceptions in multi-core setting

geurtv commented

2021-03-30 09:01:28 -05:00

(Migrated from github.com)

pico-sdk doesn't support thread-local storage (thread_local keyword), which is required for c++ exceptions to function properly in a multi-core environment. Internally libstdc++ stores some global exception state in a thread_local variable (static __thread abi::__cxa_eh_globals global; in eh_globals.cc).

If now both RP2040 cores throw or catch exceptions at the same time they are "mixed", because the thread_local state variable is the exact same variable on both cores. There are also no synchronization locks, so both cores can write to the same variable at the same time.

The code below shows how core0's std::current_exception() is overwritten by core1. The expected output is:

[core0] core0 exception
[core1] core1 exception
[core0] core0 exception
[core1] core1 exception

The actual output is:

[core0] core0 exception
[core1] core1 exception
[core0] core1 exception
[core1] core1 exception

#include <cstdio>
#include <stdexcept>
#include <pico/multicore.h>
#include <pico/stdio_usb.h>

void continue_with_other_core(semaphore_t& acquire, semaphore_t& release)
{
    sem_release(&release);
    sem_acquire_blocking(&acquire);
}

void print_current_exception()
{
    try {
        std::rethrow_exception(std::current_exception());
    }
    catch (const std::exception& e) {
        std::printf("[core%d] %s\n", get_core_num(), e.what());
    }
}

void run_test(semaphore_t& acquire, semaphore_t& release)
{
    try {
        sem_acquire_blocking(&acquire);
        throw std::runtime_error("core" + std::to_string(get_core_num()) + " exception");
    }
    catch (...) {
        print_current_exception();
        continue_with_other_core(acquire, release);
        print_current_exception();
        continue_with_other_core(acquire, release);
    }
}

int main()
{
    stdio_usb_init();
    std::getchar();
    std::puts("[start]");

    static semaphore_t sem0, sem1;
    sem_init(&sem0, 1, 1); // unlocked: start with core0
    sem_init(&sem1, 0, 1);

    multicore_launch_core1([] { run_test(sem1, sem0); });
    run_test(sem0, sem1);

    for (;;);
}

pico-sdk doesn't support thread-local storage (thread_local keyword), which is required for c++ exceptions to function properly in a multi-core environment. Internally libstdc++ stores some global exception state in a thread_local variable (`static __thread abi::__cxa_eh_globals global;` in eh_globals.cc). If now both RP2040 cores throw or catch exceptions at the same time they are "mixed", because the thread_local state variable is the exact same variable on both cores. There are also no synchronization locks, so both cores can write to the same variable at the same time. The code below shows how core0's std::current_exception() is overwritten by core1. The expected output is: ``` [core0] core0 exception [core1] core1 exception [core0] core0 exception [core1] core1 exception ``` The actual output is: ``` [core0] core0 exception [core1] core1 exception [core0] core1 exception [core1] core1 exception ``` ```c++ #include <cstdio> #include <stdexcept> #include <pico/multicore.h> #include <pico/stdio_usb.h> void continue_with_other_core(semaphore_t& acquire, semaphore_t& release) { sem_release(&release); sem_acquire_blocking(&acquire); } void print_current_exception() { try { std::rethrow_exception(std::current_exception()); } catch (const std::exception& e) { std::printf("[core%d] %s\n", get_core_num(), e.what()); } } void run_test(semaphore_t& acquire, semaphore_t& release) { try { sem_acquire_blocking(&acquire); throw std::runtime_error("core" + std::to_string(get_core_num()) + " exception"); } catch (...) { print_current_exception(); continue_with_other_core(acquire, release); print_current_exception(); continue_with_other_core(acquire, release); } } int main() { stdio_usb_init(); std::getchar(); std::puts("[start]"); static semaphore_t sem0, sem1; sem_init(&sem0, 1, 1); // unlocked: start with core0 sem_init(&sem1, 0, 1); multicore_launch_core1([] { run_test(sem1, sem0); }); run_test(sem0, sem1); for (;;); } ```

👍 1

geurtv commented

2021-07-27 06:50:26 -05:00

(Migrated from github.com)

Wrong analysis: static __thread abi::__cxa_eh_globals global is only used when _GLIBCXX_HAVE_TLS is defined. In our case the "normal" static __cxa_eh_globals eh_globals is actually used (by both cores), so obviously a c++ exception on one core then overwrites an active exception on the other core.

By the way, adding thread-local storage support is relatively easy: provide a custom implementation of __emutls_get_address and then use -Wl,--wrap= to use it. Then thread_local works just fine, but as stated that doesn't solve the issue.

Wrong analysis: `static __thread abi::__cxa_eh_globals global` is only used when `_GLIBCXX_HAVE_TLS` is defined. In our case the "normal" `static __cxa_eh_globals eh_globals` is actually used (by both cores), so obviously a c++ exception on one core then overwrites an active exception on the other core. By the way, adding thread-local storage support is relatively easy: provide a custom implementation of `__emutls_get_address` and then use `-Wl,--wrap=` to use it. Then `thread_local` works just fine, but as stated that doesn't solve the issue.

geurtv commented

2021-07-27 08:17:53 -05:00

(Migrated from github.com)

A solution could be to wrap the functions __cxa_get_globals and __cxa_get_globals_fast. One potential problem: the pointer they return is to a libstdc++-internal structure (__cxa_eh_globals), so different GCC versions can have different definitions for it.

I have a proof of principle for gcc-arm-none-eabi-10-2020-q4-major at:

https://github.com/geurtv/pico-sdk/tree/develop

file:
https://github.com/geurtv/pico-sdk/blob/develop/src/rp2_common/pico_standard_link/wrap_eh_globals.cpp

commit:
github.com/geurtv/pico-sdk@f869eb6181

A solution could be to wrap the functions `__cxa_get_globals` and `__cxa_get_globals_fast`. One potential problem: the pointer they return is to a libstdc++-internal structure (`__cxa_eh_globals`), so different GCC versions can have different definitions for it. I have a proof of principle for gcc-arm-none-eabi-10-2020-q4-major at: https://github.com/geurtv/pico-sdk/tree/develop file: https://github.com/geurtv/pico-sdk/blob/develop/src/rp2_common/pico_standard_link/wrap_eh_globals.cpp commit: https://github.com/geurtv/pico-sdk/commit/f869eb618162f605878a4e53604fb8a493721ba1

kilograham commented

2021-07-27 08:38:16 -05:00

(Migrated from github.com)

yeah, adding __thread support makes sense (but then reentrant support for lib functions makes sense which probably suggests we should build newlib ourselves with different options compared to the one shipped with gcc-arm-eabi-none)

is __cxa_get_globals in newlib or gcc libs? in any case, yes it would make sense for C++ exceptions to work on both cores (you are the first to notice that they don't... although in fairness we have them turned off by default).

yeah, adding __thread support makes sense (but then reentrant support for lib functions makes sense which probably suggests we should build newlib ourselves with different options compared to the one shipped with gcc-arm-eabi-none) is __cxa_get_globals in newlib or gcc libs? in any case, yes it would make sense for C++ exceptions to work on both cores (you are the first to notice that they don't... although in fairness we have them turned off by default).

geurtv commented

2021-07-27 08:56:35 -05:00

(Migrated from github.com)

It's all gcc as far as I can tell. __cxa_get_globals and __cxa_get_globals_fast are both in libsup++, __emutls_get_address (for thread-local storage) is in libgcc.

It's all gcc as far as I can tell. `__cxa_get_globals` and `__cxa_get_globals_fast` are both in libsup++, `__emutls_get_address` (for thread-local storage) is in libgcc.

geurtv commented

2021-07-27 09:30:45 -05:00

(Migrated from github.com)

FYI: I've also just added a wrapper for __emutls_get_address. Not sure how complete the implementation is, but thread-local storage seems to work now.

https://github.com/geurtv/pico-sdk/blob/develop/src/rp2_common/pico_standard_link/wrap_emutls.c

FYI: I've also just added a wrapper for `__emutls_get_address`. Not sure how complete the implementation is, but thread-local storage seems to work now. https://github.com/geurtv/pico-sdk/blob/develop/src/rp2_common/pico_standard_link/wrap_emutls.c

👍 1

alastairpatrick commented

2022-09-09 17:23:35 -05:00

(Migrated from github.com)

Would be good if this integrated well with an RTOS so that similar problems don't happen when there are multiple tasks running on one core. For example, FreeRTOS has vTaskSetThreadLocalStoragePointer() and pvTaskGetThreadLocalStoragePointer().

kilograham commented

2026-06-02 18:39:29 -05:00

(Migrated from github.com)

note it is worth rechecking this as thread local support was added via #2910

geurtv commented

2026-06-04 03:46:55 -05:00

(Migrated from github.com)

Nope, __thread and thread_local work correctly, but exceptions still don't. Note that gcc libs are built without thread local support (_GLIBCXX_HAVE_TLS not defined), so the internal variable active exceptions are stored in (probably still static __cxa_eh_globals eh_globals) is still the same single instance for both cores.

Tested with Arm GNU Toolchain Version 15.2.Rel1.

Nope, `__thread` and `thread_local` work correctly, but exceptions still don't. Note that gcc libs are built without thread local support (`_GLIBCXX_HAVE_TLS` not defined), so the internal variable active exceptions are stored in (probably still `static __cxa_eh_globals eh_globals`) is still the same single instance for both cores. Tested with Arm GNU Toolchain Version 15.2.Rel1.

kilograham commented

2026-06-04 11:14:55 -05:00

(Migrated from github.com)

moving exception part to 2.3.0

we are now building our own RISC-V GCC toolchain, so maybe we can look at building a thread-safe version for both Arm and GCC

Note, that is a bit annoying though; in other places __thread has been used anyway, and just defaulted to a single instance when not using our TLS support

moving exception part to 2.3.0 we are now building our own RISC-V GCC toolchain, so maybe we can look at building a thread-safe version for both Arm and GCC Note, that is a bit annoying though; in other places __thread has been used anyway, and just defaulted to a single instance when not using our TLS support

Rows
Columns

thread_local not supported; unsafe to use c++ exceptions in multi-core setting #291