about summary refs log tree commit diff
diff options
context:
space:
mode:
authorWesley Wiser <wesleywiser@microsoft.com>2021-12-02 16:16:34 -0500
committerWesley Wiser <wesleywiser@microsoft.com>2021-12-03 12:00:12 -0500
commitd5f6b9c8c228689026aedc7e6e2cc13294f1020f (patch)
treee4e0fdf7a4d87201836d248f52a72036eb125b74
parentd47a6cc3f2dab0ef046c2bb7b76a9ee8d1a0be92 (diff)
downloadrust-d5f6b9c8c228689026aedc7e6e2cc13294f1020f.tar.gz
rust-d5f6b9c8c228689026aedc7e6e2cc13294f1020f.zip
code-cov: generate dead functions with private/default linkage
As discovered in #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
https://github.com/rust-lang/rust/issues/85461#issuecomment-985005805).

Fixes #85461
-rw-r--r--compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs4
-rw-r--r--src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-85461.txt36
-rw-r--r--src/test/run-make-fulldeps/coverage/issue-85461.rs10
-rw-r--r--src/test/run-make-fulldeps/coverage/lib/inline_always_with_dead_code.rs22
-rw-r--r--src/test/ui/issues/issue-85461.rs27
5 files changed, 97 insertions, 2 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
index ef11e2972ea..96b278dbe32 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
@@ -212,8 +212,8 @@ fn declare_unused_fn(cx: &CodegenCx<'ll, 'tcx>, def_id: &DefId) -> Instance<'tcx
         ),
     );
 
-    llvm::set_linkage(llfn, llvm::Linkage::WeakAnyLinkage);
-    llvm::set_visibility(llfn, llvm::Visibility::Hidden);
+    llvm::set_linkage(llfn, llvm::Linkage::PrivateLinkage);
+    llvm::set_visibility(llfn, llvm::Visibility::Default);
 
     assert!(cx.instances.borrow_mut().insert(instance, llfn).is_none());
 
diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-85461.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-85461.txt
new file mode 100644
index 00000000000..2831e9b532a
--- /dev/null
+++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-85461.txt
@@ -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|       |}
+
diff --git a/src/test/run-make-fulldeps/coverage/issue-85461.rs b/src/test/run-make-fulldeps/coverage/issue-85461.rs
new file mode 100644
index 00000000000..a1b9ebb1ed3
--- /dev/null
+++ b/src/test/run-make-fulldeps/coverage/issue-85461.rs
@@ -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();
+}
diff --git a/src/test/run-make-fulldeps/coverage/lib/inline_always_with_dead_code.rs b/src/test/run-make-fulldeps/coverage/lib/inline_always_with_dead_code.rs
new file mode 100644
index 00000000000..b567916aea0
--- /dev/null
+++ b/src/test/run-make-fulldeps/coverage/lib/inline_always_with_dead_code.rs
@@ -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();
+    }
+}
diff --git a/src/test/ui/issues/issue-85461.rs b/src/test/ui/issues/issue-85461.rs
new file mode 100644
index 00000000000..4c6c83f2612
--- /dev/null
+++ b/src/test/ui/issues/issue-85461.rs
@@ -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();
+    }
+}