[WIP] Fixes for the THREAD_MODEL=posix build #303
Conversation
sbc100
force-pushed
the
fix_pthread_build
branch
from
f44f89c to
7af7dda
Compare
|
Still need to deal with preopens.c |
| int r = __wasi_random_get(buffer, len); | ||
| pthread_setcancelstate(cs, 0); |
There was a problem hiding this comment.
I propose that we leave out considerations of cancellation out for now. Pthread cancellation is something of an anti-sweet-spot, being particularly difficult and expensive to implement, while being particularly rarely used.
One of the particular complexities is what to do if one thread tries to cancel another thread that's currently executing host code. That might require all host code to be pthread cancel-safe, which would be very difficult for many popular host implementations today.
There was a problem hiding this comment.
Sounds reasonable, at least for a first pass. How about we just avoid using pthread_setcancelstate et al in the bottom half and just stub them out (make them no-ops) when compiling top-half code.
| #if _REENTRANT | ||
| int val; | ||
| __asm__(".globaltype g_needs_dynamic_alloc, i32\n" | ||
| "global.get __wasi_libc_pthread_self\n" |
There was a problem hiding this comment.
Where are g_needs_dynamic_alloc and __wasi_libc_pthread_self defined?
As a minor style nit, wasi-libc usually uses identifiers named __wasilibc_* rather than __wasi_libc_*.
| @@ -5,6 +5,7 @@ | |||
| #include <errno.h> | |||
| #include <threads.h> | |||
| #include <time.h> | |||
| #include <_/cdefs.h> // for __strong_reference | |||
There was a problem hiding this comment.
Instead of adding this include, let's just delete the thrd_sleep alias here. If we want thrd_sleep, it's better to use the musl definition, which correctly implements the C11 behavior.
| if (path[0] == 0 || !strcmp(path, ".") || !strcmp(path, "./")) { | ||
| return make_absolute_buf; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Could you add a comment about what this block of code does?
| "local.set %0" : "=r" (val)); | ||
| return val; | ||
| #else | ||
| return 0; |
There was a problem hiding this comment.
Does __get_tp need to be defined in non-_REENTRANT mode? It seems better to get an undefined-symbol link error if we try to use it than to have it silently return NULL, especially with NULL being a valid address.
There was a problem hiding this comment.
Thanks for all the comments. I will address all of them, this PR is certainly not ready to land. I just wanted to share what I had done so far.
There was a problem hiding this comment.
Yep, makes sense. I was just looking through it so that I could follow along, and happened to have comments along the way :-).
This change extracts the `weak*`-related parts of WebAssembly#303 as a separate PR. Note that this is slightly strange in that it uses some top-half MUSL headers in the bottom-half code, but discussion around this led me to believe that the advantages of, e.g., `LOCK` made this worthwhile. Beyond just changing uses of `weak` to `__weak__`, we also MUSL's `weak` and `weak_alias` macros in a few more places.
These changes get past several small build errors in WebAssembly#303.
This change extracts the `weak*`-related parts of #303 as a separate PR. Note that this is slightly strange in that it uses some top-half MUSL headers in the bottom-half code, but discussion around this led me to believe that the advantages of, e.g., `LOCK` made this worthwhile. Beyond just changing uses of `weak` to `__weak__`, we also MUSL's `weak` and `weak_alias` macros in a few more places.
|
Closing in favor of #311 |
No description provided.