-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Change Wasm's cdylib
etc. to be a "reactor".
#108097
base: master
Are you sure you want to change the base?
Conversation
Use `--entry` `_initialize` on `cdylib` and similar library types on Wasm targets. This is a change in how constructor functions have at least sometimes been handled. It will mean that there's an exported `_initialize` function which calls the static constructors, and it must be called from the outside before any other export is accessed. Rust doesn't officially have static constructors, but C++ does and C has not-uncommonly-used extensions that do, and there are crates in Rust that provide static-constructor functionality. Some Wasm embeddings automatically call the `_initialize` function if present because it is part of the [WASI Preview1 application ABI], though not all do. This does not implement support for dynamic linking. The format and behavior of `cdylib` and similar outputs may change in future compiler versions, especially as Wasm gains a stable dynamic linking format. What this does do, is make `cdylib` work more like what many people have been assuming `cylib` does on Wasm. It produces a Wasm module that doesn't have a main function that you can instantiate and call exports on, but which now also supports static constructors by having an `_initialize` function that you or your engine must call first. [WASI Preview1 application ABI]: https://github.com/WebAssembly/WASI/blob/main/legacy/application-abi.md
(rustbot has picked a reviewer for you, use r? to override) |
These commits modify compiler targets. |
I have mixed feelings about this. On one hand, always having an An alternative is this patch to wasm-ld which auto-wraps all reactor exports with code to check whether static constructors have been called and automatically call them the first time any export is called. That would obviate the But on the other other other hand, the Wasm component model may add a second initialization phase, which would be specifically for supporting constructors, which suggests that we should stick with an |
In the past we have said that Rust does not have "life before main", and that while those crates certainly exist, they essentially rely on using features of the platform beyond our control, or using purely compile-time trickery. I am concerned that this could be seen as "welp, life-before-main exists when we remove all the factors we can control for in the language, so add life-before-main to the language." That seems like a badly-reasoned precedent. While we could shrug and say "platform!" again, this seems to be a much more deliberate choice we are making about how we handle the code we emit. I am disinclined to write it off, especially because the mere presence of wasm32-unknown-unknown as a target has had severe impacts on our ability to reason about our floating point semantics because of choices by the WebAssembly working groups that do not align with the IEEE754-2008 rules. I am also concerned at the "zero-cost abstraction" level: making every Rust program pay for so-called "static" constructors because they hypothetically exist does not seem like a great status quo. |
Mind, I am aware that the current wasm/wasi model requires something to happen here as a matter of the "process" ABI. I am just concerned this might be the wrong thing. People are trying to depend in significant ways on the current wasm/wasi semantics, even when those are deliberately underspecified. To some extent, this is being added to satisfy those programmers who want to depend on something. With this much of the semantics up in the air, I don't feel like we can rely on such a second initialization phase being formally added. It seems at significant risk of letting people down a year out from now if people decide certain things were bad ideas. |
I know this isn't the topic of the PR itself, but: I've tried to follow the relevant threads about this but have not previously seen this conclusion. Do you have links to more information? |
There's been a bunch of discussion around the wasm targets and how these probably should be considered even more of a "sneak preview" than they are, and how they will likely be changed. A lot. Soon. I'm wondering if this PR handles #108381? If so then I think, while my concerns remain, they should probably be tabled, as they will likely be addressed in the future and/or we will have more room to discuss alternatives very soon. |
r? compiler |
I have no idea what this PR is about. |
@sunfishcode I'd like to help see this get to some resolution, but my time is very limited this month. I'm not sure if we're going to get someone from T-compiler who can dedicate time to this in the nearer term. It seems clear that there are issues here that are up for debate. Does this deserve a design meeting? or, if not that, then a dedicated zulip stream to discuss the matter? (Maybe there already is such a zulip stream?) |
My interpretation of #108097 (comment) and perhaps also the reluctance of anyone to comment on this is that Rust doesn't want to be the one telling uses that they need to make sure to call Consequently, I've started WebAssembly/tool-conventions#203 to seek to establish this convention at a WebAssembly level, and how now posted a CG meeting agenda for next week's meeting in WebAssembly/meetings#1253. I don't know if the |
I'll label this as S-blocked to try to signal that it depends on work outside of T-compiler (iiuc the above comment) @rustbot label +S-blocked -S-waiting-on-review |
☔ The latest upstream changes (presumably #127216) made this pull request unmergeable. Please resolve the merge conflicts. |
@sunfishcode any updates on this? I assume it's not blocked since the pr it was blocked on is now merged, but not sure there's more work this is blocked on or not. (if it's not blocked you can resolve the conflicts). Thanks |
There were no objections in the CG meeting, and yes, the PR WebAssembly/tool-conventions#203 is now merged, so BasicModuleABI.md is now a tool convention saying that if a module exports "_initialize" and not "_start", then "_initialize" is to be called before other exports. And this convention is now supported by at least some tools, such as wasm-tools component. The main open questions here are: First, is something like this still needed in practice? And second, would it break people's existing setups, and if so what should we do about it? |
Use
--entry
_initialize
oncdylib
and similar library types on Wasm targets.This is a change in how constructor functions have at least sometimes been handled. It will mean that there's an exported
_initialize
function which calls the static constructors, and it must be called from the outside before any other export is accessed. Rust doesn't officially have static constructors, but C++ does and C has not-uncommonly-used extensions that do, and there are crates in Rust that provide static-constructor functionality.Some Wasm embeddings automatically call the
_initialize
function if present because it is part of the WASI Preview1 application ABI, though not all do.This does not implement support for dynamic linking. The format and behavior of
cdylib
and similar outputs may change in future compiler versions, especially as Wasm gains a stable dynamic linking format.What this does do, is make
cdylib
work more like what many people have been assumingcylib
does on Wasm. It produces a Wasm module that doesn't have a main function that you can instantiate and call exports on, but which now also supports static constructors by having an_initialize
function that you or your engine must call first.