1
Fork 0

Rollup merge of #92142 - wesleywiser:fix_codecoverage_partitioning, r=tmandry

[code coverage] Fix missing dead code in modules that are never called

The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead).

The partitioning logic also caused issues in #85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols.

This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs.

Fixes #91661
Fixes #86177
Fixes #85718
Fixes #79622

r? ```@tmandry```
cc ```@richkadel```

This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
This commit is contained in:
Matthias Krüger 2022-01-13 08:11:20 +01:00 committed by GitHub
commit 5e04f513cd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 104 additions and 119 deletions

View file

@ -201,6 +201,40 @@ pub fn partition<'tcx>(
partitioner.internalize_symbols(cx, &mut post_inlining);
}
let instrument_dead_code =
tcx.sess.instrument_coverage() && !tcx.sess.instrument_coverage_except_unused_functions();
if instrument_dead_code {
assert!(
post_inlining.codegen_units.len() > 0,
"There must be at least one CGU that code coverage data can be generated in."
);
// Find the smallest CGU that has exported symbols and put the dead
// function stubs in that CGU. We look for exported symbols to increase
// the likelihood the linker won't throw away the dead functions.
// FIXME(#92165): In order to truly resolve this, we need to make sure
// the object file (CGU) containing the dead function stubs is included
// in the final binary. This will probably require forcing these
// function symbols to be included via `-u` or `/include` linker args.
let mut cgus: Vec<_> = post_inlining.codegen_units.iter_mut().collect();
cgus.sort_by_key(|cgu| cgu.size_estimate());
let dead_code_cgu = if let Some(cgu) = cgus
.into_iter()
.rev()
.filter(|cgu| cgu.items().iter().any(|(_, (linkage, _))| *linkage == Linkage::External))
.next()
{
cgu
} else {
// If there are no CGUs that have externally linked items,
// then we just pick the first CGU as a fallback.
&mut post_inlining.codegen_units[0]
};
dead_code_cgu.make_code_coverage_dead_code_cgu();
}
// Finally, sort by codegen unit name, so that we get deterministic results.
let PostInliningPartitioning {
codegen_units: mut result,