stephanosio commented 2 years ago

This issue describes the future plans for the C++ support in the Zephyr RTOS.

Tasks

[ ] Unify C++ source and header file extensions (#44196)
[ ] Establish C++ coding guidelines (#45195)
[x] Refactor C++ support in Zephyr (https://github.com/zephyrproject-rtos/zephyr/issues/36644#issue-933805956)
[ ] Add native Zephyr support in the libstdc++ (https://github.com/zephyrproject-rtos/sdk-ng/issues/350)
[ ] Add comprehensive C++ library testsuite
- shall test the functional requirements of all supported C++ libraries (mainly libstdc++ for now) to ensure that the C++ library meets the Zephyr expectations.
- shall have common codebase for testing different C++ libraries (focus on testing standard C++ features).
[ ] Add std::mutex, std::condition_variable, std::counting_semaphore, and std::binary_semaphore support (via gthr-posix.h, see below)
[ ] Add std::thread support (via gthr-posix.h, https://github.com/zephyrproject-rtos/zephyr/pull/43729)
[ ] Add std::async and std::future support https://github.com/zephyrproject-rtos/zephyr/issues/31559

Resources

Issue describing the status quo of the C++ support in Zephyr: https://github.com/zephyrproject-rtos/zephyr/issues/31281

stephanosio commented 2 years ago

cc @cfriedt @yperess @alexanderwachter

XenuIsWatching commented 2 years ago

any reason why std::condition_variable, std::counting_semaphore, and std::binary_semaphore isn't included?

cfriedt commented 2 years ago

any reason why std::condition_variable, std::counting_semaphore, and std::binary_semaphore isn't included?

Good call - definitely on the roadmap. I updated the description slightly. A lot of things we would get "for free" with #43729.

Big blocker for std::thread (at least dynamically) is #44397 (and having time to work on upstream)

aborisovich commented 2 years ago

Might be a chance to review existing C style rules. I am talking about positioning "" on the "right side" of the type definition. For C nobody cares where it is positioned (shamely), in C++ it starts to make more sense with introduction of reference types. Basically it should be: When & and characters are "glued to the left" side during type declaration (following are standard rules - note how other specifiers behave - everything type related to the left then variable name on the right, we don't do like in the line 0 here:

C++ brings more to typing on the table. Please consider this change it really makes sense.

stephanosio commented 2 years ago

As far as C is concerned, int * is the only correct form because:

int *a, *b;

For C++, we could potentially adopt int* and ban multiple variable declarations in one line.

aborisovich commented 2 years ago

As far as C is concerned, int * is the only correct form because:
int *a, *b;
For C++, we could potentially adopt int* and ban multiple variable declarations in one line.

I've just read about it... It is so "broken". Both in C and C++... Existing style makes some sense then. Seems like this "problem" carries over to multiple reference types inline declaration. int a = 1, b = 2, c = 3; int& c = a, &d = b, &e = c; However I've never seem this syntax in C++ production code until @stephanosio brought up this subject. And I think I've rarely seen anyone declaring reference types like int &a = b; Or going further: int &&a = b; Maybe I'm too young and just haven't seen enough in my life but it seems to me that the very same problem exists in C++ and it is mostly resolved by "don't use that confusing many inline syntax"... I still believe that my previous post brings more intuitive code at the only cost of that multiple inline definition.

dberlin commented 2 years ago

Just to try to help a little i can confirm that hacking up the c++conf.h and gthr.h for the compiler is sufficient to make shared_mutex/condition_variable/etc compile fine without changes. In particular, c++conf.h (for the toolchains in zephyr-sdk 0.14.2) just needs needs:

#define _GTHREAD_USE_MUTEX_INIT_FUNC 1
#define _GTHREAD_USE_COND_INIT_FUNC 1

(to not use the initializers which zephyr doesn't support)

and gthr.h needs:

#if _GLIBCXX___ANDROID__ || CONFIG_PTHREAD_IPC
#include <bits/gthr-posix.h>
#else
#include <bits/gthr-default.h>
#endif

(I have no earthly idea why it's gated on ANDROID and not a GLIBCXX__PTHREADs macro or something)

to use gthr-posix.h, you need `CONFIG_PTHREAD_IPC=y

CONFIG_POSIX_API=y

CONFIG_APP_LINK_WITH_POSIX_SUBSYS=y `

With these changes, the compiler will compile shared_mutex/etc fine. (it appears CONFIG_PTHREAD_IPC should force CONFIG_POSIX_API to 1, but it doesn't)

Note that condition_variable (and i suspect other stuff) compiles but doesn't link.

This is because all of this requires the libstc++ be compiled with threading support (and the config change), and at least the ones in zephyr-sdk (and my NRF sdk) are compiled both single threaded and obviously without the gthr changes. The above changes really have to be made in the right toolchain configure bits/etc, and the toolchains changed to enable threads.

It's possible but a bad idea to do it without changing the threading model on the toolchains: I was a GCC/LLVM hacker for 10+ years, and saw it done, but it is a really bad idea: https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_concurrency.html#manual.intro.using.concurrency.thread_safety

agrees, saying: "multithreaded C++ application are only supported when libstdc++ and all user code was built with compilers which report (via gcc/g++ -v ) the same thread model and that model is not single."

dberlin commented 2 years ago

If you'd like to play around with a working mutex/condvar/shared_mutex, the following will let you do it. You have to set the POSIX options i mention in prj.cnf.

Then unzip stl_threaded.ZIP into the project root, and then add the following to CMakeLists.txt

FILE(GLOB stl_threaded_sources lib/stl_threaded/*.cpp lib/stl_threaded/*.c)
target_sources(app PRIVATE ${stl_threaded_sources})
target_include_directories(app BEFORE PUBLIC "lib/stl_threaded")

This will ensure the bits/c++config.h changes override the toolchain ones. The condition_var/mutex source is just the correspnding ones for the toolchain from the sdk-ng project.

I took a quick pass at hacking the toolchain to enable threads (IE CT_THREADS_POSIX=y) and change the os defines, but i'm still working on getting the environment to be the same as the CI workflow uses (I tried using act to execute it locally, and even if i revert back to pre-s3-download versions, still no luck).

dberlin commented 2 years ago

So, i got the toolchain to build normally (yay). Finally all my years of managing the crosstool team come in handy.

Unfortunately, enabling threads doesn't "just work", as might be expected. It gets pretty far but eventually fails trying to compile emutls with the final toolchain due to some errors.

Here's a preliminary checklist of work needed to make it work:

[ ] The crosstool configs need to be updated. The config commands y'all execute are actually warning you to run upgradeconfig on them. It took me a while to figure out why some of my options just disappeared into the ether, and this was one cause
[ ] crosstool assumes newlib implies no thread support. newlib.in needs to be modified to not select no thread support
[ ] gcc config array in the crosstool configs needs to have --enable-threads=posix
[ ] zephyr needs an OS define defined by GCC. Right now, ZEPHYR is defined on the compile command lines by cmake configs/etc. But this means you can't use it to differentiate zephyr during toolchain builds, which we will need to do to conditonalize some of the libstdc++ support. To make gcc add it should be easy, you just have to add a config file that defines cpp builtins for the os (see vxworks.h for an example), and add it to gcc's configure (again, see vxworks)
[ ] need to use zephyr pthread.h instead of the newlib one that gets generated (haven't looked into how to do this). It doesn't work at all to compile newlib without thread support and pass enable-threads=posix to the crosstool gcc config array.

Everything but the last one should be straightforward. I hand edited around the last one in the build directory (hand-disabled emutls), and it succeeds in compiling libstdc++ with thread support. So once that header is the right one, it should work.

Unfortunately, this is all the only way to get libstdc++ to compile the right files with threads enabled.

Hacking around all this would be possible (that's what the std::thread pull request does) but the libstdc++ internal impls change quite a bit from version to version, and y'all don't require specific toolchain/zephyr combinations. So probably gotta just fix the toolchains.

I'm unfortunately out of time for a bit to hack more on this, but hopefully this is helpful

cmorganBE commented 1 year ago

Is anyone actively working on std::mutex support? I'd be interested in assisting with that (assuming it would be a wrapper around some zephyr primitives) but I'd need some pointers.

dberlin commented 1 year ago

it doesn't need anything other than toolchain support and the right defines. It already works when you set up libstdc++ to use pthreads/etc, without any changes to libstdc++ itself.

On Wed, Dec 7, 2022 at 2:18 PM cmorganBE @.***> wrote:

Is anyone actively working on std::mutex support? I'd be interested in assisting with that (assuming it would be a wrapper around some zephyr primitives) but I'd need some pointers.

— Reply to this email directly, view it on GitHub https://github.com/zephyrproject-rtos/zephyr/issues/45785#issuecomment-1341471631, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACPI24WPHJLBGC5LR2GX4DWMDPGVANCNFSM5WK6URVA . You are receiving this because you commented.Message ID: @.***>

cmorganBE commented 1 year ago

@dberlin and pthreads maps through to zephyr threads? why not enabled at the moment?

dberlin commented 1 year ago

Yes. I gave the checklist of stuff that would have to be done to make it practically work above.

cfriedt commented 1 year ago

Actually, the only remaining feature to support std::thread and friends is automatic thread stack allocation, which is nearly finished in #44379. It's just missing mmu initialization.

I recently made some POSIX changes which add PTHREAD_MUTEX_INITIALIZER, and PTHREAD_COND_INITIALIZER.

52316 has some additional POSIX subsystem fixes. To make our POSIX implementation more compatible with that of Newlib / PicoLibC, but it's waiting on reviews.

cfriedt commented 1 year ago

Request: harmonize .clang-format for both C and C++

Rather than have a separate .clang-format for C and another one for C++ (if that was the strategy), it might be better to simply ensure that the one .clang-format that we have works for both C and C++.

The main reason is that no tooling (including clang-format itself, but also e.g. VS Code) exoects to have to deal with multiple .clang-format files.

zephyrproject-rtos / zephyr

C++ Roadmap #45785

Tasks

Resources

52316 has some additional POSIX subsystem fixes. To make our POSIX implementation more compatible with that of Newlib / PicoLibC, but it's waiting on reviews.