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

ThinLTO and -o compiler option can lead to duplicate object file inclusion in staticlib #64153

Closed
nikhilm opened this issue Sep 4, 2019 · 8 comments · Fixed by #65435
Closed
Assignees
Labels
A-linkage Area: linking into static, shared libraries and binaries O-linux Operating system: Linux O-macos Operating system: macOS P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikhilm
Copy link

nikhilm commented Sep 4, 2019

I've verified this on macOS 10.14/Xcode 10.3 as well as Ubuntu 14.04/GCC 4.8
rustc -V: rustc 1.36.0-nightly (50a0def 2019-05-21) (although the code i mention is the same in master)

tl;dr object files (.o) prefixes are inferred from -o, but lto object file exclusion uses crate name. External build systems which use -o can face build failures.

Setup:

  1. A staticlib that depends on a rlib (standard crate)
  2. Build using lto=thin.
  3. The rlib is built with -o that causes it to have a custom output name.

Sample: https://gist.github.com/nikhilm/a72d89002553ecab4511fbe77df223cc
Run cargo build --release -vv to get the command lines.

First, just building the dependent rlib (by copying the cargo invocation, no -o passed)

$ ar t target/release/deps/libbitflags-9e9e338798b0f64c.rlib
__.SYMDEF
bitflags-9e9e338798b0f64c.bitflags.e90hrf8b-cgu.0.rcgu.o
rust.metadata.bin
bitflags-9e9e338798b0f64c.bitflags.e90hrf8b-cgu.0.rcgu.bc.z

notice how the prefix is bitflags

Now, tweaking the rustc command for bitflags to add a -o and running ar on that file:

__.SYMDEF
libbitflags-9e9e338798b0f64c.bitflags.e90hrf8b-cgu.0.rcgu.o
rust.metadata.bin
libbitflags-9e9e338798b0f64c.bitflags.e90hrf8b-cgu.0.rcgu.bc.z

notice how the prefix is libbitflags.

This is fine. But when compiling a staticlib comprising this rlib, there is a special case when lto is enabled.

if lto && fname.starts_with(&obj_start) && fname.ends_with(".o") {
, where the obj_start is the name of the crate (bitflags).

Now, if we take a look at the cargo generated staticlib, running ar -t target/release/libhello_world.a:

__.SYMDEF
hello_world-bdf2887e2bd4c34b.3v2guokzdntel6g4.rcgu.o
hello_world-bdf2887e2bd4c34b.alloc.a2n556im-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.backtrace_sys.cepblb1k-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.bitflags.e90hrf8b-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.core.ec2x3rcv-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.hashbrown.894rc464-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.hello_world.7socix02-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.hello_world.7socix02-cgu.1.rcgu.o
hello_world-bdf2887e2bd4c34b.libc.5av1bc7k-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.panic_unwind.354k3km2-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.rustc_demangle.cq0g9zqx-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.rustc_std_workspace_alloc.eehx1vq7-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.rustc_std_workspace_core.bloqy3w9-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.std.7g3y6w1q-cgu.0.rcgu.o
hello_world-bdf2887e2bd4c34b.unwind.5mw5tvii-cgu.0.rcgu.o
...

However, if we tweak the rustc for the staticlib to link against the rlib generated using -o
If this is different from the crate name, we are in trouble because the starts_with match fails (libbitflags does not start with bitflags).

__.SYMDEF
libhello_world-bdf2887e2bd4c34b.3v2guokzdntel6g4.rcgu.o
libhello_world-bdf2887e2bd4c34b.alloc.a2n556im-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.backtrace_sys.cepblb1k-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.bitflags.e90hrf8b-cgu.0.rcgu.o  <--------
libhello_world-bdf2887e2bd4c34b.core.ec2x3rcv-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.hashbrown.894rc464-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.hello_world.7socix02-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.hello_world.7socix02-cgu.1.rcgu.o
libhello_world-bdf2887e2bd4c34b.libc.5av1bc7k-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.panic_unwind.354k3km2-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.rustc_demangle.cq0g9zqx-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.rustc_std_workspace_alloc.eehx1vq7-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.rustc_std_workspace_core.bloqy3w9-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.std.7g3y6w1q-cgu.0.rcgu.o
libhello_world-bdf2887e2bd4c34b.unwind.5mw5tvii-cgu.0.rcgu.o
libbitflags-9e9e338798b0f64c.bitflags.e90hrf8b-cgu.0.rcgu.o    <------- symbols going to conflict with the above
...
@Centril Centril added A-linkage Area: linking into static, shared libraries and binaries O-linux Operating system: Linux O-macos Operating system: macOS T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 5, 2019
@michaelwoerister
Copy link
Member

michaelwoerister commented Sep 9, 2019

Thanks a lot for the bug report, @nikhilm! And great debugging too!

This does look problematic indeed. It might be easy to fix by making the compiler behave consistently when -o is passed.

@michaelwoerister
Copy link
Member

michaelwoerister commented Sep 9, 2019

Nominating for prioritization.

@pnkfelix
Copy link
Member

pnkfelix commented Sep 12, 2019

triage: P-high, unnominating. Assigning to @michaelwoerister with intention that they choose if they'd prefer to fix it themselves, or to mentor someone else in authoring a fix.

@pnkfelix pnkfelix added P-high High priority and removed I-nominated labels Sep 12, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Oct 3, 2019

nominating for discussion at T-compiler meeting, in hopes of finding someone to address this.

@michaelwoerister
Copy link
Member

michaelwoerister commented Oct 10, 2019

I'll look into this now.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 10, 2019

great, I'll remove the nomination label.

michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 14, 2019
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 15, 2019
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 15, 2019
…to identify Rust generated object files.
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 15, 2019
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 15, 2019
…to identify Rust generated object files.
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 15, 2019
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 15, 2019
…to identify Rust generated object files.
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 15, 2019
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 15, 2019
…to identify Rust generated object files.
Centril added a commit to Centril/rust that referenced this issue Oct 15, 2019
…=alexcrichton

Fix rust-lang#64153

This PR changes how the compiler detects if an object file from an upstream crate is a Rust object file or not. Instead of checking if the name starts with the crate name and ends with `.o` (which is not always the case, as described in rust-lang#64153), it now just checks if the filename ends with `.rcgu.o`.

This fixes rust-lang#64153. However, ideally we'd clean up the code around filename generation some more. Then this check could be made more robust.

r? @alexcrichton
bors added a commit that referenced this issue Oct 15, 2019
Rollup of 9 pull requests

Successful merges:

 - #65094 (Prefer statx on linux if available)
 - #65235 (don't assume we can *always* find a return type hint in async fn)
 - #65242 (Fix suggestion to constrain trait for method to be found)
 - #65307 (Try fix incorrect "explicit lifetime name needed")
 - #65389 (Return `false` from `needs_drop` for all zero-sized arrays.)
 - #65402 (Add troubleshooting section to PGO chapter in rustc book.)
 - #65435 (Fix #64153)
 - #65438 (Organize `never_type`  tests)
 - #65444 (Implement AsRef<[T]> for List<T>)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this issue Oct 15, 2019
…=alexcrichton

Fix rust-lang#64153

This PR changes how the compiler detects if an object file from an upstream crate is a Rust object file or not. Instead of checking if the name starts with the crate name and ends with `.o` (which is not always the case, as described in rust-lang#64153), it now just checks if the filename ends with `.rcgu.o`.

This fixes rust-lang#64153. However, ideally we'd clean up the code around filename generation some more. Then this check could be made more robust.

r? @alexcrichton
bors added a commit that referenced this issue Oct 15, 2019
Rollup of 8 pull requests

Successful merges:

 - #64603 (Reducing spurious unused lifetime warnings.)
 - #65235 (don't assume we can *always* find a return type hint in async fn)
 - #65307 (Try fix incorrect "explicit lifetime name needed")
 - #65389 (Return `false` from `needs_drop` for all zero-sized arrays.)
 - #65402 (Add troubleshooting section to PGO chapter in rustc book.)
 - #65435 (Fix #64153)
 - #65438 (Organize `never_type`  tests)
 - #65444 (Implement AsRef<[T]> for List<T>)

Failed merges:

r? @ghost
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 22, 2019
michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Oct 22, 2019
…to identify Rust generated object files.
bors added a commit that referenced this issue Oct 26, 2019
Fix #64153

This PR changes how the compiler detects if an object file from an upstream crate is a Rust object file or not. Instead of checking if the name starts with the crate name and ends with `.o` (which is not always the case, as described in #64153), it now just checks if the filename ends with `.rcgu.o`.

This fixes #64153. However, ideally we'd clean up the code around filename generation some more. Then this check could be made more robust.

r? @alexcrichton
bors added a commit that referenced this issue Oct 27, 2019
Fix #64153

This PR changes how the compiler detects if an object file from an upstream crate is a Rust object file or not. Instead of checking if the name starts with the crate name and ends with `.o` (which is not always the case, as described in #64153), it now just checks if the filename ends with `.rcgu.o`.

This fixes #64153. However, ideally we'd clean up the code around filename generation some more. Then this check could be made more robust.

r? @alexcrichton
@bors bors closed this as completed in eb5ef81 Oct 29, 2019
@adetaylor
Copy link
Contributor

adetaylor commented Jan 3, 2020

Just a quick note of appreciation. Thanks @nikhilm, @michaelwoerister and anyone else involved here. I hit this exact issue and was delighted to find it was already fixed in 1.40.0.

@michaelwoerister
Copy link
Member

michaelwoerister commented Jan 4, 2020

❤️

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-linux Operating system: Linux O-macos Operating system: macOS P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
5 participants