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's libLLVM-8.so doesn't work with clang #59034

Closed
emilio opened this issue Mar 9, 2019 · 7 comments · Fixed by #59173
Closed

Rust's libLLVM-8.so doesn't work with clang #59034

emilio opened this issue Mar 9, 2019 · 7 comments · Fixed by #59173
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. O-linux Operating system: Linux P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@emilio
Copy link
Contributor

emilio commented Mar 9, 2019

Which means that programs that run clang from a build script, or from cargo in some other way, fail.

Steps to Reproduce:
0. Have a clang binary pointing to clang-8 installed and in your path.

  1. rustup install nightly
  2. git clone https://github.com/rust-lang/rust-bindgen && cd $_
  3. cargo test

Actual results: /usr/bin/clang: relocation error: /lib64/libclangBasic.so.8: symbol _ZTIN4llvm14FoldingSetBaseE version LLVM_8 not defined in file libLLVM-8.so with link time reference

Expected results: not so

This only happens if ran from cargo, if I run the test executable manually then stuff works, so I suspect this is some LD_LIBRARY_PATH mungling that cargo or rustup do under the hood.

This is technically a regression from #58408, same problem does not exist with rust stable / beta and clang 7.

There's a slight chance that there's an issue on how clang-8 is built in Fedora, so I also filed https://bugzilla.redhat.com/show_bug.cgi?id=1687047, but I think it's unexpected that clang when called from cargo / build scripts uses rust's libLLVM.so, and probably should be avoided.

cc @alexcrichton

@estebank estebank added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. O-linux Operating system: Linux A-build regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 9, 2019
@nagisa
Copy link
Member

nagisa commented Mar 9, 2019

Is clang invoked from a build script? It is possible that cargo neglects removing the unnecessary LD_LIBRARY_PATH or similar environment variables before invoking the script in question.

EDIT: oh you wrote that in the report already. I remember there being an issue like this in cargo repo from a long long time ago, but cannot find it now.

@nagisa
Copy link
Member

nagisa commented Mar 9, 2019

cc rust-lang/cargo#2888

@cuviper
Copy link
Member

cuviper commented Mar 14, 2019

See also #55737.

We should probably set a custom suffix per #53987. Maybe even include the Rust version to help avoid bootstrap issues like what commit 2d21df8 saw. (cc @Mark-Simulacrum)

@Mark-Simulacrum
Copy link
Member

Yep, I think a suffix of rust version (or maybe LLVM submodule commit?) would be good.

I think we'd appreciate a PR for something like that; I would expect it to be a matter of making the version suffix default be -rust-1.33.0-<sha> or something along those lines.

@emilio
Copy link
Contributor Author

emilio commented Mar 14, 2019

Sure, that sounds good, I can give that a shot.

@emilio
Copy link
Contributor Author

emilio commented Mar 14, 2019

I sent #59173 with a change that I hope will address this, but see there why I cannot confirm it will :)

emilio added a commit to emilio/rust that referenced this issue Mar 14, 2019
I used version-channel-sha, hopefully that should work.

I checked that bootstrap builds, but I cannot check anything else since the llvm
build process is started from cargo, and thus calls clang, and thus I hit the
same bug I hope to fix with this change.

Hopefully fixes rust-lang#59034.
@pnkfelix
Copy link
Member

triage: P-high, assigning to @emilio

@pnkfelix pnkfelix added the P-high High priority label Mar 14, 2019
kennytm added a commit to kennytm/rust that referenced this issue Mar 16, 2019
bootstrap: Default to a sensible llvm-suffix.

I used version-channel-sha, hopefully that should work.

I checked that bootstrap builds, but I cannot check anything else since the llvm
build process is started from cargo, and thus calls clang, and thus I hit the
same bug I hope to fix with this change.

Hopefully fixes rust-lang#59034.
pietroalbini pushed a commit to pietroalbini/rust that referenced this issue Mar 19, 2019
I used version-channel-sha, hopefully that should work.

I checked that bootstrap builds, but I cannot check anything else since the llvm
build process is started from cargo, and thus calls clang, and thus I hit the
same bug I hope to fix with this change.

Hopefully fixes rust-lang#59034.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 1, 2020
bootstrap: Remove commit hash from LLVM version suffix to avoid rebuilds

The custom LLVM version suffix was introduced to avoid unintentional
library names conflicts. By default it included the LLVM submodule
commit hash. Changing the version suffix requires the complete LLVM
rebuild, and since then every change to the submodules required it as
well.

Remove the commit hash from version suffix to avoid complete rebuilds,
while leaving the `rust` string, the release number and release channel
to disambiguate the library name.

Context: version suffix was introduced by rust-lang#59173 as solution to rust-lang#59034.

Resolves rust-lang#68715.
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. O-linux Operating system: Linux P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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.

6 participants