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

linking failed: undefined reference to STATIC #60184

Closed
cuviper opened this issue Apr 22, 2019 · 10 comments · Fixed by #60313
Closed

linking failed: undefined reference to STATIC #60184

cuviper opened this issue Apr 22, 2019 · 10 comments · Fixed by #60313
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-x86_32 Target: x86 processors, 32 bit (like i686-*) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented Apr 22, 2019

ref: https://bugzilla.redhat.com/show_bug.cgi?id=1701339

BUILDSTDERR: error: linking with `cc` failed: exit code: 1
BUILDSTDERR:   |
BUILDSTDERR:   = note: "cc" [...trimmed arguments...]
BUILDSTDERR:   = note: /usr/bin/ld: /builddir/build/BUILD/tokei-9.1.1/target/release/deps/libhandlebars-cbf3fa5b112da913.rlib(handlebars-cbf3fa5b112da913.handlebars.8lxzppfs-cgu.8.rcgu.o): in function `handlebars::registry::Registry::setup_builtins':
BUILDSTDERR:           /usr/share/cargo/registry/handlebars-1.2.0/src/registry.rs:116: undefined reference to `handlebars::helpers::helper_if::IF_HELPER'
BUILDSTDERR:           /usr/bin/ld: /usr/share/cargo/registry/handlebars-1.2.0/src/registry.rs:117: undefined reference to `handlebars::helpers::helper_if::UNLESS_HELPER'
BUILDSTDERR:           /usr/bin/ld: /builddir/build/BUILD/tokei-9.1.1/target/release/build/tokei-b80c0fc585eb7773/build_script_build-b80c0fc585eb7773: hidden symbol `_ZN10handlebars7helpers9helper_if9IF_HELPER17h94bc0b06c1b1df67E' isn't defined
BUILDSTDERR:           /usr/bin/ld: final link failed: bad value
BUILDSTDERR:           collect2: error: ld returned 1 exit status
BUILDSTDERR:           
BUILDSTDERR: error: aborting due to previous error
BUILDSTDERR: error: Could not compile `tokei`.

We ran into this linking failure only on i686 Fedora 30+, with LLVM 8. Other arches are fine with LLVM 8, and all arches are fine with LLVM 7 on Fedora 29.

I can reproduce this building stock tokei with normal crates.io dependencies, so that takes Fedora's crate packaging out of the picture. I also reproduced similar errors with ffsend, but different symbols: colored::style::CLEAR and time::NSEC_PER_SEC.

I can't reproduce this with official stable-1.34, 1.35-beta, nor 1.36-nightly compiler builds. However, I can reproduce it when I build those locally myself, even with a minimal config so I'm using rust's llvm-project. I have no idea what is the difference in my build at this point.

@cuviper
Copy link
Member Author

cuviper commented Apr 22, 2019

Using rust master, I bisected LLVM between 7 and 8 to find rust-lang/llvm-project@be8d199 as the point of regression. We gained this in rust when we moved to the monorepo in #57675, especially the change to rustllvm/PassWrapper.cpp.

I haven't figured out what's wrong with this yet, nor why it would only be affecting i686.

As a workaround avoiding ThinLTO, it builds fine with codegen-units = 1.

@cuviper cuviper added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-x86 T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 22, 2019
@cuviper
Copy link
Member Author

cuviper commented Apr 24, 2019

@alexcrichton, any idea why statics would be apparently lost by ThinLTO?

@alexcrichton
Copy link
Member

Oh wow starting from a bisected LLVM commit is quite nice, thanks for doing the legwork to find that!

That commit in LLVM sounds pretty suspicious though because it sounds like it's working from the assumption that ThinLTO has complete and total knowledge of the entire compilation, I'm not sure there's ever a case you can internalize something that's otherwise public. Especially in Rust's case we can never do that because we don't learn about cross-crate dependencies until the linker, and our ThinLTO phase only ever has total knowledge of one crate, no others.

From browsing around the patch it looks like we may be able to fix this though on our end. If this line is changed to false instead of true I think that this will be fixed, @cuviper would you be able to test that out?

@cuviper
Copy link
Member Author

cuviper commented Apr 25, 2019

If this line is changed to false instead of true I think that this will be fixed, @cuviper would you be able to test that out?

Huh, can it be so simple? I did have a suspicious eye on that argument, but the callers in LLVM made me think we would want true too. But yes, setting that false does seem to work! 🎉

I'll run through more tests to make sure that's not regressing anything, but I'm hopeful -- thanks!

@cuviper
Copy link
Member Author

cuviper commented Apr 25, 2019

With ImportEnabled = false, it failed a couple tests on i686, although x86_64 still passed. 😖

---- [run-pass] run-pass/thinlto/thin-lto-inlines.rs stdout ----

error: test run failed!
status: exit code: 101
command: "/home/jistone/rust/rust/build-llvm-i686/build/i686-unknown-linux-gnu/test/run-pass/thinlto/thin-lto-inlines/a"
stdout:
------------------------------------------
3 3

------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `233`,
 right: `184`', /home/jistone/rust/rust/src/test/run-pass/thinlto/thin-lto-inlines.rs:28:9
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

------------------------------------------


---- [run-pass] run-pass/thinlto/thin-lto-inlines2.rs stdout ----

error: test run failed!
status: exit code: 101
command: "/home/jistone/rust/rust/build-llvm-i686/build/i686-unknown-linux-gnu/test/run-pass/thinlto/thin-lto-inlines2/a"
stdout:
------------------------------------------
3 3

------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `83`,
 right: `184`', /home/jistone/rust/rust/src/test/run-pass/thinlto/thin-lto-inlines2.rs:26:9
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

------------------------------------------



failures:
    [run-pass] run-pass/thinlto/thin-lto-inlines.rs
    [run-pass] run-pass/thinlto/thin-lto-inlines2.rs

test result: FAILED. 2945 passed; 2 failed; 9 ignored; 0 measured; 0 filtered out

@cuviper
Copy link
Member Author

cuviper commented Apr 25, 2019

Umm, actually those tests fail for me on i686 without modification too... 😕

@alexcrichton
Copy link
Member

Those tests are pretty brittle and are probably likely to produce false negatives, but I'd also be curious to see what causes it! Sounds like otherwise it may be fixed so yay!

FWIW all I do when updating llvm is copy their code for thinlto as well and this is the first time that strategy hasn't worked!

@cuviper
Copy link
Member Author

cuviper commented Apr 25, 2019

FWIW all I do when updating llvm is copy their code for thinlto as well and this is the first time that strategy hasn't worked!

😆

I'm trying again in the i686-gnu docker image -- if I can get that to pass, I'll open a PR.

@nikic
Copy link
Contributor

nikic commented Apr 26, 2019

That commit in LLVM sounds pretty suspicious though because it sounds like it's working from the assumption that ThinLTO has complete and total knowledge of the entire compilation, I'm not sure there's ever a case you can internalize something that's otherwise public. Especially in Rust's case we can never do that because we don't learn about cross-crate dependencies until the linker, and our ThinLTO phase only ever has total knowledge of one crate, no others.

Should we be making a distinction between local and global ThinLTO here? The latter should have a full cross-crate picture, right?

@alexcrichton
Copy link
Member

I think we could do that if no native libraries are linked, but as soon as there's some native library is somewhere we also don't have the full picture because it could be relying on linking to a static that's only read from Rust (or something like that)

Centril added a commit to Centril/rust that referenced this issue Apr 27, 2019
bors added a commit that referenced this issue Apr 27, 2019
Limit internalization in LLVM 8 ThinLTO

Fixes #60184.
r? @alexcrichton
@Noratrieb Noratrieb added O-x86_32 Target: x86 processors, 32 bit (like i686-*) and removed O-x86-all labels Oct 25, 2023
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. O-x86_32 Target: x86 processors, 32 bit (like i686-*) 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