-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Panic in LLVM when using #[linkage] even with supported targets #33992
Comments
Just ran this on Ubuntu, expecting it to work, and it didn't. I guess this isn't Windows issue after all. Am I using this attribute incorrectly? Does it work? |
Well it works with |
If I'm reading https://github.com/rust-lang/llvm/blob/master/include/llvm/IR/GlobalValue.h#L32 correctly, ExternalWeakLinkage should equal 14 and CommonLinkage should equal 15. https://github.com/rust-lang/rust/blob/master/src/librustc_llvm/lib.rs#L102 |
I'm putting together a PR with the fix and some compile-pass tests. Might take a few hours to compile, though 😢 |
The `Linkage` enum in librustc_llvm got out of sync with the version in LLVM and it caused two variants of the #[linkage=""] attribute to break. Fixes rust-lang#33992
The `Linkage` enum in librustc_llvm got out of sync with the version in LLVM and it caused two variants of the #[linkage=""] attribute to break. This adds the functions `LLVMRustGetLinkage` and `LLVMRustSetLinkage` which convert between the Rust Linkage enum and the LLVM one, which should stop this from breaking every time LLVM changes it. Fixes rust-lang#33992
…chton Fix incorrect LLVM Linkage enum Followup of rust-lang#33994 to actually work. The `Linkage` enum in librustc_llvm got out of sync with the version in LLVM and it caused two variants of the `#[linkage=""]` attribute to break. This adds the functions `LLVMRustGetLinkage` and `LLVMRustSetLinkage` which convert between the Rust Linkage enum and the LLVM one, which should stop this from breaking every time LLVM changes it. Possible remaining concerns: 1. There could be a codegen test to make sure that the attributes are applied correctly (I don't know how to do this). 2. The test does not exercise the `appending` linkage. I can't figure out how to make a global static raw pointer to an array. This might not even be possible? If not we should probably remove appending linkage as its unusable in rust. 3. The test only runs on Linux. Fixes rust-lang#33992 r? @alexcrichton
Edit: This was meant to be posted to the corresponding pull request.
|
The `Linkage` enum in librustc_llvm got out of sync with the version in LLVM and it caused two variants of the #[linkage=""] attribute to break. This adds the functions `LLVMRustGetLinkage` and `LLVMRustSetLinkage` which convert between the Rust Linkage enum and the LLVM one, which should stop this from breaking every time LLVM changes it. Fixes rust-lang#33992
Fix incorrect LLVM Linkage enum Followup of #33994 to actually work. The `Linkage` enum in librustc_llvm got out of sync with the version in LLVM and it caused two variants of the `#[linkage=""]` attribute to break. This adds the functions `LLVMRustGetLinkage` and `LLVMRustSetLinkage` which convert between the Rust Linkage enum and the LLVM one, which should stop this from breaking every time LLVM changes it. Possible remaining concerns: 1. There could be a codegen test to make sure that the attributes are applied correctly (I don't know how to do this). 2. ~~The test does not exercise the `appending` linkage. I can't figure out how to make a global static raw pointer to an array. This might not even be possible? If not we should probably remove appending linkage as its unusable in rust.~~ Appending linkage is not 'emittable' anyway. 3. The test only runs on Linux. Fixes #33992 r? @alexcrichton
Nothing too unexpected, since most targets don't support weak linkage. The error message could be a bit better though, rather than panicing in LLVM.
I was a bit surprised that this problem persists when cross compiling to supported targets, but I suppose that its an LLVM limitation, and a very niche one at that.
cc #29603
The text was updated successfully, but these errors were encountered: