Fix mapping of unmaterialized global values during metadata linking
[oota-llvm.git] / lib / Linker / LinkModules.cpp
index b7aab9e964420cb4af2e23a26d2bbaf1dfc5207d..5419c22fbf7b69dea916910eccf7cb98eb8bc4fb 100644 (file)
@@ -421,9 +421,6 @@ class ModuleLinker {
   // Vector of GlobalValues to lazily link in.
   std::vector<GlobalValue *> LazilyLinkGlobalValues;
 
-  /// Functions that have replaced other functions.
-  SmallPtrSet<const Function *, 16> OverridingFunctions;
-
   DiagnosticHandlerFunction DiagnosticHandler;
 
   /// For symbol clashes, prefer those from Src.
@@ -443,6 +440,11 @@ class ModuleLinker {
   /// as part of a different backend compilation process.
   bool HasExportedFunctions;
 
+  /// Set to true when all global value body linking is complete (including
+  /// lazy linking). Used to prevent metadata linking from creating new
+  /// references.
+  bool DoneLinkingBodies;
+
 public:
   ModuleLinker(Module *dstM, Linker::IdentifiedStructTypeSet &Set, Module *srcM,
                DiagnosticHandlerFunction DiagnosticHandler, unsigned Flags,
@@ -451,7 +453,8 @@ public:
       : DstM(dstM), SrcM(srcM), TypeMap(Set),
         ValMaterializer(TypeMap, DstM, LazilyLinkGlobalValues, this),
         DiagnosticHandler(DiagnosticHandler), Flags(Flags), ImportIndex(Index),
-        ImportFunction(FuncToImport), HasExportedFunctions(false) {
+        ImportFunction(FuncToImport), HasExportedFunctions(false),
+        DoneLinkingBodies(false) {
     assert((ImportIndex || !ImportFunction) &&
            "Expect a FunctionInfoIndex when importing");
     // If we have a FunctionInfoIndex but no function to import,
@@ -478,6 +481,9 @@ public:
   /// Check if we should promote the given local value to global scope.
   bool doPromoteLocalToGlobal(const GlobalValue *SGV);
 
+  /// Check if all global value body linking is complete.
+  bool doneLinkingBodies() { return DoneLinkingBodies; }
+
 private:
   bool shouldLinkFromSource(bool &LinkFromSrc, const GlobalValue &Dest,
                             const GlobalValue &Src);
@@ -503,6 +509,8 @@ private:
       ComdatsChosen;
   bool getComdatResult(const Comdat *SrcC, Comdat::SelectionKind &SK,
                        bool &LinkFromSrc);
+  // Keep track of the global value members of each comdat in source.
+  DenseMap<const Comdat *, std::vector<GlobalValue *>> ComdatMembers;
 
   /// Given a global in the source module, return the global in the
   /// destination module that is being linked to, if any.
@@ -581,7 +589,6 @@ private:
                      const GlobalValue *DGV = nullptr);
 
   void linkNamedMDNodes();
-  void stripReplacedSubprograms();
 };
 }
 
@@ -621,9 +628,7 @@ void ModuleLinker::copyGVAttributes(GlobalValue *NewGV,
   // being imported as a declaration. In that case copy the attributes from the
   // base object.
   if (GA && !dyn_cast<GlobalAlias>(NewGV)) {
-    assert(isPerformingImport() &&
-           (GA->hasWeakAnyLinkage() ||
-            !doImportAsDefinition(GA->getBaseObject())));
+    assert(isPerformingImport() && !doImportAsDefinition(GA));
     NewGV->copyAttributesFrom(GA->getBaseObject());
   } else
     NewGV->copyAttributesFrom(SrcGV);
@@ -646,12 +651,21 @@ static bool isLessConstraining(GlobalValue::VisibilityTypes a,
 bool ModuleLinker::doImportAsDefinition(const GlobalValue *SGV) {
   if (!isPerformingImport())
     return false;
-  // Always import GlobalVariable definitions. The linkage changes
+  auto *GA = dyn_cast<GlobalAlias>(SGV);
+  if (GA) {
+    if (GA->hasWeakAnyLinkage())
+      return false;
+    return doImportAsDefinition(GA->getBaseObject());
+  }
+  // Always import GlobalVariable definitions, except for the special
+  // case of WeakAny which are imported as ExternalWeak declarations
+  // (see comments in ModuleLinker::getLinkage). The linkage changes
   // described in ModuleLinker::getLinkage ensure the correct behavior (e.g.
   // global variables with external linkage are transformed to
-  // available_externally defintions, which are ultimately turned into
-  // declaratios after the EliminateAvailableExternally pass).
-  if (dyn_cast<GlobalVariable>(SGV) && !SGV->isDeclaration())
+  // available_externally definitions, which are ultimately turned into
+  // declarations after the EliminateAvailableExternally pass).
+  if (dyn_cast<GlobalVariable>(SGV) && !SGV->isDeclaration() &&
+      !SGV->hasWeakAnyLinkage())
     return true;
   // Only import the function requested for importing.
   auto *SF = dyn_cast<Function>(SGV);
@@ -720,7 +734,7 @@ GlobalValue::LinkageTypes ModuleLinker::getLinkage(const GlobalValue *SGV) {
     // definitions upon import, so that they are available for inlining
     // and/or optimization, but are turned into declarations later
     // during the EliminateAvailableExternally pass.
-    if (doImportAsDefinition(SGV))
+    if (doImportAsDefinition(SGV) && !dyn_cast<GlobalAlias>(SGV))
       return GlobalValue::AvailableExternallyLinkage;
     // An imported external declaration stays external.
     return SGV->getLinkage();
@@ -753,7 +767,7 @@ GlobalValue::LinkageTypes ModuleLinker::getLinkage(const GlobalValue *SGV) {
     // equivalent, so the issue described above for weak_any does not exist,
     // and the definition can be imported. It can be treated similarly
     // to an imported externally visible global value.
-    if (doImportAsDefinition(SGV))
+    if (doImportAsDefinition(SGV) && !dyn_cast<GlobalAlias>(SGV))
       return GlobalValue::AvailableExternallyLinkage;
     else
       return GlobalValue::ExternalLinkage;
@@ -763,14 +777,14 @@ GlobalValue::LinkageTypes ModuleLinker::getLinkage(const GlobalValue *SGV) {
     // since it would cause global constructors/destructors to be
     // executed multiple times. This should have already been handled
     // by linkGlobalValueProto.
-    assert(false && "Cannot import appending linkage variable");
+    llvm_unreachable("Cannot import appending linkage variable");
 
   case GlobalValue::InternalLinkage:
   case GlobalValue::PrivateLinkage:
     // If we are promoting the local to global scope, it is handled
     // similarly to a normal externally visible global.
     if (doPromoteLocalToGlobal(SGV)) {
-      if (doImportAsDefinition(SGV))
+      if (doImportAsDefinition(SGV) && !dyn_cast<GlobalAlias>(SGV))
         return GlobalValue::AvailableExternallyLinkage;
       else
         return GlobalValue::ExternalLinkage;
@@ -829,8 +843,7 @@ GlobalValue *ModuleLinker::copyGlobalAliasProto(TypeMapTy &TypeMap,
   // as a declaration as well, which involves converting it to a non-alias.
   // See comments in ModuleLinker::getLinkage for why we cannot import
   // weak_any defintions.
-  if (isPerformingImport() && (SGA->hasWeakAnyLinkage() ||
-                               !doImportAsDefinition(SGA->getBaseObject()))) {
+  if (isPerformingImport() && !doImportAsDefinition(SGA)) {
     // Need to convert to declaration. All aliases must be definitions.
     const GlobalValue *GVal = SGA->getBaseObject();
     GlobalValue *NewGV;
@@ -841,12 +854,12 @@ GlobalValue *ModuleLinker::copyGlobalAliasProto(TypeMapTy &TypeMap,
       assert(F);
       NewGV = copyFunctionProto(TypeMap, F);
     }
-    // Set the linkage to ExternalWeak, see also comments in
-    // ModuleLinker::getLinkage.
+    // Set the linkage to External or ExternalWeak (see comments in
+    // ModuleLinker::getLinkage for why WeakAny is converted to ExternalWeak).
     if (SGA->hasWeakAnyLinkage())
       NewGV->setLinkage(GlobalValue::ExternalWeakLinkage);
-    // Don't attempt to link body, needs to be a declaration.
-    DoNotLinkFromSource.insert(SGA);
+    else
+      NewGV->setLinkage(GlobalValue::ExternalLinkage);
     return NewGV;
   }
   // If there is no linkage to be performed or we're linking from the source,
@@ -890,6 +903,12 @@ Value *ValueMaterializerTy::materializeValueFor(Value *V) {
   if (!SGV)
     return nullptr;
 
+  // If we are done linking global value bodies (i.e. we are performing
+  // metadata linking), don't link in the global value due to this
+  // reference, simply map it to null.
+  if (ModLinker->doneLinkingBodies())
+    return nullptr;
+
   GlobalValue *DGV = ModLinker->copyGlobalValueProto(TypeMap, SGV);
 
   if (Comdat *SC = SGV->getComdat()) {
@@ -1369,6 +1388,7 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) {
     std::tie(SK, LinkFromSrc) = ComdatsChosen[SC];
     C = DstM->getOrInsertComdat(SC->getName());
     C->setSelectionKind(SK);
+    ComdatMembers[SC].push_back(SGV);
   } else if (DGV) {
     if (shouldLinkFromSource(LinkFromSrc, *DGV, *SGV))
       return true;
@@ -1415,9 +1435,8 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) {
 
     NewGV = copyGlobalValueProto(TypeMap, SGV, DGV);
 
-    if (DGV && isa<Function>(DGV))
-      if (auto *NewF = dyn_cast<Function>(NewGV))
-        OverridingFunctions.insert(NewF);
+    if (isPerformingImport() && !doImportAsDefinition(SGV))
+      DoNotLinkFromSource.insert(SGV);
   }
 
   NewGV->setUnnamedAddr(HasUnnamedAddr);
@@ -1573,6 +1592,19 @@ void ModuleLinker::linkAliasBody(GlobalAlias &Dst, GlobalAlias &Src) {
 bool ModuleLinker::linkGlobalValueBody(GlobalValue &Src) {
   Value *Dst = ValueMap[&Src];
   assert(Dst);
+  if (const Comdat *SC = Src.getComdat()) {
+    // To ensure that we don't generate an incomplete comdat group,
+    // we must materialize and map in any other members that are not
+    // yet materialized in Dst, which also ensures their definitions
+    // are linked in. Otherwise, linkonce and other lazy linked GVs will
+    // not be materialized if they aren't referenced.
+    for (auto *SGV : ComdatMembers[SC]) {
+      if (ValueMap[SGV])
+        continue;
+      Value *NewV = ValMaterializer.materializeValueFor(SGV);
+      ValueMap[SGV] = NewV;
+    }
+  }
   if (shouldInternalizeLinkedSymbols())
     if (auto *DGV = dyn_cast<GlobalValue>(Dst))
       DGV->setLinkage(GlobalValue::InternalLinkage);
@@ -1596,43 +1628,9 @@ void ModuleLinker::linkNamedMDNodes() {
     NamedMDNode *DestNMD = DstM->getOrInsertNamedMetadata(NMD.getName());
     // Add Src elements into Dest node.
     for (const MDNode *op : NMD.operands())
-      DestNMD->addOperand(MapMetadata(op, ValueMap, RF_MoveDistinctMDs,
-                                      &TypeMap, &ValMaterializer));
-  }
-}
-
-/// Drop DISubprograms that have been superseded.
-///
-/// FIXME: this creates an asymmetric result: we strip functions from losing
-/// subprograms in DstM, but leave losing subprograms in SrcM.
-/// TODO: Remove this logic once the backend can correctly determine canonical
-/// subprograms.
-void ModuleLinker::stripReplacedSubprograms() {
-  // Avoid quadratic runtime by returning early when there's nothing to do.
-  if (OverridingFunctions.empty())
-    return;
-
-  // Move the functions now, so the set gets cleared even on early returns.
-  auto Functions = std::move(OverridingFunctions);
-  OverridingFunctions.clear();
-
-  // Drop functions from subprograms if they've been overridden by the new
-  // compile unit.
-  NamedMDNode *CompileUnits = DstM->getNamedMetadata("llvm.dbg.cu");
-  if (!CompileUnits)
-    return;
-  for (unsigned I = 0, E = CompileUnits->getNumOperands(); I != E; ++I) {
-    auto *CU = cast<DICompileUnit>(CompileUnits->getOperand(I));
-    assert(CU && "Expected valid compile unit");
-
-    for (DISubprogram *SP : CU->getSubprograms()) {
-      if (!SP || !SP->getFunction() || !Functions.count(SP->getFunction()))
-        continue;
-
-      // Prevent DebugInfoFinder from tagging this as the canonical subprogram,
-      // since the canonical one is in the incoming module.
-      SP->replaceFunction(nullptr);
-    }
+      DestNMD->addOperand(MapMetadata(
+          op, ValueMap, RF_MoveDistinctMDs | RF_NullMapMissingGlobalValues,
+          &TypeMap, &ValMaterializer));
   }
 }
 
@@ -1907,13 +1905,6 @@ bool ModuleLinker::run() {
       MapValue(GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer);
   }
 
-  // Strip replaced subprograms before mapping any metadata -- so that we're
-  // not changing metadata from the source module (note that
-  // linkGlobalValueBody() eventually calls RemapInstruction() and therefore
-  // MapMetadata()) -- but after linking global value protocols -- so that
-  // OverridingFunctions has been built.
-  stripReplacedSubprograms();
-
   // Link in the function bodies that are defined in the source module into
   // DstM.
   for (Function &SF : *SrcM) {
@@ -1925,10 +1916,6 @@ bool ModuleLinker::run() {
     if (DoNotLinkFromSource.count(&SF))
       continue;
 
-    // When importing, only materialize the function requested for import.
-    if (isPerformingImport() && &SF != ImportFunction)
-      continue;
-
     if (linkGlobalValueBody(SF))
       return true;
   }
@@ -1940,15 +1927,6 @@ bool ModuleLinker::run() {
     linkGlobalValueBody(Src);
   }
 
-  // Remap all of the named MDNodes in Src into the DstM module. We do this
-  // after linking GlobalValues so that MDNodes that reference GlobalValues
-  // are properly remapped.
-  linkNamedMDNodes();
-
-  // Merge the module flags into the DstM module.
-  if (linkModuleFlagsMetadata())
-    return true;
-
   // Update the initializers in the DstM module now that all globals that may
   // be referenced are in DstM.
   for (GlobalVariable &Src : SrcM->globals()) {
@@ -1975,6 +1953,19 @@ bool ModuleLinker::run() {
       return true;
   }
 
+  // Note that we are done linking global value bodies. This prevents
+  // metadata linking from creating new references.
+  DoneLinkingBodies = true;
+
+  // Remap all of the named MDNodes in Src into the DstM module. We do this
+  // after linking GlobalValues so that MDNodes that reference GlobalValues
+  // are properly remapped.
+  linkNamedMDNodes();
+
+  // Merge the module flags into the DstM module.
+  if (linkModuleFlagsMetadata())
+    return true;
+
   return false;
 }