search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.59k stars 12.62k forks source link

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

Open xd009642 opened 5 years ago

xd009642 commented 5 years ago

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: https://github.com/xd009642/tarpaulin/issues/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)

xd009642 commented 5 years ago

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

sharksforarms commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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

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 :smile:

richkadel commented 3 years ago

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

richkadel commented 3 years ago

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 commented 3 years ago

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

richkadel commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.