[PATCH] D31735: ThinLTOBitcodeWriter: delete comdats if their keys are renamed

[Bob Haarman via Phabricator via llvm-commits]

inglorion created this revision.
Herald added a subscriber: Prazek.

COFF requires that every comdat contain a symbol with the same name as the comdat. ThinLTOBitcodeWriter renames symbols, which may cause this requirement to be violated. This change avoids such violations by deleting the affected comdats.


https://reviews.llvm.org/D31735

Files:
  lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  test/Transforms/ThinLTOBitcodeWriter/comdat.ll


Index: test/Transforms/ThinLTOBitcodeWriter/comdat.ll
===================================================================
--- /dev/null
+++ test/Transforms/ThinLTOBitcodeWriter/comdat.ll
@@ -0,0 +1,25 @@
+; RUN: opt -thinlto-bc -o %t %s
+; RUN: llvm-modextract -n 0 -o - %t | llvm-dis | FileCheck --check-prefix=CHECK0 %s
+; RUN: llvm-modextract -n 1 -o - %t | llvm-dis | FileCheck --check-prefix=CHECK1 %s
+
+; foo should have been renamed, and there should no longer be a comdat
+; with the original name.
+; CHECK0: {{@"?foo[^ ]+}} = external hidden global
+; CHECK1-NOT: $foo = comdat
+; CHECK1: {{@"?foo[^ ]+}} = hidden
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.0.24215"
+
+$foo = comdat any
+
+ at anon = private unnamed_addr constant { [1 x i8*] } { [1 x i8*] [i8* null] }, comdat($foo), !type !0
+
+ at foo = internal unnamed_addr alias i8*, getelementptr inbounds ({ [1 x i8*] }, { [1 x i8*] }* @anon, i32 0, i32 0, i32 1)
+
+define i8* @bar() {
+  %1 = load i8*, i8** @foo
+  ret i8* %1
+}
+
+!0 = !{i64 8, !"?AVA@@"}
Index: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
===================================================================
--- lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -69,23 +69,54 @@
 // Promote each local-linkage entity defined by ExportM and used by ImportM by
 // changing visibility and appending the given ModuleId.
 void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId) {
+  DenseMap<StringRef, SmallVector<GlobalObject *, 1>> ComdatMap;
+  DenseSet<StringRef> ComdatsToDelete;
   for (auto &ExportGV : ExportM.global_values()) {
+    Comdat *C = ExportGV.getComdat();
+    if (C) {
+      if (auto *GO = dyn_cast<GlobalObject>(&ExportGV)) {
+        if (ComdatsToDelete.count(C->getName()))
+          GO->setComdat(nullptr);
+        else if (C->getName() != ExportGV.getName())
+          ComdatMap[C->getName()].push_back(GO);
+      }
+    }
+
     if (!ExportGV.hasLocalLinkage())
       continue;
 
-    GlobalValue *ImportGV = ImportM.getNamedValue(ExportGV.getName());
+    StringRef Name = ExportGV.getName();
+    GlobalValue *ImportGV = ImportM.getNamedValue(Name);
     if (!ImportGV || ImportGV->use_empty())
       continue;
 
-    std::string NewName = (ExportGV.getName() + ModuleId).str();
+    std::string NewName = (Name + ModuleId).str();
+
+    // COFF requires that every COMDAT contain a symbol with the same name
+    // as the COMDAT. If renaming this value would break that requirement,
+    // delete the COMDAT. This is safe, because this COMDAT is not going to
+    // be merged anyway.
+    if (C && C->getName() == Name) {
+      if (auto *GO = dyn_cast<GlobalObject>(&ExportGV))
+        GO->setComdat(nullptr);
+      ComdatsToDelete.insert(Name);
+      auto I = ComdatMap.find(Name);
+      if (I != ComdatMap.end())
+        for (auto &O : I->second)
+          O->setComdat(nullptr);
+    }
 
     ExportGV.setName(NewName);
     ExportGV.setLinkage(GlobalValue::ExternalLinkage);
     ExportGV.setVisibility(GlobalValue::HiddenVisibility);
 
     ImportGV->setName(NewName);
     ImportGV->setVisibility(GlobalValue::HiddenVisibility);
   }
+
+  auto &Comdats = ExportM.getComdatSymbolTable();
+  for (auto C : ComdatsToDelete)
+    Comdats.erase(C);
 }
 
 // Promote all internal (i.e. distinct) type ids used by the module by replacing


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31735.94304.patch
Type: text/x-patch
Size: 3443 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170405/4e6c02ea/attachment.bin>