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

LTO strips out functions used by the trampolines resulting in a linking error #5768

Closed
koute opened this issue Feb 13, 2023 · 2 comments · Fixed by #5773
Closed

LTO strips out functions used by the trampolines resulting in a linking error #5768

koute opened this issue Feb 13, 2023 · 2 comments · Fixed by #5773
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@koute
Copy link
Contributor

koute commented Feb 13, 2023

(This looks like a rustc bug to me, but I'm reporting it here since it can be trivially worked around in wasmtime.)

When wasmtime is linked with LTO some of the symbols which are passed into the global_asm! trampolines are not properly preserved and are stripped by LTO, resulting in a link-time compile failure.

We hit this after we upgraded from wasmtime 1.0.2 to wasmtime 5.0.0.

Steps to Reproduce

git clone https://github.com/paritytech/substrate.git
cd substrate
git fetch origin 10bf4bd0d9ed75c62bf9e094759fa9315e1c2017
git checkout 10bf4bd0d9ed75c62bf9e094759fa9315e1c2017
cd bin/node/cli
cargo build --profile=production

This will take quite a while; sorry, I don't have a more minimal reproduction on hand since the issue doesn't reproduce on a toy example and reducing the actual reproduction into a minimal example is tricky when it takes forever to reproduce.

Expected Results

The compilation succeeds.

Actual Results

Linking fails.

error: linking with `cc` failed: exit status: 1
  |
  = note: "cc" "-m64" "/tmp/rustcVp2xZW/symbols.o" "/home/benchbot/cargo_target_dir/production/deps/substrate-5e6858252d8e1fde.substrate.8fb52a54-cgu.0.rcgu.o" "-Wl,--as-needed" "-L" "/home/benchbot/cargo_target_dir/production/deps" "-L" "/home/benchbot/cargo_target_dir/production/build/secp256k1-sys-325112220bd38f82/out" "-L" "/home/benchbot/cargo_target_dir/production/build/psm-9a0468c1d0238995/out" "-L" "/home/benchbot/cargo_target_dir/production/build/zstd-sys-89ca5ccaf9b199da/out" "-L" "/home/benchbot/cargo_target_dir/production/build/wasmtime-runtime-abbae99d5a8a5288/out" "-L" "/home/benchbot/cargo_target_dir/production/build/blake3-3c4da28db5e4fd3e/out" "-L" "/home/benchbot/cargo_target_dir/production/build/blake3-3c4da28db5e4fd3e/out" "-L" "/home/benchbot/cargo_target_dir/production/build/ring-2ca6aaf61769762f/out" "-L" "/home/benchbot/cargo_target_dir/production/build/librocksdb-sys-9f56c52ab5fb72b3/out" "-L" "/home/benchbot/cargo_target_dir/production/build/librocksdb-sys-9f56c52ab5fb72b3/out" "-L" "/home/benchbot/cargo_target_dir/production/build/tikv-jemalloc-sys-1c1b04141739d384/out/build/lib" "-L" "/home/benchbot/cargo_target_dir/production/build/lz4-sys-8668a2a3112e8575/out" "-L" "/usr/local/rustup/toolchains/1.66.1-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/tmp/rustcVp2xZW/liblz4_sys-d7fc638637648eea.rlib" "/tmp/rustcVp2xZW/liblibrocksdb_sys-8e2bd3fbce00bae8.rlib" "/tmp/rustcVp2xZW/libring-eaff6bf0aa221db4.rlib" "/tmp/rustcVp2xZW/libblake3-71adb6285cf48b0f.rlib" "/tmp/rustcVp2xZW/libsecp256k1_sys-3053efd9f5b6aa76.rlib" "/tmp/rustcVp2xZW/libpsm-9a050d5bd674c210.rlib" "/tmp/rustcVp2xZW/libzstd_sys-21f01571affc3d59.rlib" "/tmp/rustcVp2xZW/libwasmtime_runtime-0c33dd4e6df14e3b.rlib" "/usr/local/rustup/toolchains/1.66.1-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-91d9d5141f4e57a1.rlib" "-Wl,-Bdynamic" "-lstdc++" "-lz" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-znoexecstack" "-L" "/usr/local/rustup/toolchains/1.66.1-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/home/benchbot/cargo_target_dir/production/deps/substrate-5e6858252d8e1fde" "-Wl,--gc-sections" "-pie" "-Wl,-zrelro,-znow" "-Wl,-O1" "-nodefaultlibs"
  = note: /usr/bin/ld: /home/benchbot/cargo_target_dir/production/deps/substrate-5e6858252d8e1fde.substrate.8fb52a54-cgu.0.rcgu.o: in function `table_grow_funcref':
          substrate.8fb52a54-cgu.0:(.text+0x151): undefined reference to `wasmtime_runtime::libcalls::trampolines::impl_table_grow_funcref'
          /usr/bin/ld: /home/benchbot/cargo_target_dir/production/deps/substrate-5e6858252d8e1fde.substrate.8fb52a54-cgu.0.rcgu.o: in function `table_fill_funcref':
          substrate.8fb52a54-cgu.0:(.text+0x1b1): undefined reference to `wasmtime_runtime::libcalls::trampolines::impl_table_fill_funcref'
          collect2: error: ld returned 1 exit status

Versions and Environment

  • rustc 1.69.0-nightly (c8e6a9e8b 2023-01-23)
  • wasmtime 5.0.0
  • Linux+amd64

The issue also reproduces on macOS+aarch64.

Extra Info

There's a trivial workaround which fixes the issue:

diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs
index 5bed4d8ef..626fb861f 100644
--- a/crates/runtime/src/libcalls.rs
+++ b/crates/runtime/src/libcalls.rs
@@ -109,7 +109,7 @@ pub mod trampolines {
                 // the `sym` operator to get the symbol here, but other targets
                 // like s390x need to use outlined assembly files which requires
                 // `no_mangle`.
-                #[cfg_attr(target_arch = "s390x", no_mangle)]
+                #[no_mangle]
                 unsafe extern "C" fn [<impl_ $name>](
                     vmctx : *mut VMContext,
                     $( $pname : libcall!(@ty $param), )*

I have verified that this "fixes" the issue. (Although of course ideally it'd be best to fix the underlying rustc bug.)

@koute koute added the bug Incorrect behavior in the current implementation that needs fixing label Feb 13, 2023
@koute
Copy link
Contributor Author

koute commented Feb 14, 2023

Alternative workaround which also seems to work from what I can see:

diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs
index 626cae408..2ce3bfc7d 100644
--- a/crates/runtime/src/libcalls.rs
+++ b/crates/runtime/src/libcalls.rs
@@ -122,6 +122,17 @@ pub mod trampolines {
                         Err(panic) => crate::traphandlers::resume_panic(panic),
                     }
                 }
+
+                #[allow(non_upper_case_globals)]
+                #[used]
+                static [<impl_ $name _ref>]: unsafe extern "C" fn(
+                    *mut VMContext,
+                    $( $pname : libcall!(@ty $param), )*
+                ) $( -> libcall!(@ty $result))? = [<impl_ $name>];
             )*
         }};

@koute
Copy link
Contributor Author

koute commented Feb 14, 2023

I've also created a rustc issue here: rust-lang/rust#108030

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant