From 683bdb08a094dbb31bac7388af44c3e515f98ba8 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek@google.com>
Date: Tue, 31 Aug 2021 23:10:30 -0700
Subject: [PATCH] [Linker] Support weak symbols in nodeduplicate COMDAT group

When a nodeduplicate COMDAT group contains a weak symbol, choose
a non-weak symbol (or one of the weak ones) rather than reporting
an error. This should address issue PR51394.

With the current IR representation, a generic comdat nodeduplicate
semantics is not representable for LTO. In the linker, sections and
symbols are separate concepts. A dropped weak symbol does not force the
defining input section to be dropped as well (though it can be collected
by GC). In the IR, when a weak linkage symbol is dropped, its associate
section content is dropped as well.

For InstrProfiling, which is where ran into this issue in PR51394, the
deduplication semantic is a sufficient workaround.

Differential Revision: https://reviews.llvm.org/D108689
---
 lib/Linker/LinkModules.cpp | 22 +++++++++++++++++++---
 test/Linker/comdat4.ll     | 11 ++++++++++-
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp
index 97d6f8cd807..efdbc49cdf4 100644
--- a/lib/Linker/LinkModules.cpp
+++ b/lib/Linker/LinkModules.cpp
@@ -177,9 +177,25 @@ bool ModuleLinker::computeResultingSelectionKind(StringRef ComdatName,
     // Go with Dst.
     LinkFromSrc = false;
     break;
-  case Comdat::SelectionKind::NoDeduplicate:
-    return emitError("Linking COMDATs named '" + ComdatName +
-                     "': nodeduplicate has been violated!");
+  case Comdat::SelectionKind::NoDeduplicate: {
+    const GlobalVariable *DstGV;
+    const GlobalVariable *SrcGV;
+    if (getComdatLeader(DstM, ComdatName, DstGV) ||
+        getComdatLeader(*SrcM, ComdatName, SrcGV))
+      return true;
+
+    if (SrcGV->isWeakForLinker()) {
+      // Go with Dst.
+      LinkFromSrc = false;
+    } else if (DstGV->isWeakForLinker()) {
+      // Go with Src.
+      LinkFromSrc = true;
+    } else {
+      return emitError("Linking COMDATs named '" + ComdatName +
+                       "': nodeduplicate has been violated!");
+    }
+    break;
+  }
   case Comdat::SelectionKind::ExactMatch:
   case Comdat::SelectionKind::Largest:
   case Comdat::SelectionKind::SameSize: {
diff --git a/test/Linker/comdat4.ll b/test/Linker/comdat4.ll
index c0af43a516b..69e1d6efe75 100644
--- a/test/Linker/comdat4.ll
+++ b/test/Linker/comdat4.ll
@@ -1,5 +1,14 @@
-; RUN: not llvm-link %s %p/Inputs/comdat3.ll -S -o - 2>&1 | FileCheck %s
+; RUN: split-file %s %t.dir
+; RUN: not llvm-link %t.dir/global.ll %t.dir/global.ll -S -o - 2>&1 | FileCheck %s
+; RUN: llvm-link %t.dir/global.ll %t.dir/weak.ll -S -o - 2>&1
+; RUN: llvm-link %t.dir/weak.ll %t.dir/global.ll -S -o - 2>&1
+; RUN: llvm-link %t.dir/weak.ll %t.dir/weak.ll -S -o - 2>&1
 
+;--- global.ll
 $foo = comdat nodeduplicate
 @foo = global i64 43, comdat($foo)
 ; CHECK: Linking COMDATs named 'foo': nodeduplicate has been violated!
+
+;--- weak.ll
+$foo = comdat nodeduplicate
+@foo = weak global i64 43, comdat($foo)