-
Notifications
You must be signed in to change notification settings - Fork 228
wasip2: Improve "command" startup flow #710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Favor `wasm-tools` on the system | ||
| find_program(WASM_TOOLS_EXECUTABLE NAMES wasm-tools) | ||
| if (NOT WASM_TOOLS_EXECUTABLE) | ||
| include(ba-download) | ||
| ba_download( | ||
| wasm-tools | ||
| "https://github.com/bytecodealliance/wasm-tools" | ||
| "1.244.0" | ||
| ) | ||
| ExternalProject_Get_Property(wasm-tools SOURCE_DIR) | ||
| set(wasm_tools "${SOURCE_DIR}/wasm-tools") | ||
| else() | ||
| add_custom_target(wasm-tools) | ||
| set(wasm_tools ${WASM_TOOLS_EXECUTABLE}) | ||
| endif() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,54 +9,64 @@ extern void __wasm_call_ctors(void); | |
| extern int __main_void(void); | ||
| extern void __wasm_call_dtors(void); | ||
|
|
||
| __attribute__((export_name("_start"))) | ||
| void _start(void) { | ||
| // Commands should only be called once per instance. This simple check | ||
| // ensures that the `_start` function isn't started more than once. | ||
| // | ||
| // We use `volatile` here to prevent the store to `started` from being | ||
| // sunk past any subsequent code, and to prevent any compiler from | ||
| // optimizing based on the knowledge that `_start` is the program | ||
| // entrypoint. | ||
| #if defined(__wasip1__) | ||
| __attribute__((export_name("_start"))) void _start(void) | ||
| #elif defined(__wasip2__) | ||
| // Note that this is manually doing what `wit-bindgen` might otherwise be | ||
| // doing. Given the special nature of this symbol this skip the typical | ||
| // `wit-bindgen` rigamarole and the signature of this function is simple enough | ||
| // that this shouldn't be too problematic (in theory). | ||
| __attribute__((export_name("wasi:cli/run@0.2.0#run"))) int _start(void) | ||
| #elif defined(__wasip3__) | ||
| __attribute__((export_name("wasi:cli/run@0.3.0-rc-2025-09-16#run"))) int | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct yeah, this is intentionally a sync lift into an async function type which the runtime will handle with stack switching and such. While reactor-style programs will be able to work with callbacks I don't think there's any route for us to transform |
||
| _start(void) | ||
| #else | ||
| #error "Unsupported WASI version" | ||
| #endif | ||
| { | ||
| // Commands should only be called once per instance. This simple check | ||
| // ensures that the `_start` function isn't started more than once. | ||
| // | ||
| // We use `volatile` here to prevent the store to `started` from being | ||
| // sunk past any subsequent code, and to prevent any compiler from | ||
| // optimizing based on the knowledge that `_start` is the program | ||
| // entrypoint. | ||
| #ifdef _REENTRANT | ||
| static volatile _Atomic int started = 0; | ||
| int expected = 0; | ||
| if (!atomic_compare_exchange_strong(&started, &expected, 1)) { | ||
| __builtin_trap(); | ||
| } | ||
| static volatile _Atomic int started = 0; | ||
| int expected = 0; | ||
| if (!atomic_compare_exchange_strong(&started, &expected, 1)) { | ||
| __builtin_trap(); | ||
| } | ||
| #else | ||
| static volatile int started = 0; | ||
| if (started != 0) { | ||
| __builtin_trap(); | ||
| } | ||
| started = 1; | ||
| static volatile int started = 0; | ||
| if (started != 0) { | ||
| __builtin_trap(); | ||
| } | ||
| started = 1; | ||
| #endif | ||
|
|
||
| __wasi_init_tp(); | ||
| __wasi_init_tp(); | ||
|
|
||
| // The linker synthesizes this to call constructors. | ||
| __wasm_call_ctors(); | ||
| // The linker synthesizes this to call constructors. | ||
| __wasm_call_ctors(); | ||
|
|
||
| // Call `__main_void` which will either be the application's zero-argument | ||
| // `__main_void` function or a libc routine which obtains the command-line | ||
| // arguments and calls `__main_argv_argc`. | ||
| int r = __main_void(); | ||
| // Call `__main_void` which will either be the application's zero-argument | ||
| // `__main_void` function or a libc routine which obtains the command-line | ||
| // arguments and calls `__main_argv_argc`. | ||
| int r = __main_void(); | ||
|
|
||
| // Call atexit functions, destructors, stdio cleanup, etc. | ||
| __wasm_call_dtors(); | ||
| // Call atexit functions, destructors, stdio cleanup, etc. | ||
| __wasm_call_dtors(); | ||
|
|
||
| // If main exited successfully, just return, otherwise call | ||
| // `__wasi_proc_exit`. | ||
| // If main exited successfully, just return, otherwise call | ||
| // `__wasi_proc_exit`. | ||
| #if defined(__wasip1__) | ||
| if (r != 0) { | ||
| __wasi_proc_exit(r); | ||
| } | ||
| if (r != 0) { | ||
| __wasi_proc_exit(r); | ||
| } | ||
| #elif defined(__wasip2__) || defined(__wasip3__) | ||
| if (r != 0) { | ||
| exit_result_void_void_t status = { .is_err = true }; | ||
| exit_exit(&status); | ||
| } | ||
| return r != 0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With a target-specific entrypoint signature I was able to update to directly return the |
||
| #else | ||
| # error "Unsupported WASI version" | ||
| #error "Unsupported WASI version" | ||
| #endif | ||
| } | ||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be hard to add an assembly syntax for this so that clang/llvm can produce this object file without needing external tools?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I poked at that a bit, but was unfruitful in my ability to get it working. That being said you probably know whether this is possible more than I, so question for you!
The need here is that a custom section needs to be located inside of
crt1-command.o(wasm custom section, not linking custom section). Thiswasm-toolscommand will embed the data within the object with a particular section name. We can pretty easily generate the data to embed ahead-of-time (similar to howwit-bindgen-generated bindings are checked in to this repo). Effectively what I want is something like:or... something like that. I couldn't figure out either
.custom_sectionfor__asm__,#embed, nor how to emit awasm.custom_sectionin Clang via a C global or something like that. Do you know if these are all possible to combine?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks like something that it should be possible to support with the asm syntax yes. Assuming that
#embedplays nice with__asm__in general (i.e. on other platforms) i don't see why it shouldn't for us.BTW, In this case are you actually embedding a wasm file as a custom section in another wasm file (i.e. the object file)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my problem is I can't actually figure out a syntax/incantation that works. I suspect something like this should work, but I'm not enough of a clang wizard to figure it out.
And yeah, the type information for components is itself a component (a wasm file) which is embedded in objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to push on this harder. I don't know how to get
__asm__to work with#embed. I triedwasm-ld --relocatableto link two objects together but that seems to lose__attribute__((export_name("...")))directives on symbols. Another option is some sort of preprocessing similar-ish toxxd -i, but overall I'm not sure that anything else will be all that much better than runningwasm-tools component embedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could leave a comment there suggesting that a simpler solution might be possible with inline asm.
In the long run I do like the idea striving for "its just clang" as much as possible in wasi-sdk, but for the users and for the developers. I know that was one of the original goals when we first started wasi-sdk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed yeah, comment added!