search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.81k stars 12.77k forks source link

Signature mismatch in __wasilibc_find_relpath #83686

Open jrandall opened 3 years ago

jrandall commented 3 years ago

In https://github.com/rust-lang/rust/pull/67267, @alexcrichton indicated that the signature of __wasilibc_find_relpath had changed upstream, and that PR was merged changing the signature from a 4-argument version to a 2-argument version.

I'm not 100% sure what "upstream" means in this context, but I had previously thought it was wasi-libc from the WebAssembly repo. It does seem to be the case that until recently, WebAssembly/wasi-libc had a 2-argument version of __wasilibc_find_relpath so that seems consistent.

However, about 4 months ago in https://github.com/WebAssembly/wasi-libc/pull/214, @alexcrichton changed the signature of __wasilibc_find_relpath from a 2-argument version to a 4-argument version (among higher-level changes relating to getcwd/chdir): https://github.com/WebAssembly/wasi-libc/pull/214/files#diff-a1d7011f390a08ae599db3993ed73f49d1b52748faae3fc11d0f5a12e5e1a2edR28-R31

We have a project in which we build a rust staticlib crate using the wasm32-wasi target, and then link that with C code that is also built against wasm32-wasi. This had been working, but after updating rust to 1.50.0 (from a much older version) it is now broken with the following final (llvm-11) linker error:

wasm-ld: warning: function signature mismatch: __wasilibc_find_relpath
     >>> defined as (i32, i32) -> i32 in /build/example/libexample.a(std-0bc94da15b7c3ad9.std.553gu1ty-cgu.0.rcgu.o)
     >>> defined as (i32, i32, i32, i32) -> i32 in /opt/wasi-sdk/share/sysroot/lib/wasm32-wasi/libc.a(preopens.o)

If "upstream" means what I think it does, I think the rust signature of __wasilibc_find_relpath now no longer matches the current wasi-libc version.

alexcrichton commented 3 years ago

When using an external version of wasi-libc you'll need to match the version of wasi-libc that libstd is compiled against. In this case you'll need to have an updated version externally I believe

jrandall commented 3 years ago

That's very helpful insight, thanks @alexcrichton!

Not sure why I didn't think to mention it, but we did also update to the latest released wasi-sdk at the same time as updating rust 1.50 and clang 11, so this signature mismatch was observed with wasi-sdk 12.0 (https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-12/libclang_rt.builtins-wasm32-wasi-12.0.tar.gz), rust 1.50, and clang 11.

I've dug around the rust repo and found this script that installs wasi-libc: https://github.com/rust-lang/rust/blob/1.50.0/src/ci/docker/host-x86_64/dist-various-2/build-wasi-toolchain.sh -- is this the correct place to tell what version is being used for libstd? (i.e. is this the version to try to match against?)

If so, it looks like the version of wasi-libc it is referencing is a specific commit hash from 31st July 2020 (https://github.com/WebAssembly/wasi-libc/commit/215adc8ac9f91eb055311acc72683fd2eb1ae15a). That is prior to the changes you made that changed the signature of __wasilibc_find_relpath, so I guess we should actually be using an older version of wasi-libc and it is the newest release of wasi-sdk-12 that causes this signature mismatch (so I expect this issue may become relevant when updating the wasi-libc version in rust to the latest wasi-libc).

The version of wasi-libc rust is using does not appear to be a match to any of the versions released in wasi-sdk - the change associated with the commit it is using is listed as one of those that went into the wasi-sdk-12 release, but there were numerous other changes included in wasi-sdk-12, including the change to the signature of __wasi-libc_find_relpath (so the commit rust 1.50 is using for wasi-libc is three commits past what wasi-sdk-11 uses and seven commits before what wasi-sdk-12 uses).

I can confirm that when built against wasi-sdk-11, this signature mismatch does not occur.

However, it concerns me that there does not seem to be a way to use a released version of wasi-sdk libc to match the version of wasi-libc that rust uses.

I wonder if it would be possible to make it easier to integrate rust wasm32-wasi code with external code compiled against wasi-sdk by aligning releases between the two in some way. For example, rust could only pull in versions of wasi-libc that have been released in a wasi-sdk. Or maybe additional wasi-sdk minor releases could be arranged to correspond to the versions rust uses?

If I am correct about the toolchain installation script (https://github.com/rust-lang/rust/blob/1.51.0/src/ci/docker/host-x86_64/dist-various-2/build-wasi-toolchain.sh), it looks like rust 1.51 uses wasi-libc 58795582905e08fa7748846c1971b4ab911d1e16 which is two commits prior to the one used by wasi-sdk-12 and eight commits past the one used by wasi-sdk-11. However, the rust 1.51.0 version includes the commit that changes the __wasilibc_find_relpath signature, and it looks like rust has indeed been updated to go back to the 4-argument version.

So, to sum up in case anyone comes across this while looking into similar problems:

alexcrichton commented 3 years ago

You're right yeah that build-wasi-toolchain.sh is where Rust pulls in a particular commit. So far we've been using raw git sha's and updating as necessary. AFAIK there's not a tagging/branching scheme for what's maintained with wasi-sdk, I think wasi-sdk is just updated periodically. We try to keep things at least roughly in sync but we don't always do the best job unfortunately. I think the latest nightly Rust should have the various changes required for these signatures to match, but IIRC that update was a bit slow compared to others which I think is why wasi-sdk updated first.

In any case though it's not strictly required that the exact commits match, mainly just that signatures match. Most signatures don't change most of the time so it's fine to drift a little, but we should in general try to use the same commit as wasi-sdk as well.

jrandall commented 3 years ago

That makes sense. I wonder if there would be an appropriate place in the rust release process to add some test cases that would validate the wasm32-wasi libc it includes against a (perhaps particular documented-to-work) wasi-sdk release?

Perhaps it could explicitly be tested against the latest release of wask-sdk and would be a test that all signatures match (either by doing a static comparison of the wasi-libc included in rust with the one included in wask-sdk to detect signature mismatches or perhaps by doing some functional tests to validate that code generated by rust can be linked with C code built against wasi-sdk).

I'd be happy to try to help come up with something along these lines, but need direction in terms of what sort of tests might be appropriate and where they might best live.

alexcrichton commented 3 years ago

I think that'd be nice to have, yeah, but I'm unfortunately unsure where that would live or how it would fit best in CI. For now we're sort of relegated to manual checks unfortunately.

bjorn3 commented 1 year ago

As of https://github.com/rust-lang/rust/pull/105395 wasi-sdk-17 is used by rust. Should this be closed?