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

Linker error because -wL as-needed conflicts with -C link-dead-code #64685

Open
xd009642 opened this issue Sep 22, 2019 · 11 comments
Open

Linker error because -wL as-needed conflicts with -C link-dead-code #64685

xd009642 opened this issue Sep 22, 2019 · 11 comments
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. link-dead-code Linkage: using -Clink-dead-code T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@xd009642
Copy link
Contributor

When a project has the [lib] tag in the Cargo.toml the -wL as-needed flag is added to the projects linker flags. However, if a project uses -C link-dead-code the two flags conflict causing a linker error of undefined reference for any functions in the lib.

This was discovered because link-dead-code is used for code coverage in tarpaulin. I spent some time seeing if there was a way around it as a user but I didn't manage to solve it myself. Issue that lead me to this for reference: xd009642/tarpaulin#126

Any solutions I could currently implement would be appreciated if this requires a PR I'm also happy to contribute (though may need some mentorship)

@jonas-schievink jonas-schievink added A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 22, 2019
@xd009642
Copy link
Contributor Author

xd009642 commented Oct 9, 2019

Is there any idea about what it would take to fix this? I'm interested in helping out if someone can help guide me 😄

@sharksforarms
Copy link

Getting a similar issue in suricata: https://github.com/OISF/suricata/tree/master/rust

I tried removing the [lib] but still getting undefined symbols... I wonder if it's the extern C stuff

@richkadel
Copy link
Contributor

When compiling Rust binaries with -Z instrument-coverage and -C link-dead-code enabled, Rust binaries compiled for the Windows-MSVC target would typically crash (seg-fault) during the post-exit phase of writing the coverage-counter results to the *.profraw file (or at best, generate an empty file).

After learning of this known bug, I discovered a workaround: Disabling link-dead-code, via -C link-dead-code=no, resolves the problem with -Z instrument-coverage when targeting Windows-MSVC. I'm incorporating this workaround in PR #75828 , but note that this does not actually correct the bug.

It is an acceptable workaround for now, but I would prefer to enable link-dead-code, so dead code would still show up in the coverage reports (with zero execution counts).

Before learning of this workaround, I did some extensive investigation of the bug with -Z instrument-coverage on MSVC. I've documented my discoveries and a lot of the intermediate results, in case they are helpful to someone with more knowledge in this area. I think I understand why the program fails (and how it appears to be a link-time issue), but I was not able to determine why the linker is doing what it is doing, or how to fix it.

@richkadel
Copy link
Contributor

After some sidebar discussions, I think the issue with -Z instrument-coverage and -C link-dead-code (which is only present when targeting MSVC and using the MSVC linker) is different. I've created a new bug report, Issue #76038 , and referenced this one, just in case someone sees a relationship.

@xd009642
Copy link
Contributor Author

@jonas-schievink @richkadel Just tagging you both as you seem to be the two contributors who have interacted with this issue 😉

I was considering attempting to tackle this one as it's been a proverbial thorn in my side for a while. As an approach I was considering looking for -wL as-needed when linking a dependency and then just not adding link-dead-code for that dependency. I know you can pass linker flags to just the crate you're compiling via cargo rustc so I figured this approach would naively work.

Just wanted to make sure it made sense as an approach and there's no linker things I'm oblivious too and if so a vague pointer towards where in the rust code to look to figure it out would be appreciated 😄

@richkadel
Copy link
Contributor

I'm afraid I don't really have enough depth-of-knowledge here to be of much help.

@richkadel
Copy link
Contributor

A brief note for anyone reviewing this issue, as of PR #79109, -Z instrument-coverage no longer automatically enables -C link-dead-code.

This issue is no longer relevant to LLVM source code coverage, but may still be relevant to other use cases of course.

@xd009642
Copy link
Contributor Author

xd009642 commented Dec 3, 2020

@richkadel won't that impact the accuracy of the LLVM code coverage then?

@richkadel
Copy link
Contributor

I implemented coverage for unused functions in a different way, which is actually more complete than anything I was getting from -C link-dead-code. The -C link-dead-code didn't help much before, and now it's redundant.

@xd009642
Copy link
Contributor Author

xd009642 commented Dec 3, 2020

Oh that's great! How does it tackle things like unused generic code? Will it cover them or only cover them if instantiated with a type somewhere?

@richkadel
Copy link
Contributor

Unused generics are covered too. Since I'm going back to MIR, the MIR is generated based on the parsed code, not used code, so I don't need it to be instantiated.

That's one of the issues I was trying to solve with this new approach.

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 C-bug Category: This is a bug. link-dead-code Linkage: using -Clink-dead-code T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants