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

Namespace conflicts with LLVM #154

Open
xermicus opened this issue Sep 12, 2023 · 7 comments
Open

Namespace conflicts with LLVM #154

xermicus opened this issue Sep 12, 2023 · 7 comments

Comments

@xermicus
Copy link

xermicus commented Sep 12, 2023

In Solang, a Solidity compiler, we use LLVM (via inkwell) for emitting code and wasm-opt for optimizing our Wasm artifacts.

We noticed that, starting from version 0.114.0 on, wasm-opt introduces many namespace conflicts during linking, if it is used together with llvm-sys or as a transitive dependency.

Examples (full output too long to fit in the issue) = note: /usr/bin/ld: /home/glow/code/foo/target/debug/deps/libllvm_sys-a8e7e255aa417530.rlib(Debug.cpp.o):(.bss._ZN4llvm9DebugFlagE+0x0): multiple definition of `llvm::DebugFlag'; /home/gl ow/code/foo/target/debug/deps/libwasm_opt_sys-39024ee919d3d0a2.rlib(4036ad2f21ce3a71-LLVMDebug.o):/home/glow/code/foo/target/debug/build/wasm-opt-sys-6495f62a81ee79e6/out/LLVMDebug.cpp:43: first defined here 1 /usr/bin/ld: /home/glow/code/foo/target/debug/deps/libllvm_sys-a8e7e255aa417530.rlib(Debug.cpp.o): in function `llvm::dbgs() [clone .localalias]': 2 Debug.cpp:(.text._ZN4llvm4dbgsEv+0x0): multiple definition of `llvm::dbgs()'; /home/glow/code/foo/target/debug/deps/libwasm_opt_sys-39024ee919d3d0a2.rlib(4036ad2f21ce3a71-LLVMDeb ug.o):/home/glow/code/foo/target/debug/build/wasm-opt-sys-6495f62a81ee79e6/out/LLVMDebug.cpp:148: first defined here ... Path.cpp:(.text._ZN4llvm3sys4path6nativeERKNS_5TwineERNS_15SmallVectorImplIcEENS1_5StyleE+0x0): multiple definition of `llvm::sys::path::native(llvm::Twine const&, llvm::SmallVectorImpl&, llvm::sys::path::Style)'; /home/glow/code/foo/target/debug/deps/libwasm_opt_sys-39024ee919d3d0a2.rlib(dae784741d7c58d5-Path.o):/home/glow/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wasm-opt-sys-0.114.1/binaryen/third_party/llvm-project/Path.cpp:477: first defined here collect2: error: ld returned 1 exit status

On a quick glance this seems to be caused by wasm-opt including a bunch of LLVM source dependencies.

This leaves anyone depending on LLVM and wasm-opt at the same time in an unfortunate situation.

To make matters worse, while Linux ld detects that and aborts the compilation, we noticed on our CI that on Mac (arm) the problem is worse: The linker on MacOS doesn't seem to care and compiles it anyways, instead resulting in a broken build artifact segfaulting at runtime 😅

Minimal reproducer

Just cargo build on a new rust project with the following:

Cargo.toml

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

[dependencies]
llvm-sys = "150.0.0"
wasm-opt = "0.114"

src/main.rs

fn main() {
    unsafe { llvm_sys::core::LLVMModuleCreateWithName(std::ptr::null()) };
    wasm_opt::OptimizationOptions::new_opt_level_0();
}
@brson
Copy link
Owner

brson commented Sep 14, 2023

Hi @xermicus.

Thanks for this report. We've reproduced the problem.

It was introduced in 0.114.1, in this pr #151. I thought this would not be a breaking change, but hadn't considered the possibility you have run into.

As a temporary workaround, if you pin wasm-opt to 0.114.0, the problem should go away. This can be done by using "=0.114.0" as the version specifier.

Binaryen includes LLVM source code to handle DWARF, and previously we (accidentally) did not compile these files, so prior to 0.114.1 the wasm-opt crate did not correctly execute any of binaryen's DWARF passes. This likely hadn't been noticed because most users are telling rustc not to emit DWARF for release.

At the moment I can think of two possible resolutions:

In the short-term we can add a cargo feature to control whether the LLVM/DWARF support is compiled, probably "dwarf". For a 0.114.2 release this would probably be a non-default feature for compatibility with 0.114.0, but for 0.115.0 it would be a default feature that would need to be manually disabled. I would probably then yank the 0.114.1 release.

Would that work for you @xermicus?

A more desirable solution would be to somehow disambiguate the LLVM names used by binaryen. It's not obvious to me how to do this at the moment. Just rewriting the LLVM namespaces to something unambiguous seems possible, though from a glance it looks like some of the LLVM code is not namespaced.

cc @dzfrias some discussion here about potential future changes to how the LLVM source files are compiled. Could affect your usage.

@dzfrias
Copy link
Contributor

dzfrias commented Sep 19, 2023

Hmm, that's a tough problem. I think the "dwarf" feature is a good idea, and making it enabled by default in 0.115.0 seems reasonable. Thank you for letting me know for when 0.114.2 comes out!

@xermicus
Copy link
Author

Hi @brson

I see, thanks for breaking down the problem. Yeah we we're concerned about this being not trivial to resolve. Solving the problem properly by disambiguating the namespaces is probably what we would need in the long term. But I think a feature flag might work for us short term (wasm-opt is also a transitive dependency in our case). Thanks for catching it up!

@tareknaser
Copy link

Hey @brson

Any updates on this issue?
Are you planning to release a new version with a fix soon?

@brson
Copy link
Owner

brson commented Oct 16, 2023

@tareknaser I think we'll put out a new build of 0.114 this week, and a also 0.116.

@brson
Copy link
Owner

brson commented Oct 19, 2023

We've done as discussed previously:

  • yanked 0.114.1
  • published 0.114.2 with a "dwarf" feature that enables the LLVM code and dwarf passes
    • this is a non-default feature for compatibility with 0.114.0, but breaking for those using the dwarf features of 0.114.1
  • published 0.116.0 with the "dwarf" feature enabled by default
    • those on this version that need to link to LLVM will need to set default-features = false

@xermicus
Copy link
Author

@brson thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants