-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
WebAssembly: remove unconditional --allow-undefined flag #14102
Conversation
In ziglang#1622, when targeting WebAsembly, the --allow-undefined flag became unconditionally added to the linker. This is not always desirable. First, this is error prone. Code with references to unkown symbols will link just fine, but then fail at run-time. This behavior is inconsistent with all other targets. For freestanding wasm applications, and applications that only use WASI, undefined references are better reported at compile-time. This behavior is also inconsistent with clang itself. Autoconf and cmake scripts checking for function presence think that all tested functions exist, but then resulting application cannot run. For example, this is one of the reasons compilation of Ruby 3.2.0 to WASI fails with zig cc, while it works out of the box with clang. But all applications checking for symbol existence before compilation are affected. This reverts the behavior to the one Zig had before ziglang#1622, and introduces an `import_symbols` flag to ignore undefined symbols, assuming that the webassembly runtime will define them.
No longer automatically append the `--export` flag for each exported function unconditionally. This was essentially a hack to prevent binary bloat caused by compiler-rt symbols being always included in the final binary as they were exported and therefore not garbage- collected. This is no longer needed as we now support the ability to set the visibility of exports. This essentially reverts 6d951af
When we're compiling compiler_rt for any WebAssembly target, we do not want to expose all the compiler-rt functions to the host runtime. By setting the visibility of all exports to `hidden`, we allow the linker to resolve the symbols during linktime, while not expose the functions to the host runtime. This also means the linker can properly garbage collect any compiler-rt function that does not get resolved. The symbol visibility for all target remains the same as before: `default`.
This hack was initially introduced as we would export all symbols unconditionally, including non-function definitions. This would cause an error from the Wasmtime runtime engine, which this flag would suppress. As we now properly export symbols, this flag is no longer needed and any user running into this error can manually include it. This commit also adds the `--import-symbols` ability to build.zig
c62c88e
to
4bffe64
Compare
There are some Wasm linker test failures on
|
Force importing symbols to show the correct functions are being imported from the host environment.
e9291a4
to
8403612
Compare
My bad, I forgot to update the linker tests. |
As a wasm noob learning more about this and playing with Zig+wasm (freestanding), does this mean I have to set those build settings for my |
This would indeed affect your current codebase. If you wanted those functions to be available to the host environment, you'd either have to add This does impose the topic of developer experience. Perhaps we should add this attribute for any Zig export where the visibility is |
Got it. Yeah I’m fine either way just wanted to check what’s up. As a user, it feels weird that a lib I make with “export” doen’t become available to other thinks “loading it” by default. i.e If I am making an object file for ELF “export” makes it generally available to anything that links with that O file. If I create an object file for wasm it feels like “export” should make it generally available for “loading” it into the browser environment (its not “linking” I guess, though…). Like I said, no real dog in the fight but my 2c. I’m fine adapting to either solution. |
This is actually the same behavior as with ELF. When you export it, you make it available to anything that links with the object file, just like with ELF. The difference being is that WebAssembly has a secondary 'link' stage which happens during module instantiation, i.e. when you load it into the browser or any other runtime which is more similar to running an ELF binary on an operating system. I do appreciate your opinion though, I think the majority of the people writing Zig code targetting WebAssembly will expect exactly what you mentioned when they write |
Great work! |
I think for sure the common / expected case should be the default when exporting, and the uncommon / unexpected case is the one you need |
Supersedes #14069, see Frank's excellent explanation of why we wouldn't want to set this flag unconditionally.
This PR also effectively reverts #10517 and we will no longer automatically append
--export
for each function that is being exported. This is now possible as stage2 is the default compiler, with no dependency on the old stage1 compiler. This means we can make use of the newvisibility
field onstd.builtin.ExportOptions
, where we set it tohidden
when building compiler-rt for any WebAssembly target. This will allow the linker to still resolve the symbols to other modules but prevents it from exporting compiler-rt functions to the host environment, which also allows the functions to be garbage collected and we don't end up with bloated binaries.Note: This is a breaking change and users who wish to automatically export their symbols can do so by using the
-rdynamic
flag, or set the--export
flag themselves. As we will now no longer provide the--allow-undefined
flag unconditionally, you'll now get a proper linking error. When you do want to import a symbol from the host environment, use the--import-symbols
flag.cc @jedisct1