Implement a stub pthreads library for `THREAD_MODEL=single`
~~This patch series first starts with a number of commits stubbing out functions in the existing THREAD_model=posix code. According to "The Open Group Base Specifications Issue 7, 2018 edition", there are a number of mandatory functions which have not been provided. There are also some optional functions that have been partially provided in a not-useful way (e.g. get but no set function). For these, I have chosen to clean them up and remove the get functions for consistency.~~ EDIT: These have been split off into separate PRs and merged.
The remainder of the patches then build up a stub implementation of pthreads for THREAD_MODEL=single. I have done my best to try to make sure that all functions are as conforming as possible (under the assumption that another thread cannot ever be launched). This means that objects such as mutexes and rwlocks actually do update their state and will correctly fail when locks cannot be acquired.
When an inevitable deadlock occurs, I have chosen to return EDEADLK when it has been explicitly listed as a permissible return value, and to invoke __builtin_trap otherwise.
I have tested this by rebuilding libc++ with threads enabled and then smoke-testing Clang/LLVM-on-WASI to make sure that it can compile a simple program. I have not run any more-extensive conformance testing.
Fixes #501
does this break pthread detection mechanisms like cmake find_package(Threads)?
does this break pthread detection mechanisms like cmake find_package(Threads)?
I don't know. I don't use cmake beyond building other software that uses it.
Several of the comments in the linked issue are implying that, even if it might, not having these stubs is causing even more difficulty with porting software.
does this break pthread detection mechanisms like cmake find_package(Threads)?
I don't know. I don't use cmake beyond building other software that uses it.
Several of the comments in the linked issue are implying that, even if it might, not having these stubs is causing even more difficulty with porting software.
having a stub for mutex etc is probably fine.
otoh, i feel it isn't a great idea to provide pthread_create.
does this break pthread detection mechanisms like cmake find_package(Threads)?
I don't know. I don't use cmake beyond building other software that uses it. Several of the comments in the linked issue are implying that, even if it might, not having these stubs is causing even more difficulty with porting software.
having a stub for mutex etc is probably fine.
otoh, i feel it isn't a great idea to provide pthread_create.
Although there are also thoughts of potentially changing it, libcxx currently does require/expect pthread_create to exist. Incidentally, it performs detection whether or not pthreads is supported by looking for pthread_once and not pthread_create (although said detection doesn't actually seem to work properly for some reason, possibly because wasm handles undefined symbols by importing them from env rather than failing?)
I've rebased this PR and fixed the issues that CI uncovered. I do wish to merge this, but a decision needs to be made about how to handle the "pthreads detection in existing code" issue.
ping?
does this break pthread detection mechanisms like cmake find_package(Threads)?
I don't know. I don't use cmake beyond building other software that uses it. Several of the comments in the linked issue are implying that, even if it might, not having these stubs is causing even more difficulty with porting software.
having a stub for mutex etc is probably fine. otoh, i feel it isn't a great idea to provide pthread_create.
Although there are also thoughts of potentially changing it, libcxx currently does require/expect pthread_create to exist. Incidentally, it performs detection whether or not pthreads is supported by looking for
pthread_onceand notpthread_create(although said detection doesn't actually seem to work properly for some reason, possibly because wasm handles undefined symbols by importing them from env rather than failing?)
anyway, libcxx is not a representative application, i suspect.
can you explain why having a stub pthread_create is important for you? otherwise, can you consider to drop it?
The immediate reason is: I want to be able to build clang+LLVM for WASI (using @whitequark's patchset), which requires threading functions inside libcxx. The usage of types such as std::mutex is so pervasive throughout the LLVM codebase that it will probably never be acceptable to put compile-time conditionals around all of them. This is what started the desire for some kind of dummy threading stubs.
As far as I can tell, the only possible choices are:
- do what I've done here, where all of the stubs (i.e. enough to build libcxx with threads enabled) exist inside wasi-libc. Subsequently change wasi-sdk such that all builds have threads enabled inside libcxx. Use the resulting wasi-sdk to build clang+LLVM
- do choice 1 except introduce three threading models (i.e. "single", "stub", "posix"). This was discussed in a review comment with @sbc100, who didn't seem to see it necessary to do that.
- make it entirely libcxx's problem
- by making libcxx carry stubs for everything (synchronization and nonfunctional thread creation). I predict that this would result in simply having this exact same discussion in a brand-new GitHub thread.
- by making libcxx carry stubs for synchronization primitives only, still removing e.g.
std::thread. This would subsequently require hunting down all uses of these types inside the clang+LLVM codebase in order to build the final intended application, but this looks much more doable compared to trying to remove e.g.std::mutex. There does exist some desire to do this. However, this doesn't handle any C code that would need pthreads stubs, only C++ code. I am currently trying to avoid having to spend the effort required to make this option work.
- reject upstreaming a stub for
pthread_createhere (keeping only synchronization primitive stubs). This would mean that libcxx still wouldn't build.- Fix libcxx to support "synchronization primitives exist but
pthread_createdoesn't" as a platform. I don't know about the libcxx maintainers, but I personally would consider this very inelegant and clearly a hack, as such a "platform" would certainly not be POSIX-conformant. This can technically be done by simply conditionally guarding one line of code inside libcxx in order to end up with a libcxx wherestd::threadexists but always fails, but once again "I predict that this would result in simply having this exact same discussion in a brand-new GitHub thread." Alternatively, this can be done by introducing the ability for libcxx to havestd::threadetc. deleted butstd::mutexetc. present. This would be just as difficult as doing option 3.ii. - Do option 3.i or 3.ii anyways. This works but results in twice the amount of stubs existing.
- Some downstream user creates a stub for
pthread_createanyways and subsequently rebuilds libcxx against it. This is strictly worse as you're now doing the exact same thing except for each downstream user.
- Fix libcxx to support "synchronization primitives exist but
The tl;dr is "I need libcxx. I am trying to avoid having to put in all of the effort needed to enable a new 'libcxx has synchronization primitives but no thread creation' build mode for libcxx. I also generally don't think omitting pthread_create is a particularly good idea."
In general, the code I want to get ported to WASI would look something like
// in do_work_{foo|bar|baz|qux}.c
if (use_multiple_threads) {
pthread_create(...);
} else {
do_work_synchronously();
}
// in main.c
if (check_command_line_flags("-threads"))
use_multiple_threads = 1;
With full pthreads stubs in wasi-libc, this code can either be ported by doing nothing ("just" don't pass the -threads command line argument) or by having a single #ifdef __WASI__ in main.c, instead of multiple #ifdefs sprinkled throughout all of the do_work_*.c parts of the program.
Can you perhaps elaborate further as to why you oppose having pthread_create?
(Thanks for doing this work, @ArcaneNibble)
The immediate reason is: I want to be able to build clang+LLVM for WASI (using @whitequark's patchset), which requires threading functions inside libcxx. The usage of types such as
std::mutexis so pervasive throughout the LLVM codebase that it will probably never be acceptable to put compile-time conditionals around all of them. This is what started the desire for some kind of dummy threading stubs.
how about having libwasi-emulated-pthread.a to follow the precedents?
Can you perhaps elaborate further as to why you oppose having
pthread_create?
i don't want to break pthread-detection mechanisms around there, including cmake FindThreads.
how about having libwasi-emulated-pthread.a to follow the precedents?
Oh! I quite like this idea! I would still want libcxx to be built with threading support enabled for both single and posix though, and I hope to avoid having to hack up its build system to do something special.
i don't want to break pthread-detection mechanisms around there, including cmake FindThreads.
The deliberate intention was to cause detection mechanisms to now detect "this platform has pthreads." This is why I implemented all of the stub functions s.t. they comply with the POSIX specifications (rather than just returning 0 like some other examples that have been posted in #501 ). EAGAIN is explicitly listed as a permissible way for pthread_create to fail, so software should be expected to do something in response.
Can we get community consensus on the approach of "put single-threaded pthreads stubs into libwasi-emulated-pthread.a but always build libcxx with threading enabled (thus causing a program to have undefined symbols if it uses libcxx threading but doesn't link the .a file)" before I go ahead and implement it?
I would still want libcxx to be built with threading support enabled for both single and posix though
do you mean to make wasi-sdk ship libcxx built with LIBCXX_ENABLE_THREADS=ON?
why?
do you mean to make wasi-sdk ship libcxx built with LIBCXX_ENABLE_THREADS=ON?
why?
Yes, because I specifically want to easily port software which uses C++11 threading. i.e. the exact same reason why I am trying to enable pthreads stubs
@yamt I feel like we are repeatedly talking past each other. Is there any way we can jump into a real-time discussion (e.g. IRC)?
ping, is there some way through which we can arrive at community consensus about how this pthreads stub functionality should be named and exposed?
As there have been multiple people in support of something like this, I have:
- moved the stub files to a new top-level directory (not inside libc-top-half anymore)
- linked the synchronization primitives into libc
- put pthread_create, pthread_detach, pthread_join into libwasi-emulated-pthread.a
Or were there some real needs for this in your Clang use case?
Clang does actually need the initialization and destruction of primitives to succeed, this is not just an arbitrary choice. Overall, I like @ArcaneNibble's chosen strategy, which is to produce a conformant single-threaded implementation that traps on deadlock instead of halting.
Oh, one last comment: @ArcaneNibble, as I reviewed this, I noticed that the recent #522 modifies the expected files so we can't merge this as-is. Can you rebase or merge that in?
Rebased