forked from rust-lang/rust
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rollup merge of rust-lang#91470 - wesleywiser:code_coverage_link_erro…
…r, r=tmandry code-cov: generate dead functions with private/default linkage As discovered in rust-lang#85461, the MSVC linker treats weak symbols slightly differently than unix-y linkers do. This causes link.exe to fail with LNK1227 "conflicting weak extern definition" where as other targets are able to link successfully. This changes the dead functions from being generated as weak/hidden to private/default which, as the LLVM reference says: > Global values with “private” linkage are only directly accessible by objects in the current module. In particular, linking code into a module with a private global value may cause the private to be renamed as necessary to avoid collisions. Because the symbol is private to the module, all references can be updated. This doesn’t show up in any symbol table in the object file. This fixes the conflicting weak symbols but doesn't address the reason *why* we have conflicting symbols for these dead functions. The test cases added in this commit contain a minimal repro of the fundamental issue which is that the logic used to decide what dead code functions should be codegen'd in the current CGU doesn't take into account that functions can be duplicated across multiple CGUs (for instance, in the case of `#[inline(always)]` functions). Fixing that is likely to be a more complex change (see rust-lang#85461 (comment)). Fixes rust-lang#85461
- Loading branch information
Showing
5 changed files
with
97 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-85461.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
../coverage/issue-85461.rs: | ||
1| |// Regression test for #85461: MSVC sometimes fail to link with dead code and #[inline(always)] | ||
2| | | ||
3| |extern crate inline_always_with_dead_code; | ||
4| | | ||
5| |use inline_always_with_dead_code::{bar, baz}; | ||
6| | | ||
7| 1|fn main() { | ||
8| 1| bar::call_me(); | ||
9| 1| baz::call_me(); | ||
10| 1|} | ||
|
||
../coverage/lib/inline_always_with_dead_code.rs: | ||
1| |// compile-flags: -Zinstrument-coverage -Ccodegen-units=4 -Copt-level=0 | ||
2| | | ||
3| |#![allow(dead_code)] | ||
4| | | ||
5| |mod foo { | ||
6| | #[inline(always)] | ||
7| 2| pub fn called() { } | ||
8| | | ||
9| 0| fn uncalled() { } | ||
10| |} | ||
11| | | ||
12| |pub mod bar { | ||
13| 1| pub fn call_me() { | ||
14| 1| super::foo::called(); | ||
15| 1| } | ||
16| |} | ||
17| | | ||
18| |pub mod baz { | ||
19| 1| pub fn call_me() { | ||
20| 1| super::foo::called(); | ||
21| 1| } | ||
22| |} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
// Regression test for #85461: MSVC sometimes fail to link with dead code and #[inline(always)] | ||
|
||
extern crate inline_always_with_dead_code; | ||
|
||
use inline_always_with_dead_code::{bar, baz}; | ||
|
||
fn main() { | ||
bar::call_me(); | ||
baz::call_me(); | ||
} |
22 changes: 22 additions & 0 deletions
22
src/test/run-make-fulldeps/coverage/lib/inline_always_with_dead_code.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
// compile-flags: -Zinstrument-coverage -Ccodegen-units=4 -Copt-level=0 | ||
|
||
#![allow(dead_code)] | ||
|
||
mod foo { | ||
#[inline(always)] | ||
pub fn called() { } | ||
|
||
fn uncalled() { } | ||
} | ||
|
||
pub mod bar { | ||
pub fn call_me() { | ||
super::foo::called(); | ||
} | ||
} | ||
|
||
pub mod baz { | ||
pub fn call_me() { | ||
super::foo::called(); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// compile-flags: -Zinstrument-coverage -Ccodegen-units=4 --crate-type dylib -Copt-level=0 | ||
// build-pass | ||
// needs-profiler-support | ||
|
||
// Regression test for #85461 where MSVC sometimes fails to link instrument-coverage binaries | ||
// with dead code and #[inline(always)]. | ||
|
||
#![allow(dead_code)] | ||
|
||
mod foo { | ||
#[inline(always)] | ||
pub fn called() { } | ||
|
||
fn uncalled() { } | ||
} | ||
|
||
pub mod bar { | ||
pub fn call_me() { | ||
super::foo::called(); | ||
} | ||
} | ||
|
||
pub mod baz { | ||
pub fn call_me() { | ||
super::foo::called(); | ||
} | ||
} |