Use MUSL's weak* feature in bottom half
#306
Merged
+12
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
abrown
commented
Jul 25, 2022
| @@ -70,7 +70,7 @@ int __wasilibc_find_relpath_alloc( | |||
| char **relative, | |||
| size_t *relative_len, | |||
| int can_realloc | |||
| ) __attribute__((weak)); | |||
| ) __attribute__((__weak__)); | |||
There was a problem hiding this comment.
Here is a place that I could have used just weak but when I do the build fails:
#
# Test that it compiles.
#
clang -O2 -DNDEBUG --target=wasm32-wasi -fno-trapping-math -Wall -Wextra -Werror -Wno-null-pointer-arithmetic -Wno-unused-parameter -Wno-sign-compare -Wno-unused-variable -Wno-unused-function -Wno-ignored-attributes -Wno-missing-braces -Wno-ignored-pragmas -Wno-unused-but-set-variable -Wno-unknown-warning-option -mthread-model single -isystem "/home/abrown/Code/wasi-libc/sysroot/include" -fsyntax-only "/home/abrown/Code/wasi-libc/sysroot/share/wasm32-wasi/include-all.c" -Wno-\#warnings
In file included from /home/abrown/Code/wasi-libc/sysroot/share/wasm32-wasi/include-all.c:166:
/home/abrown/Code/wasi-libc/sysroot/include/wasi/libc-find-relpath.h:73:3: error: expected function body after function declarator
) weak;
^
1 error generated.
I guess there is some other place to add an include path if we want to use weak everywhere. (Not sure I'm fully convinced that weak is better than __attribute((__weak__)), though... Just trying to be consistent.)
There was a problem hiding this comment.
Adding #include <features.h> to this file should do it.
There was a problem hiding this comment.
That seemed like the right thing to do so I tried it but it blew up with the same error...
|
LGTM! |
sbc100
reviewed
Jul 26, 2022
| @@ -11,7 +11,7 @@ | |||
| /// Statically-initialize it to an invalid pointer value so that we can | |||
| /// detect if it's been explicitly initialized (we can't use `NULL` because | |||
| /// `clearenv` sets it to NULL. | |||
| char **__wasilibc_environ __attribute__((weak)) = (char **)-1; | |||
| char **__wasilibc_environ weak = (char **)-1; | |||
There was a problem hiding this comment.
I think this reads way easier with weak at the start of the line.. and I think that seems to be how musl uses it.
| @@ -87,7 +87,7 @@ void __wasilibc_deinitialize_environ(void) { | |||
| } | |||
|
|
|||
| // See the comments in libc-environ.h. | |||
| __attribute__((weak)) | |||
| weak | |||
| void __wasilibc_maybe_reinitialize_environ_eagerly(void) { | |||
abrown
added a commit
to abrown/wasi-libc
that referenced
this pull request
Jul 26, 2022
This is a follow-up based on @sbc100's comments in WebAssembly#306.
sbc100
pushed a commit
that referenced
this pull request
Jul 26, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.,
LOCKmade this worthwhile.Beyond just changing uses of
weakto__weak__, we also MUSL'sweakand
weak_aliasmacros in a few more places.