feat: Wasm Linux support wasm32-wali
#4244
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tgross35 (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
…libtest functionality
| // * Architecture-specific features | ||
| // * Dynamic linking (until Wasm gains support) | ||
| // Currently only supports the 'musl' environment | ||
| fn test_wasm32_linux(target: &str) { |
There was a problem hiding this comment.
Is most of this copied from test_linux? If so, it would just make more sense to configure wali using that same function instead of duplicating.
There was a problem hiding this comment.
A lot of it is configured using test_linux, I can use the same function. Although if it's currently only Tier-3, we could maybe hold-off on libc-test support until a later point in time. I feel like the latter makes more sense to me at the moment
There was a problem hiding this comment.
I'm okay to add testing and think it's probably a good idea if more API is planned, but that is up to you as the target maintainer. Reasonably small changes in test_linux are just preferable over having the same config in two places.
| #![feature(c_variadic)] | ||
| #![feature(concat_idents)] |
There was a problem hiding this comment.
This is just for experimentation, correct? Will need to be removed before merge.
There was a problem hiding this comment.
This will not be necessary if we remove the syscall shims.
There was a problem hiding this comment.
Same question as in #3778, why is there a b64 module when the arch is wasm32?
There was a problem hiding this comment.
Also, please make sure the unneeded comments are cleaned up once this is ready.
There was a problem hiding this comment.
As mentioned in #3778, please drop the syscall shims
There was a problem hiding this comment.
The syscall shims are rather integral to portability of many applications. What is the best direction forward to get that into the ecosystem?
There was a problem hiding this comment.
The part I am missing is, why doesn't wali just add a varidic syscall(long, ...) symbol that we could bind to? If portability is important then this seems relevant beyond Rust's libc, and it would be better to just not have the number->signature mapping responsibility in this repo.
If that doesn't work then I can help rework the macro to not rely on unstable features, and we could probably start by mapping a subset like e.g. whatever std will need.
In any case I think it's easiest to just drop this portion for now, get the rest merged, and then revisit.
There was a problem hiding this comment.
The wali-libc does provide musl's variadic syscall that can be bound too. The problem I encountered though is that most invocations of syscall used by many libraries don't adhere to the exact type signature for some arguments (using i32 instead of i64 for instance). This would not be type-checked by Rust
This causes a unique issue for variadic argument parsing in C. It has not been an issue for hardware ISAs in musl till date since they just pass arguments to registers. However, for Wasm as a virtual ISA target, variadic calls (as per clang compiler today) are supported through arguments packed into a memory buffer -- passing incorrect types will lead to wrong offset parsing.
It is currently unclear what it handle this problem in the compiler, or if it even needs to be done. The easiest reliable fix is to type-cast all arguments accordingly in Rust, which is what the shim provides.
There was a problem hiding this comment.
That being said, I'm on board for dropping the shim for the first merge, and we can start by increasing support as you suggested.
There was a problem hiding this comment.
Interesting; do varidic functions aside from syscall work cross-language, or is that one special? Rust's ... should work with the exact same ABI as C's ... and if not, it is a bug. Shimming to work around an ABI mismatch isn't a great solution.
You should open an issue in rust-lang/rust to investigate further, but if Rust's varidics aren't compatible with C's varidics then that just needs to be fixed.
There was a problem hiding this comment.
This is not a Rust variadics issue since this also happens when invoking syscall from C. It's an LLVM compiler issue in marshalling variadic arguments and should be fixable.
Description
Retargetting support to
main(from #3778) forwasm32-waliaccording to MCP rust-lang/compiler-team#797Sources
WALI documentation: https://github.com/arjunr2/WALI
Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see #3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI