Enable threading in libcxx in all cases

[ArcaneNibble]

This has not been tested locally! I don't have enough disk space to store yet another LLVM tree, and I couldn't quite figure out how to test only the sysroot changes, so I'm hoping the CI pipeline can test this

This enables threads in libcxx for all builds, building on top of the stubs for libpthread

[alexcrichton]

I kicked off the CI builds and it looks like they're failing with:

2024-10-30T15:12:55.2446007Z [1249/1301] Linking CXX shared library lib/libc++.so�[K
2024-10-30T15:12:55.2447078Z [1250/1301] Linking CXX shared library lib/libc++.so�[K
2024-10-30T15:12:55.2447920Z �[31mFAILED: �[0mlib/libc++.so 
2024-10-30T15:12:55.2485184Z : && /src/build/install/bin/clang++ --sysroot=/src/build/sysroot/install/share/wasi-sysroot -fPIC -D_WASI_EMULATED_PTHREAD --target=wasm32-wasi -fdebug-prefix-map=/src=wasisdk://v24.13g1e06ec8b881e --sysroot /src/build/sysroot/install/share/wasi-sysroot -resource-dir /src/build/sysroot/install/wasi-resource-dir -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O2 -g -DNDEBUG  -Wl,-z,defs -Wl,--color-diagnostics -shared  -o lib/libc++.so libcxx/src/CMakeFiles/cxx_shared.dir/algorithm.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/any.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/bind.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/call_once.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/charconv.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/chrono.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/error_category.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/exception.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/expected.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/filesystem_clock.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/filesystem_error.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/path.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/functional.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/hash.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/legacy_pointer_safety.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/memory.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/memory_resource.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/new_handler.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/new_helpers.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/optional.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/print.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/random_shuffle.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/ryu/d2fixed.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/ryu/d2s.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/ryu/f2s.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/stdexcept.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/string.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/system_error.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/typeinfo.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/valarray.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/variant.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/vector.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/verbose_abort.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/barrier.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/condition_variable_destructor.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/condition_variable.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/future.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/mutex_destructor.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/mutex.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/shared_mutex.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/thread.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/random.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/fstream.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/ios.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/ios.instantiations.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/iostream.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/locale.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/ostream.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/regex.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/strstream.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/directory_entry.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/directory_iterator.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/operations.cpp.o libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/int128_builtins.cpp.o  -nostdlib++  lib/libc++abi.so  -lpthread  -lc && :
2024-10-30T15:12:55.2518305Z wasm-ld: �[0;35mwarning: �[0munknown -z value: defs
2024-10-30T15:12:55.2519505Z wasm-ld: �[0;35mwarning: �[0mcreating shared libraries, with -shared, is not yet stable
2024-10-30T15:12:55.2521231Z wasm-ld: �[0;31merror: �[0mlibcxx/src/CMakeFiles/cxx_shared.dir/thread.cpp.o: undefined symbol: pthread_join
2024-10-30T15:12:55.2523110Z wasm-ld: �[0;31merror: �[0mlibcxx/src/CMakeFiles/cxx_shared.dir/thread.cpp.o: undefined symbol: pthread_detach
2024-10-30T15:12:55.2524881Z clang++: �[0;1;31merror: �[0m�[1mlinker command failed with exit code 1 (use -v to see invocation)�[0m
2024-10-30T15:12:55.3378068Z 

I think the failure is due to the fact that -lwasi-emulated-pthread isn't passed. With the -D flag passed the -l flag also needs to be passed to link in the dummy libraries. I'm not sure how best to manage that in the build here since if libcxx is built with the -D flag then it will always require -lwasi-emulated-pthread which probably isn't what users of libcxx or C++ in general expect.

[sunfishcode]

Could you do some measurements of the code size impact of this change? For a basic C++ program that doesn't use threads, how much code size difference is there between threading disabled in the libcxx build vs. threading stubbed out in libc?

[ArcaneNibble]

As far as I understand it, wasi-emulated-pthread should only be needed if the program actually ends up referencing pthread_{create|detach|join}. However, this wouldn't work if you're building a .so (TIL that that was even possible on wasm), and I'm also not sure how to deal with it either.

[ArcaneNibble]

I just compared the size difference between a C++ std::cout hello world using <some random wasi sysroot I happened to have lying around on my computer, that should've hopefully come from an official release> against <a different random sysroot that definitely contains both the threading stubs in wasi-libc as well as a manually-recompiled version of libc++ with threading enabled> (these are not built from the same codebase, and they definitely weren't built by the same version of llvm either) and

the version with stubs and threading in libc++ is actually smaller (223K --> 183K)

this is not at all a fair comparison, but the real different is probably minor?

[whitequark]

However, this wouldn't work if you're building a .so (TIL that that was even possible on wasm), and I'm also not sure how to deal with it either.

Would it not work? A .so can reference symbols from the main binary on ELF platforms, so provided it references an undefined pthread_create, and Wasm shared objects behave more or less like ELF ones, that should work.

[ArcaneNibble]

Hmm, you are indeed correct. I forgot that you can do that.

I'm still not sure how to handle documentation and user expectations for these issues though (especially since linkage seems to often be poorly understood).

[whitequark]

I'm still not sure how to handle documentation and user expectations for these issues though (especially since linkage seems to often be poorly understood).

I think we can say that you have to link the main app with -lwasi-emulated-pthread?

[ArcaneNibble]

@alexcrichton stated above that requiring users to explicitly pass -lwasi-emulated-pthread probably isn't what users expect.

However, I was just reminded of how glibc a long time ago would require passing -ldl -lrt -lpthread etc , so requiring something like this is not unprecedented. Perhaps this is acceptable with "enough" documentation?

[whitequark]

stated above that requiring users to explicitly pass -lwasi-emulated-pthread probably isn't what users expect.

We could also change the clang compiler driver to do so.

[alexcrichton]

Oh definitely don't take my off-hand thought as a given or anything like that. We're already surprising lots of folks by not having pthread apis at all, and in general folks expect pthread apis to be available and do something. I don't think it would be great if -lwasi-emulated-... was basically required for C++ code but it's not practically that much worse than today.

(regardless though this won't be able to land unless CI is passing, so something would need to be done about that)

[sbc100]

stated above that requiring users to explicitly pass -lwasi-emulated-pthread probably isn't what users expect.

We could also change the clang compiler driver to do so.

I think the idea with these wasi-emulated-xxx libraries is that users have to explicitly opt into them. So having them enabled by default kind of defeats the purpose.

[whitequark]

I think the idea with these wasi-emulated-xxx libraries is that users have to explicitly opt into them. So having them enabled by default kind of defeats the purpose.

To be honest I don't actually understand the problem here. @alexcrichton says that the library should not be explicitly opted into while @sbc100 says it should be (which is also how I understood this should work). What is the UX design goal here? I'm lost.

[alexcrichton]

Sorry I can try to explain more. Right now wasi-libc has a few features which are not supported by default and are opt-in. The headers using them have a #error which request you pass -DWASI_EMULATED_... and link with -lwasi-emulated-.... This system does not work transitively, however. The problem here is that libcxx is shipped in a precompiled form and doesn't have the with/without emulated bits split. This PR unconditionally enables usage of pthread APIs in libcxx, meaning (if I understand correctly) that usage of std::mutex is always there but it always uses the pthread APIs.

This means that users of libcxx who did not opt-in with -DWASI_EMULATED_... are going to be required to pass -lwasi-emulated-.... They won't even realize it until link errors happen as there's no #error to guide them there. This basically means that the opt-in design pattern of wasi-libc itself isn't working for those depending on wasi-libc, or more work would need to be done upstream in libcxx to get this split done (which I suspect probably isn't too feasible).

That's at least my understanding of the situation, but I'm no C++ expert, so I could very well be off-the-mark here.

[whitequark]

So if I understand it right, there's a #define of some kind in libcxx that enables or disables APIs like std::mutex. As shipped right now, this define is disabled, so you can't use std::mutex (it errors out). With this PR, it will be enabled, and when used a link time reference to pthread symbols will appear.

Can we make that #define conditional on threading model being "multi" or emulated pthreads being enabled? Then if it is missing, the object files for std::mutex etc will not be included and so there won't be a linker error.

[alexcrichton]

That's my understanding, yeah. Libcxx either does or doesn't use pthreads and that's a build-time-of-libcxx decision, not a build-time-when-you-pull-in-libcxx decision. Whether or not what you're proposing is viable, having a build-time-when-you-pull-in-libcxx #define, I'm not sure, but I suspect it's not supported today.

[alexcrichton]

To put forth an idea which I do know will work, but I realize may be controversial: merge "emulated pthreads" directly into wasi-libc. Don't require -lwasi-emulated-pthread or -DWASI_EMULATED_PTHREAD and instead unconditionally provide the symbols with -lc and always provide <pthread.h>. That would solve this distribution problem because then libcxx could always have std::mutex and the symbols are always there.

[sbc100]

To put forth an idea which I do know will work, but I realize may be controversial: merge "emulated pthreads" directly into wasi-libc. Don't require -lwasi-emulated-pthread or -DWASI_EMULATED_PTHREAD and instead unconditionally provide the symbols with -lc and always provide <pthread.h>. That would solve this distribution problem because then libcxx could always have std::mutex and the symbols are always there.

Sure, but that argument could also be made for all the emulated stuff. The simplest solution in all cases is just to include emulation for everything and then more things "just work". However, that would go again the current design of wasi-libc which is not to include things that are fake/emulated unless they are opt-ed into explicitly.

Another possible downside: If we enable threading in libc++ will that cause it be slower/bigger/bloated compared to the no-threads version? I.e. will be be redundantly locking mutexed etc when it doesn't need to? I guess LTO should take care of more of that by not all.

[whitequark]

Okay, let me try to approach this from the other end.

1. Is it desired by wasi-sdk maintainers to be able to build (for example; not exclusively) upstream LLVM that is runnable on non-multithreaded runtimes?
2. If the answer is yes, what are the hard UX constraints within which the solution must be found?

[sbc100]

Okay, let me try to approach this from the other end.

1. Is it desired by wasi-sdk maintainers to be able to build (for example; not exclusively) upstream LLVM that is runnable on non-multithreaded runtimes?

Sure. Multi-threaded runtimes are not really a thing since wasi-threads is hold for now (pending shared-everything threads). So single-threaded runtimes are the primary target of wasi-libc and wasi-sdk.

The question is, how much software can be built for a target that doesn't support threads? For example, I don't know if LLVM can be built without threading support since I don't know of any other platforms it targets that don'e have thread. So, it may need the emulation in order to build at all. But its seems reasonable in that case for anyone building such software to opt into the fake/emulated threading support doesn't it?

[whitequark]

The question is, how much software can be built for a target that doesn't support threads? For example, I don't know if LLVM can be built without threading support since I don't know of any other platforms it targets that don'e have thread.

LLVM can be built without threading support (it has an -DLLVM_ENABLE_THREADS switch) but it expects std::mutex etc to still be available even with -DLLVM_ENABLE_THREADS=OFF. I have proposed adding #ifdefs around them and this was rejected by the LLVM community because of the ongoing maintenance cost, therefore either wasi-sdk provides a way to get std::mutex on single-threaded platforms or upstream LLVM can't be built on upstream wasi-sdk.

(I should note that I have a working port of LLVM to WASI and the PRs mentioned above directly relate to it.)

Most software that uses threads as a way to optionally improve the performance of some operation through parallelism, in my experience, has a way to bypass the spawning of additional threads. As another example of something I ported to WASI, nextpnr also hit this problem. In this case, I had organizational influence that helped me add the required #ifdefs, but if I did not have to do it the port would have been easier and lower maintenance.

But its seems reasonable in that case for anyone building such software to opt into the fake/emulated threading support doesn't it?

From my perspective, the desired end result is that the opt-in is done at the build system level (./configure or cmake option), and doesn't require patching either wasi-sdk or the software itself.

---

Does this help clear things up a bit?

[sbc100]

From my perspective, the desired end result is that the opt-in is done at the build system level (./configure or cmake option), and doesn't require patching either wasi-sdk or the software itself.

Surely adding link flags such as -lc++-mt -lwasi-emulated-pthread when building for WASI is reasonable isn't it? Doesn't LLVM already have lots of per-platform link and compiler flags?

[whitequark]

Surely adding link flags such as -lc++-mt -lwasi-emulated-pthread when building for WASI is reasonable isn't it? Doesn't LLVM already have lots of per-platform link and compiler flags?

Completely reasonable, albeit since the C++ compiler frontend will add -lc++ implicitly there would have to be a carve-out for that.