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 prunes symbols which are used resulting in a linking error #108030

Closed
koute opened this issue Feb 14, 2023 · 4 comments · Fixed by #115757
Closed

LTO prunes symbols which are used resulting in a linking error #108030

koute opened this issue Feb 14, 2023 · 4 comments · Fixed by #115757
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@koute
Copy link
Member

koute commented Feb 14, 2023

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 (c8e6a9e 2023-01-23)
  • rustc 1.69.0-nightly (065852d 2023-02-13)
  • Linux+amd64

The issue also reproduces on macOS+aarch64.

Extra Info

I'm not sure whether this is a generic LTO bug, or a bug with global_asm!.

The wasmtime crate sets up a bunch of global_asm! trampolines like this:

Relevant code
cfg_if::cfg_if! {
        #[macro_export]
        macro_rules! asm_func {
            ($name:expr, $body:expr $(, $($args:tt)*)?) => {
                std::arch::global_asm!(
                    concat!(
                        ".p2align 4\n",
                        ".hidden ", $name, "\n",
                        ".global ", $name, "\n",
                        $crate::elf_func_type_header!($name),
                        $name, ":\n",
                        $body,
                        ".size ", $name, ",.-", $name,
                    )
                    $(, $($args)*)?
                );
            };
        }
    }
}

// ...

#[rustfmt::skip]
macro_rules! wasm_to_libcall_trampoline {
    ($libcall:ident ; $libcall_impl:ident) => {
        wasmtime_asm_macros::asm_func!(
            stringify!($libcall),
            concat!(
                "
                   .cfi_startproc simple
                   .cfi_def_cfa_offset 0

                    // Load the pointer to `VMRuntimeLimits` in `", scratch0!(), "`.
                    mov ", scratch0!(), ", 8[", arg0!(), "]

                    // Store the last Wasm FP into the `last_wasm_exit_fp` in the limits.
                    mov 24[", scratch0!(), "], rbp

                    // Store the last Wasm PC into the `last_wasm_exit_pc` in the limits.
                    mov ", scratch1!(), ", [rsp]
                    mov 32[", scratch0!(), "], ", scratch1!(), "

                    // Tail call to the actual implementation of this libcall.
                    jmp {}

                    .cfi_endproc
                ",
            ),
            sym $libcall_impl
        );
    };
}

// ...

    macro_rules! libcall {
        (
            $(
                $( #[$attr:meta] )*
                $name:ident( vmctx: vmctx $(, $pname:ident: $param:ident )* ) $( -> $result:ident )?;
            )*
        ) => {paste::paste! {
            $(
                // The actual libcall itself, which has the `pub` name here, is
                // defined via the `wasm_to_libcall_trampoline!` macro on
                // supported platforms or otherwise in inline assembly for
                // platforms like s390x which don't have stable `global_asm!`
                // yet.
                extern "C" {
                    #[allow(missing_docs)]
                    #[allow(improper_ctypes)]
                    pub fn $name(
                        vmctx: *mut VMContext,
                        $( $pname: libcall!(@ty $param), )*
                    ) $(-> libcall!(@ty $result))?;
                }

                wasm_to_libcall_trampoline!($name ; [<impl_ $name>]);

                // This is the direct entrypoint from the inline assembly which
                // still has the same raw signature as the trampoline itself.
                // This will delegate to the outer module to the actual
                // implementation and automatically perform `catch_unwind` along
                // with conversion of the return value in the face of traps.
                //
                // Note that rust targets which support `global_asm!` can use
                // 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)]
                unsafe extern "C" fn [<impl_ $name>](
                    vmctx : *mut VMContext,
                    $( $pname : libcall!(@ty $param), )*
                ) $( -> libcall!(@ty $result))? {
                    let result = std::panic::catch_unwind(|| {
                        super::$name(vmctx, $($pname),*)
                    });
                    match result {
                        Ok(ret) => LibcallResult::convert(ret),
                        Err(panic) => crate::traphandlers::resume_panic(panic),
                    }
                }
            )*
        }};

        (@ty i32) => (u32);
        (@ty i64) => (u64);
        (@ty reference) => (*mut u8);
        (@ty pointer) => (*mut u8);
        (@ty vmctx) => (*mut VMContext);
    }

// ...

libcall! {
    // ...
    /// Returns an index for Wasm's `table.fill` instruction for `externref`s.
    table_fill_externref(vmctx: vmctx, table: i32, dst: i32, val: reference, len: i32);
    /// Returns an index for Wasm's `table.fill` instruction for `funcref`s.
    table_fill_funcref(vmctx: vmctx, table: i32, dst: i32, val: pointer, len: i32);
    // ...
}

// ...

// Implementation of `table.fill`.
unsafe fn table_fill(
    vmctx: *mut VMContext,
    table_index: u32,
    dst: u32,
    // NB: we don't know whether this is a `VMExternRef` or a pointer to a
    // `VMCallerCheckedAnyfunc` until we look at the table's element type.
    val: *mut u8,
    len: u32,
) -> Result<(), Trap> {
    // ...
}

use table_fill as table_fill_funcref;
use table_fill as table_fill_externref;

So as you can see the symbol that's being unnecessarily stripped out is being passed into the global_asm! block (sym $libcall_impl) and that is being called from assembly (jmp {}). That symbol is just a normal mangled extern "C" function (unsafe extern "C" fn [<impl_ $name>]). What's also interesting is also that this seems to affect only these two symbols (there are more that are defined, and it seems like those are not being stripped out for some reason), and that it doesn't reproduce on toy examples.

There are also two potential workarounds; first workaround is to just mark the function as #[no_mangle]:

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), )*

The other is to create a dummy static variable with a #[used] on it:

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>];
             )*
         }};

After applying either of those the linking succeeds even when LTO is enabled.

@koute
Copy link
Member Author

koute commented Feb 14, 2023

Might or might not be related: #107345

@DianQK
Copy link
Member

DianQK commented Mar 4, 2023

This is a minimal reproduction that works on macOS and Linux (x86_64).

linking-error.zip

I think the D145293 fixes this issue.

@DianQK
Copy link
Member

DianQK commented Mar 4, 2023

@rustbot label +A-llvm

@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Mar 4, 2023
@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@DianQK
Copy link
Member

DianQK commented Jul 22, 2023

@rustbot claim

DianQK added a commit to DianQK/rust that referenced this issue Sep 11, 2023
Closes rust-lang#108030.
This issue has been resolved in LLVM 17.
DianQK added a commit to DianQK/rust that referenced this issue Sep 11, 2023
Closes rust-lang#108030.
This issue has been resolved in LLVM 17.
DianQK added a commit to DianQK/rust that referenced this issue Sep 11, 2023
Closes rust-lang#108030.
This issue has been resolved in LLVM 17.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 11, 2023
…sleywiser

Add a test for rust-lang#108030

Closes rust-lang#108030.

This issue has been resolved in LLVM 17. I can verify that this test fails on 63a81b0.

r? compiler
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 11, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#115548 (Extract parallel operations in `rustc_data_structures::sync` into a new `parallel` submodule)
 - rust-lang#115591 (Add regression test for LLVM 17-rc3 miscompile)
 - rust-lang#115631 (Don't ICE when computing ctype's `repr_nullable_ptr` for possibly-unsized ty)
 - rust-lang#115708 (fix homogeneous_aggregate not ignoring some ZST)
 - rust-lang#115730 (Some more small driver refactors)
 - rust-lang#115749 (Allow loading the SMIR for constants and statics)
 - rust-lang#115757 (Add a test for rust-lang#108030)
 - rust-lang#115761 (Update books)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in b99ace4 Sep 11, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 11, 2023
Rollup merge of rust-lang#115757 - DianQK:lto-linkage-used-attr, r=wesleywiser

Add a test for rust-lang#108030

Closes rust-lang#108030.

This issue has been resolved in LLVM 17. I can verify that this test fails on 63a81b0.

r? compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants