zephyr

zephyr copied to clipboard

This commit adds support for std::thread and std::this_thread when the configured C++ standard is >= C++11.

Fixes #25569

SeaBIOS (version rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org)
Booting from ROM..*** Booting Zephyr OS build zephyr-v3.0.0-1074-g90c018160af3  ***
version 201703
Running test suite std_thread_tests
===================================================================
START - test_this_thread_get_id
 PASS - test_this_thread_get_id in 0.2 seconds
===================================================================
START - test_this_thread_yield
 PASS - test_this_thread_yield in 0.1 seconds
===================================================================
START - test_this_thread_sleep_until
 PASS - test_this_thread_sleep_until in 0.1 seconds
===================================================================
START - test_this_thread_sleep_for
 PASS - test_this_thread_sleep_for in 0.1 seconds
===================================================================
START - test_thread_get_id
 PASS - test_thread_get_id in 0.1 seconds
===================================================================
START - test_thread_native_handle
 SKIP - test_thread_native_handle in 0.1 seconds
===================================================================
START - test_thread_hardware_concurrency
 PASS - test_thread_hardware_concurrency in 0.1 seconds
===================================================================
START - test_thread_join
 SKIP - test_thread_join in 0.1 seconds
===================================================================
START - test_thread_detach
 SKIP - test_thread_detach in 0.1 seconds
===================================================================
START - test_thread_swap
 SKIP - test_thread_swap in 0.1 seconds
===================================================================
Test suite std_thread_tests succeeded
===================================================================
PROJECT EXECUTION SUCCESSFUL

Notes:

• Tests pass for CONFIG_ARCH_POSIX=y and CONFIG_ARCH_POSIX=n
• Currently skipping a few tests when CONFIG_ARCH_POSIX=n as we cannot yet auto-allocate thread stacks
• Requires a couple of toolchain header modifications (described in comments / PR)
• Eventually would be ideal to bypass POSIX threads and use Zephyr native threads, but they are a convenient stepping stone

The toolchain header include/bits/gthr.h was modified as follows:

#if _GLIBCXX___ANDROID__
#include <bits/gthr-posix.h>
#elif defined(__ZEPHYR__)
#include <bits/gthr-zephyr.h>
#else
#include <bits/gthr-default.h>
#endif

The file include/bits/gthr-zephyr.h is a copy of the upstream gthr-posix.h with the following additions. Normally, I would suggest keeping this in the toolchain, as this file is GPLv3. It is included here for reference purposes only.

#define __GTHREADS 1
#define __GTHREADS_CXX0X 1
#define _GTHREAD_USE_MUTEX_TIMEDLOCK 1
#define _GLIBCXX_USE_NANOSLEEP 1

struct __sFILE_fake {
	unsigned char *_p; /* current position in (some) buffer */
	int _r; /* read space left for getc() */
	int _w; /* write space left for putc() */
	short _flags; /* flags, below; this FILE is free if 0 */
	short _file; /* fileno, if Unix descriptor, else -1 */
	struct __sbuf _bf; /* the buffer (at least 1 byte, if !NULL) */
	int _lbfsize; /* 0 or -_bf._size, for inline putc */

	struct _reent *_data;
};

typedef long _off_t;
typedef unsigned long size_t;
typedef long ssize_t;
#define _fpos_t long
#define __FILE struct __sFILE_fake
typedef __builtin_va_list __gnuc_va_list;
#define PTHREAD_MUTEX_INITIALIZER {0}
#define PTHREAD_COND_INITIALIZER {0}

#include <posix/pthread.h>

#if ((defined(_LIBOBJC) || defined(_LIBOBJC_WEAK)) \
     || !defined(_GTHREAD_USE_MUTEX_TIMEDLOCK))
# include <posix/unistd.h>

@cfriedt is there any reason why this pull-req is not progressing forward?

I'm also very interested in getting better C++ support into Zephyr and would like to help out. Is there any specific I can help out with here?

I've tested this PR with the SDK changes mentioned above and my 5-min test says it works fine.

@cfriedt is there any reason why this pull-req is not progressing forward?

@ortogonal The biggest reasons are

1. Zephyr has no way currently to dynamically allocate a thread stack (there is a draft pr for that #44379)
2. The related toolchain integrations need to happen (some work for @stephanosio)
3. Additional work may be necessary around k_mutex (iirc, there may be some static requirement for Zephyr mutex objects)

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@cfriedt I want to add more relevant implementations to this PR like:

• std::mutex
• std::timed_mutex

Which I see were referecned in #45785 but are not addressed here. But are all of the blocking items still relevant even if it's just the mutex?

Hi @ShaharHD

@cfriedt I want to add more relevant implementations to this PR like:

• std::mutex
• std::timed_mutex

Which I see were referecned in #45785 but are not addressed here.

Well, we could, in theory, add C++ support now using the gthr_posix approach, but it would come with the caveat that C++ std::thread would be completely non-functional.

But are all of the blocking items still relevant even if it's just the mutex?

From what I remember, no. That can be done.

Thoughts, @stephanosio ?

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

• rebased (kind of insane to believe the last rev for this was 3 years ago 😅 )
• now you just need to change bits/gthr.h to include bits/gthr-posix.h

Currently fails, as it relies on automatic thread stack allocation via pthreads, but that is expected until #44379 and #44727 are merged.

TSC Meeting 2023-08-30: Since this came up again, and since it was only relatively recently that this was unblocked (#44379, #44727), I had a few minutes so I thought I would give it another shot.

On my desk, using Zephyr SDK v0.16.1 and Zephyr commit f0c10d0332ec7cfc7582873c4cb4e3205efe2106.

With unmodified gthr-posix.h there are only a few minute tweaks needed to get this working.

The modifications I made were below.

1. include <bits/gthr-posix.h> instead of <bits/gthr-default.h> in <bits/gthr.h>
2. #define _GLIBCXX_HAS_GTHREADS 1 in bits/c++config.h
3. #define _GLIBCXX_USE_NANOSLEEP 1 in bits/c++config.h
4. Use pthread_exit(nullptr) (or k_thread_abort(k_current_get())) instead of terminate() in std::terminate()

For the last one, the (C) terminate() function is undocumented and seems like a gcc-ism. It's unfortunately non-weak, so we can't easily override it.

The reason we cannot usestd::terminate() is that the (C) terminate() ends up calling abort().

That's OK on an OS with the concept of a process, because the OS would naturally keep going after a process terminates, but for a multi-threaded RTOS with a single-process view, it causes everything to stop.

I think at this point, it's probably safe to make a PR to sdk-ng.

Have not yet attempted any C++ threading with LLVM.

cc @nashif @carlescufi @yperess @stephanosio

@cfriedt I think the concern is that even if we get it working with the Zephyr toolchain that doesn't solve the issue for everyone right? Some projects use LLVM, others might have an internal GCC (can't use the Zephyr toolchain). While I think there's value in doing this for the Zephyr SDK. I do like the Pigweed solution better because it's a software solution that will work for everyone.

@cfriedt I think the concern is that even if we get it working with the Zephyr toolchain that doesn't solve the issue for everyone right?

Not sure I agree. Both GCC and LLVM have first-class support for POSIX in terms of C++ threads, so from that perspective, it's already like 95% of users.

It's relatively straightforward to configure GCC to do what we need. I just haven't gotten to LLVM yet - would be great to get some help there.

Some projects use LLVM, others might have an internal GCC (can't use the Zephyr toolchain).

Of course. The Zephyr SDK is just GCC at the end of the day, and we can provide our config as a reference for those who want to use custom GCC-based toolchains. Similarly, once we can support LLVM, then we'll provide the relevant configuration bits as well.

One step at a time though.

The cpp_synchronization sample should probably be updated to use std::binary_semaphore or std::counting_semaphore.

std::mutex and std::condition_variable are working fine (using gcc)

Some TODO items:

• modify Zephyr SDK config options to support C++ by default
• provide documentation for those configuring their own toolchains and / or 3rd-party toolchain vendors based on GCC
• get C++ working with LLVM

Added tests for std::mutex and std::condition_variable as well.

Hi @cfriedt ,

I am trying to build a custom aarch64 toolchain with crosstool-ng and newlib and also c++ support to use with zephyr 3.4 and posix.

I've tried these steps so I can use std::mutex, std::thread, etc.

With unmodified gthr-posix.h there are only a few minute tweaks needed to get this working.

The modifications I made were below.

1. include `<bits/gthr-posix.h>` instead of `<bits/gthr-default.h>` in `<bits/gthr.h>`

2. `#define _GLIBCXX_HAS_GTHREADS 1` in `bits/c++config.h`

3. `#define _GLIBCXX_USE_NANOSLEEP 1` in `bits/c++config.h`

4. Use `pthread_exit(nullptr)` (or `k_thread_abort(k_current_get())`) instead of `terminate()` in `std::terminate()`

Do you confirm that the following steps are done after toolchain's install step? because if doing these modifications before building gcc the build breaks because of undefined pthread objects, such as:

src/gcc/libgcc/gthr-posix.h:102:9: error: 'pthread_once' undeclared here (not in a function); did you mean 'pthread_once_t'? src/gcc/libgcc/gthr-posix.h:103:9: error: 'pthread_getspecific' undeclared here (not in a function) src/gcc/libgcc/gthr-posix.h:106:9: error: 'pthread_create' undeclared here (not in a function)

However, if I patch c++config.h and gthr.h after the install step, I manage to build the toolchain and when building some zephyr code that uses std::mutex/condition_variable/thread it compiles fine but of course fails at link time because the cc files are all flagged with #ifdef _GLIBCXX_HAS_GTHREADS (which is not defined when building libstdc++-v3)

Do you spot if I am missing something here?

Thanks in advance,

Valentin

Hi @valentinkorenblit

Do you confirm that the following steps are done after toolchain's install step? because if doing this modifications before building gcc the build breaks because of undefined pthread objects, such as:

That was the way that I tested my code, yes.

However, the proper way to configure a custom GCC + Newlib toolchain would be to apply any necessary patches and then use the appropriate arguments to ./configure.

There are great projects out there to make it significantly easier - yocto, buildroot, gentoo, crossdev-ng, of course.

However, if I patch c++config.h and gthr.h after the install step, I manage to build the toolchain and when building some zephyr code that uses std::mutex/condition_variable/thread it compiles fine but of course fails at link time because the cc files are all flagged with #ifdef _GLIBCXX_HAS_GTHREADS (which is not defined when building libstdc++-v3)

Exactly - so you would need to apply patches first, and then use the appropriate arguments to ./configure.

Do you spot if I am missing something here?

Nope - you're basically a couple of steps ahead of me due to lack of bandwidth on my part.

If you happen to find the correct arguments to ./configure, can you please note them down for us?

This would be really helpful to add to a page under "Beyond the Getting Started Guide", and similar steps would be great to document for configuring LLVM.

Eventually, it would be great to stabilize this and just upstream the configuration to both GCC and LLVM, but that is a bit further down the road.

Thanks in advance

Likewise 😀

I tried some rudimentary semaphore tests (binary and counting) and that works fine. No changes required.

What I would very much like to test out is std::coroutine

There is also a separate PR for LLVM's libc++ (#63301), which is great 😁 As much as people love to hate on C and C++, they are at least standardized by ISO.

cc: @kgugala, @yperess

Reproducing again, the patch below is all that is required to modify an existing Zephyr SDK toolchain install (to use C++).

I mitigated item 4 by calling std::set_terminate() from a SYS_INIT() macro (so no longer need to patch c++config.h with pthread_exit().

diff -Nuar riscv64-zephyr-elf_unomodified/riscv64-zephyr-elf/include/c++/12.2.0/riscv64-zephyr-elf/rv64imac_zicsr_zifencei/lp64/medany/bits/c++config.h riscv64-zephyr-elf/riscv64-zephyr-elf/include/c++/12.2.0/riscv64-zephyr-elf/rv64imac_zicsr_zifencei/lp64/medany/bits/c++config.h
--- riscv64-zephyr-elf_unomodified/riscv64-zephyr-elf/include/c++/12.2.0/riscv64-zephyr-elf/rv64imac_zicsr_zifencei/lp64/medany/bits/c++config.h	2023-11-15 06:45:56.000000000 -0500
+++ riscv64-zephyr-elf/riscv64-zephyr-elf/include/c++/12.2.0/riscv64-zephyr-elf/rv64imac_zicsr_zifencei/lp64/medany/bits/c++config.h	2023-12-01 14:54:24.261480902 -0500
@@ -1688,7 +1688,7 @@
 #define _GLIBCXX_FULLY_DYNAMIC_STRING 0
 
 /* Define if gthreads library is available. */
-/* #undef _GLIBCXX_HAS_GTHREADS */
+#define _GLIBCXX_HAS_GTHREADS 1
 
 /* Define to 1 if a full hosted library is built, or 0 if freestanding. */
 #define _GLIBCXX_HOSTED 1
@@ -1820,7 +1820,7 @@
 /* #undef _GLIBCXX_USE_LSTAT */
 
 /* Defined if nanosleep is available. */
-/* #undef _GLIBCXX_USE_NANOSLEEP */
+#define _GLIBCXX_USE_NANOSLEEP 1
 
 /* Define if NLS translations are to be used. */
 /* #undef _GLIBCXX_USE_NLS */
diff -Nuar riscv64-zephyr-elf_unomodified/riscv64-zephyr-elf/include/c++/12.2.0/riscv64-zephyr-elf/rv64imac_zicsr_zifencei/lp64/medany/bits/gthr.h riscv64-zephyr-elf/riscv64-zephyr-elf/include/c++/12.2.0/riscv64-zephyr-elf/rv64imac_zicsr_zifencei/lp64/medany/bits/gthr.h
--- riscv64-zephyr-elf_unomodified/riscv64-zephyr-elf/include/c++/12.2.0/riscv64-zephyr-elf/rv64imac_zicsr_zifencei/lp64/medany/bits/gthr.h	2023-11-15 06:45:56.000000000 -0500
+++ riscv64-zephyr-elf/riscv64-zephyr-elf/include/c++/12.2.0/riscv64-zephyr-elf/rv64imac_zicsr_zifencei/lp64/medany/bits/gthr.h	2023-12-01 13:24:00.184251743 -0500
@@ -145,7 +145,7 @@
 #define _GLIBCXX_GTHREAD_USE_WEAK 1
 #endif
 #endif
-#include <bits/gthr-default.h>
+#include <bits/gthr-posix.h>
 
 #ifndef _GLIBCXX_HIDE_EXPORTS
 #pragma GCC visibility pop

Links to relevant SDK-related PRs

• [ ] zephyrproject-rtos/gcc#25
• [ ] zephyrproject-rtos/sdk-ng#722

Building the Zephyr SDK manually is successful with the current changes in zephyrproject-rtos/gcc#25 zephyrproject-rtos/sdk-ng#722 but unfortunately the CI fails without really having any kind of decent diagnostics or debug information.

build.log.xz.txt (xz -d < build.log.xz.txt > build.log.txt)

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Slightly different direction at this point - namely using the ISO C11 threads API to support ISO C++11 threads in gcc. Just performing some final testing.

https://github.com/zephyrproject-rtos/sdk-ng/pull/735

I am trying to build a custom aarch64 toolchain with crosstool-ng and newlib and also c++ support to use with zephyr 3.4 and posix.

@valentinkorenblit - check out the changes in this PR as well as the submodule changes (it renders much better on the desktop at the moment).

https://github.com/zephyrproject-rtos/sdk-ng/pull/735

That should allow you quite easily to build a cross toolchain with C++ support using either newlib or picolibc.

I am trying to build a custom aarch64 toolchain with crosstool-ng and newlib and also c++ support to use with zephyr 3.4 and posix.

@valentinkorenblit - check out the changes in this PR as well as the submodule changes (it renders much better on the desktop at the moment).

zephyrproject-rtos/sdk-ng#735

That should allow you quite easily to build a cross toolchain with C++ support using either newlib or picolibc.

Hi @cfriedt , unfortunately I won't be able to test it because my target does not have an FPU and I had to do a lot more tricks to get it working. I finished this a few months ago but it is not something worth upstreaming.

marked DNM until sdk-ng changes are accepted.