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

rust-lld links WebAssembly imports during compilation #56309

Closed
PiotrSikora opened this issue Nov 28, 2018 · 1 comment · Fixed by #67363
Closed

rust-lld links WebAssembly imports during compilation #56309

PiotrSikora opened this issue Nov 28, 2018 · 1 comment · Fixed by #67363
Labels
A-linkage Area: linking into static, shared libraries and binaries O-wasm Target: WASM (WebAssembly), http://webassembly.org/

Comments

@PiotrSikora
Copy link

PiotrSikora commented Nov 28, 2018

It appears that rust-lld links WebAssembly imports during compilation, even though they should be only imported at runtime.

Code

#[link(wasm_import_module = "test")]
extern "C" {
    fn log(message_data: u32, message_size: u32);
}

#[no_mangle]
pub fn main() {
    let message = "Hello, world!";
    unsafe {
        log(message.as_ptr() as u32, message.len() as u32);
    }
}

Cargo (stable)

Build succeeds:

$ cargo build --target=wasm32-unknown-unknown --release --verbose
   Compiling wasm-log v0.0.1 (/home/piotrsikora/wasm-log)
     Running `rustc --crate-name wasm_log src/lib.rs --color always --crate-type cdylib --emit=dep-info,link -C opt-level=3 -C panic=abort -C lto -C metadata=974cbcc98065209d --out-dir /home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps --target wasm32-unknown-unknown -L dependency=/home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps -L dependency=/home/piotrsikora/wasm-log/target/release/deps`
    Finished release [optimized] target(s) in 0.49s

but there are no imports in the compiled WASM module:

$ wasm-dis target/wasm32-unknown-unknown/release/wasm_log.wasm | grep import
<empty>

and it actually inlines some log() function (probably from compiler-builtins).

Cargo (nightly)

Build fails due to a conflict between WebAssembly import (test.log) and compiler-builtins's log():

$ cargo +nightly build --target=wasm32-unknown-unknown --release --verbose
   Compiling wasm-log v0.0.1 (/home/piotrsikora/wasm-log)
     Running `rustc --crate-name wasm_log src/lib.rs --color always --crate-type cdylib --emit=dep-info,link -C opt-level=3 -C panic=abort -C lto -C metadata=974cbcc98065209d --out-dir /home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps --target wasm32-unknown-unknown -L dependency=/home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps -L dependency=/home/piotrsikora/wasm-log/target/release/deps`
error: linking with `rust-lld` failed: exit code: 1
  |
  = note: "rust-lld" "-flavor" "wasm" "-L" "/home/piotrsikora/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/wasm32-unknown-unknown/lib" "/home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps/wasm_log.wasm_log.ch5ijyk4-cgu.0.rcgu.o" "-o" "/home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps/wasm_log.wasm" "--export" "main" "--gc-sections" "-O3" "-L" "/home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps" "-L" "/home/piotrsikora/wasm-log/target/release/deps" "-L" "/home/piotrsikora/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/wasm32-unknown-unknown/lib" "/home/piotrsikora/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/wasm32-unknown-unknown/lib/libcompiler_builtins-e275954395f12431.rlib" "--no-threads" "-z" "stack-size=1048576" "--stack-first" "--allow-undefined" "--no-entry" "--export-table" "--fatal-warnings"
  = note: rust-lld: error: function signature mismatch: log
          >>> defined as (I32, I32) -> void in /home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps/wasm_log.wasm_log.ch5ijyk4-cgu.0.rcgu.o
          >>> defined as (F64) -> F64 in /home/piotrsikora/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/wasm32-unknown-unknown/lib/libcompiler_builtins-e275954395f12431.rlib(compiler_builtins-e275954395f12431.compiler_builtins.2709pw9r-cgu.0.rcgu.o)


error: aborting due to previous error

error: Could not compile `wasm-log`.

Caused by:
  process didn't exit successfully: `rustc --crate-name wasm_log src/lib.rs --color always --crate-type cdylib --emit=dep-info,link -C opt-level=3 -C panic=abort -C lto -C metadata=974cbcc98065209d --out-dir /home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps --target wasm32-unknown-unknown -L dependency=/home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps -L dependency=/home/piotrsikora/wasm-log/target/release/deps` (exit code: 1)

Workaround

Simply changing the function name, either directly or by using #[link_name = "xlog"], to avoid the conflict works around the issue, but that's obviously far from perfect.

#[link(wasm_import_module = "test")]
extern "C" {
    #[link_name = "xlog"]
    fn log(message_data: u32, message_size: u32);
}

#[no_mangle]
pub fn main() {
    let message = "Hello, world!";
    unsafe {
        log(message.as_ptr() as u32, message.len() as u32);
    }
}
$ cargo +nightly build --target=wasm32-unknown-unknown --release --verbose
   Compiling wasm-log v0.0.1 (/home/piotrsikora/wasm-log)
     Running `rustc --crate-name wasm_log src/lib.rs --color always --crate-type cdylib --emit=dep-info,link -C opt-level=3 -C panic=abort -C lto -C metadata=974cbcc98065209d --out-dir /home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps --target wasm32-unknown-unknown -L dependency=/home/piotrsikora/wasm-log/target/wasm32-unknown-unknown/release/deps -L dependency=/home/piotrsikora/wasm-log/target/release/deps`
    Finished release [optimized] target(s) in 0.36s
$ wasm-dis target/wasm32-unknown-unknown/release/wasm_log.wasm | grep import
 (import "test" "xlog" (func $xlog (param i32 i32)))

Version info

$ rustc --version --verbose
rustc 1.30.1 (1433507eb 2018-11-07)
binary: rustc
commit-hash: 1433507eba7d1a114e4c6f27ae0e1a74f60f20de
commit-date: 2018-11-07
host: x86_64-unknown-linux-gnu
release: 1.30.1
LLVM version: 8.0
$ rustc +nightly --version --verbose
rustc 1.32.0-nightly (6acbb5b65 2018-11-25)
binary: rustc
commit-hash: 6acbb5b65c06d82c867a94c54ce51dab4707ac61
commit-date: 2018-11-25
host: x86_64-unknown-linux-gnu
release: 1.32.0-nightly
LLVM version: 8.0
@alexcrichton
Copy link
Member

Thanks for the reprot! This is due to the fact that log is a built-in symbol for the mathematical log function. (this symbol is common across all platforms). As you've found though renaming does the trick!

@jonas-schievink jonas-schievink added A-linkage Area: linking into static, shared libraries and binaries O-wasm Target: WASM (WebAssembly), http://webassembly.org/ labels Jan 27, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Dec 19, 2019
…=eddyb

Fix handling of wasm import modules and names

The WebAssembly targets of rustc have weird issues around name mangling
and import the same name from different modules. This all largely stems
from the fact that we're using literal symbol names in LLVM IR to
represent what a function is called when it's imported, and we're not
using the wasm-specific `wasm-import-name` attribute. This in turn leads
to two issues:

* If, in the same codegen unit, the same FFI symbol is referenced twice
  then rustc, when translating to LLVM IR, will only reference one
  symbol from the first wasm module referenced.

* There's also a bug in LLD [1] where even if two codegen units
  reference different modules, having the same symbol names means that
  LLD coalesces the symbols and only refers to one wasm module.

Put another way, all our imported wasm symbols from the environment are
keyed off their LLVM IR symbol name, which has lots of collisions today.
This commit fixes the issue by implementing two changes:

1. All wasm symbols with `#[link(wasm_import_module = "...")]` are
   mangled by default in LLVM IR. This means they're all given unique names.

2. Symbols then use the `wasm-import-name` attribute to ensure that the
   WebAssembly file uses the correct import name.

When put together this should ensure we don't trip over the LLD bug [1]
and we also codegen IR correctly always referencing the right symbols
with the right import module/name pairs.

Closes rust-lang#50021
Closes rust-lang#56309
Closes rust-lang#63562

[1]: https://bugs.llvm.org/show_bug.cgi?id=44316
Centril added a commit to Centril/rust that referenced this issue Dec 20, 2019
…=eddyb

Fix handling of wasm import modules and names

The WebAssembly targets of rustc have weird issues around name mangling
and import the same name from different modules. This all largely stems
from the fact that we're using literal symbol names in LLVM IR to
represent what a function is called when it's imported, and we're not
using the wasm-specific `wasm-import-name` attribute. This in turn leads
to two issues:

* If, in the same codegen unit, the same FFI symbol is referenced twice
  then rustc, when translating to LLVM IR, will only reference one
  symbol from the first wasm module referenced.

* There's also a bug in LLD [1] where even if two codegen units
  reference different modules, having the same symbol names means that
  LLD coalesces the symbols and only refers to one wasm module.

Put another way, all our imported wasm symbols from the environment are
keyed off their LLVM IR symbol name, which has lots of collisions today.
This commit fixes the issue by implementing two changes:

1. All wasm symbols with `#[link(wasm_import_module = "...")]` are
   mangled by default in LLVM IR. This means they're all given unique names.

2. Symbols then use the `wasm-import-name` attribute to ensure that the
   WebAssembly file uses the correct import name.

When put together this should ensure we don't trip over the LLD bug [1]
and we also codegen IR correctly always referencing the right symbols
with the right import module/name pairs.

Closes rust-lang#50021
Closes rust-lang#56309
Closes rust-lang#63562

[1]: https://bugs.llvm.org/show_bug.cgi?id=44316
Centril added a commit to Centril/rust that referenced this issue Dec 20, 2019
…=eddyb

Fix handling of wasm import modules and names

The WebAssembly targets of rustc have weird issues around name mangling
and import the same name from different modules. This all largely stems
from the fact that we're using literal symbol names in LLVM IR to
represent what a function is called when it's imported, and we're not
using the wasm-specific `wasm-import-name` attribute. This in turn leads
to two issues:

* If, in the same codegen unit, the same FFI symbol is referenced twice
  then rustc, when translating to LLVM IR, will only reference one
  symbol from the first wasm module referenced.

* There's also a bug in LLD [1] where even if two codegen units
  reference different modules, having the same symbol names means that
  LLD coalesces the symbols and only refers to one wasm module.

Put another way, all our imported wasm symbols from the environment are
keyed off their LLVM IR symbol name, which has lots of collisions today.
This commit fixes the issue by implementing two changes:

1. All wasm symbols with `#[link(wasm_import_module = "...")]` are
   mangled by default in LLVM IR. This means they're all given unique names.

2. Symbols then use the `wasm-import-name` attribute to ensure that the
   WebAssembly file uses the correct import name.

When put together this should ensure we don't trip over the LLD bug [1]
and we also codegen IR correctly always referencing the right symbols
with the right import module/name pairs.

Closes rust-lang#50021
Closes rust-lang#56309
Closes rust-lang#63562

[1]: https://bugs.llvm.org/show_bug.cgi?id=44316
@bors bors closed this as completed in aa0ef5a Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries O-wasm Target: WASM (WebAssembly), http://webassembly.org/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants