Skip to content
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

command and extra exports #493

Open
yamt opened this issue Sep 6, 2022 · 14 comments
Open

command and extra exports #493

yamt opened this issue Sep 6, 2022 · 14 comments

Comments

@yamt
Copy link
Contributor

yamt commented Sep 6, 2022

recent versions of llvm wasm-ld inserts ctor/dtor for every exports for a command. [1]
it broke a few use cases including wamr's malloc/free exports. [2]

while preview2 slide [3] p7 says "No other exports", my understanding is that component-model will use some exports like canonical_abi_realloc/free, which are essentially the same as what the above mentioned wamr malloc/free exports do.

anyway, whatever we will do for preview2, it's better to unbreak the existing use cases.
IMO, the wasm-ld patch in question should be reverted, or at least conditionalized to allow plain exports.
how do you think?

[1] https://reviews.llvm.org/D81689
[2] https://reviews.llvm.org/D81689#3611504
[3] https://github.com/WebAssembly/meetings/blob/main/wasi/2022/presentations/2022-06-30-gohman-wasi-preview2.pdf

@sbc100
Copy link
Member

sbc100 commented Sep 6, 2022

From https://reviews.llvm.org/D81689: "This new behavior is disabled when the input has an explicit call to
__wasm_call_ctors, indicating code not expecting new-style command
support."

This means that if you export a traditional _start function (which should call __wasm_call_ctors) then the new behaviour should not kick in. In your examples that broke, how was __wasm_call_ctors being called?

@yamt
Copy link
Contributor Author

yamt commented Sep 6, 2022

From https://reviews.llvm.org/D81689: "This new behavior is disabled when the input has an explicit call to __wasm_call_ctors, indicating code not expecting new-style command support."

This means that if you export a traditional _start function (which should call __wasm_call_ctors) then the new behaviour should not kick in. In your examples that broke, how was __wasm_call_ctors being called?

i'm sure the new behavior kicks in for simple programs like https://github.com/yamt/garbage/tree/8dda71f39714720c921533324290b8523e263e5a/wasm/hello

@sbc100
Copy link
Member

sbc100 commented Sep 6, 2022

Putting it another way: Assuming the new behaviour can easily be avoided for those that what the more transitional single-entry command (with extra exports that are not entry point) would that be a satisfactory solution for you?

Perhaps there is some kind of bug in the automatic fallback to the old behaviour.

@sbc100
Copy link
Member

sbc100 commented Sep 6, 2022

IIRC wasi-libc's _start function should trigger the old behavior because it calls __wasm_call_ctors itself: https://github.com/WebAssembly/wasi-libc/blob/2057ce9262f76f7ef5a2002fa16da219e2176896/libc-bottom-half/crt/crt1.c#L9

Perhaps I'm missing something here? @sunfishcode ?

@yamt
Copy link
Contributor Author

yamt commented Sep 6, 2022

Putting it another way: Assuming the new behaviour can easily be avoided for those that what the more transitional single-entry command (with extra exports that are not entry point) would that be a satisfactory solution for you?

yes.
i couldn't find a nice workaround though. (i don't want to modify application C sources for this.)

@yamt
Copy link
Contributor Author

yamt commented Sep 6, 2022

IIRC wasi-libc's _start function should trigger the old behavior because it calls __wasm_call_ctors itself: https://github.com/WebAssembly/wasi-libc/blob/2057ce9262f76f7ef5a2002fa16da219e2176896/libc-bottom-half/crt/crt1.c#L9

iirc llvm picks crt1-command.o if it exists.
(this one: https://github.com/WebAssembly/wasi-libc/blob/2057ce9262f76f7ef5a2002fa16da219e2176896/libc-bottom-half/crt/crt1-command.c)

@sbc100
Copy link
Member

sbc100 commented Sep 6, 2022

Ah I see, I that case perhaps its wasi-sdk, or clang it self that should have some kind of option for force the old style crt1 to be chosen.

@yamt
Copy link
Contributor Author

yamt commented Sep 6, 2022

Ah I see, I that case perhaps its wasi-sdk, or clang it self that should have some kind of option for force the old style crt1 to be chosen.

probably.

btw, how do you think about my concern about the interaction with canonical_abi_realloc/free?

@sbc100
Copy link
Member

sbc100 commented Sep 6, 2022

Yes, clearly canonical_abi_mallc/realloc would never want to be wrapped in calls to ctors/dtors. Linker support for canonical ABI stuff is yet to be decided IIUC.. its still being thought about/worked on. For example, I've heard talk of making commands run main at instantiation time.. I'm not sure how allocation works in that case since exports are not yet available to the host (perhaps I'm confusing component instantiation with module instantiation though).

@sunfishcode
Copy link
Member

Yes, I now think we should make a change along these lines. I'm attempting to move deliberately here, to make sure that we don't cause more churn as a result of this change than we need to.

The currently-implemented concept of "new-style commands" where every entrypoint runs the constructors was based on preliminary specs for wasm commands that predate the component model and the canonical ABI. Now, the underlying standards have evolved to the point where those ideas no longer make sense. So I think it makes sense to drop the "run ctors on all exports" mode.

I can also add that the current thinking for components is that for now, wasm-ld will continue to emit core wasm modules, and we'll have a separate tool convert them into components (this may change over time, but we can consider how that works separately). So we can focus on core modules here.

@sunfishcode
Copy link
Member

I've now submitted WebAssembly/wasi-libc#328, which changes wasi-libc to always call __wasm_call_ctors for commands, which tells wasm-ld not to emit those calls itself.

@PiotrSikora
Copy link
Contributor

Either calling __wasm_call_ctors or linking with -Wl,--export=__wasm_call_ctors (from bytecodealliance/wasm-micro-runtime#1255) should work as a workaround. See #471 for previous discussion.

@yamt
Copy link
Contributor Author

yamt commented Sep 29, 2022

Either calling __wasm_call_ctors or linking with -Wl,--export=__wasm_call_ctors (from bytecodealliance/wasm-micro-runtime#1255) should work as a workaround. See #471 for previous discussion.

the former needs modification to app code, which i don't want to have.
the latter still needs someone to call ctors.

@sunfishcode
Copy link
Member

Yes, the workaround is useful for some people, and works today. The fix will be useful to everyone, once it's published, as from that point on the toolchain will just do the right thing (now that we know what the right thing is).

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 7, 2022
… r=pietroalbini

Update the wasi toolchain.

Update the WASI build to LLVM 15.0 and the wasi-libc version from [wasi-sdk-17].

This will require a ci-mirrors.rust-lang.org file load. Specifically, we need [this LLVM release tarball] uploaded to be downloadable from [this URL].

The biggest change in wasi-sdk-17 is that user exports no longer automatically run constructor functions. More details at: WebAssembly/WASI#493.

[this LLVM release tarball]: https://github.com/llvm/llvm-project/releases/download/llvmorg-15.0.6/clang+llvm-15.0.6-x86_64-linux-gnu-ubuntu-18.04.tar.xz
[this URL]: https://ci-mirrors.rust-lang.org/rustc/2022-12-06-clang%2Bllvm-15.0.6-x86_64-linux-gnu-ubuntu-18.04.tar.xz
[wasi-sdk-17]: https://github.com/WebAssembly/wasi-sdk/releases/tag/wasi-sdk-17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants