[CFI] Propagate GUIDs correctly after PR #184065

llvm/include/llvm/IR/AutoUpgrade.h

@@ -71,6 +71,9 @@ namespace llvm {
   /// old retain release marker to new module flag format.
   LLVM_ABI void UpgradeARCRuntime(Module &M);
 
+  /// Add GUID field of the CFI metadata.
+  LLVM_ABI void UpgradeCFIFunctions(Module &M);
+
   LLVM_ABI void UpgradeSectionAttributes(Module &M);
 
   /// Correct any IR that is relying on old function attribute behavior.

llvm/include/llvm/IR/GlobalValue.h

@@ -638,6 +638,10 @@ class GlobalValue : public Constant {
   /// that might pre-date the storage of GUIDs in metadata.
   GUID getGUIDOrFallback() const;
 
+  /// same as getGUIDOrFallback, but for fallback we dropLLVMManglingEscape the
+  /// name first.
+  GUID getGUIDOrFallbackDropEscape() const;
+
   /// @name Materialization
   /// Materialization is used to construct functions only as they're needed.
   /// This

llvm/include/llvm/IR/ModuleSummaryIndex.h

@@ -23,6 +23,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/GlobalValue.h"
@@ -1333,12 +1334,24 @@ struct TypeIdSummary {
 };
 
 class CfiFunctionIndex {
-  DenseMap<GlobalValue::GUID, std::set<std::string, std::less<>>> Index;
+  DenseMap<GlobalValue::GUID, std::set<std::string, std::less<>>>
+      ThinLTOToNameIndex;
+  StringSet<> Names;
+
   using IndexIterator =
       DenseMap<GlobalValue::GUID,
                std::set<std::string, std::less<>>>::const_iterator;
   using NestedIterator = std::set<std::string, std::less<>>::const_iterator;
 
+  template <typename It> CfiFunctionIndex(It B, It E) {
+    for (; B != E; ++B) {
+      StringRef S(*B);
+      GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+          GlobalValue::dropLLVMManglingEscape(S));
+      ThinLTOToNameIndex[GUID].emplace(S);
+      Names.insert(S);
+    }
+  }
 public:
   // Iterates keys of the DenseMap.
   class GUIDIterator : public iterator_adaptor_base<GUIDIterator, IndexIterator,
@@ -1354,50 +1367,45 @@ class CfiFunctionIndex {
   };
 
   CfiFunctionIndex() = default;
-  template <typename It> CfiFunctionIndex(It B, It E) {
-    for (; B != E; ++B)
-      emplace(*B);
+
+  template <typename It>
+  static CfiFunctionIndex createForTest(It B, It E) {
+    return CfiFunctionIndex(B, E);
   }
 
   std::vector<StringRef> symbols() const {
     std::vector<StringRef> Symbols;
-    for (auto &[GUID, Syms] : Index) {
-      (void)GUID;
-      llvm::append_range(Symbols, Syms);
+    for (auto &[_, Syms] : Names) {
+      Symbols.emplace_back(Names);
     }
     return Symbols;
   }
 
-  GUIDIterator guid_begin() const { return GUIDIterator(Index.begin()); }
-  GUIDIterator guid_end() const { return GUIDIterator(Index.end()); }
-  iterator_range<GUIDIterator> guids() const {
-    return make_range(guid_begin(), guid_end());
+  auto getExportedGuids() const {
+    return map_range(ThinLTOToNameIndex, [](auto I) { return I.first; });
   }
 
-  iterator_range<NestedIterator> forGuid(GlobalValue::GUID GUID) const {
-    auto I = Index.find(GUID);
-    if (I == Index.end())
+  iterator_range<NestedIterator>
+  getMatchingNamesForThinLTOGUID(GlobalValue::GUID GUID) const {
+    auto I = ThinLTOToNameIndex.find(GUID);
+    if (I == ThinLTOToNameIndex.end())
       return make_range(NestedIterator{}, NestedIterator{});
     return make_range(I->second.begin(), I->second.end());
   }
 
-  template <typename... Args> void emplace(Args &&...A) {
-    StringRef S(std::forward<Args>(A)...);
-    GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
-        GlobalValue::dropLLVMManglingEscape(S));
-    Index[GUID].emplace(S);
+  void addSymbolWithThinLTOGUID(StringRef S, GlobalValue::GUID GUID) {
+    ThinLTOToNameIndex[GUID].emplace(S);
+    Names.insert(S);
   }
 
-  size_t count(StringRef S) const {
-    GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
-        GlobalValue::dropLLVMManglingEscape(S));
-    auto I = Index.find(GUID);
-    if (I == Index.end())
-      return 0;
-    return I->second.count(S);
+  bool contains(StringRef Name) const {
+    return Names.find(Name) != Names.end();
   }
 
-  bool empty() const { return Index.empty(); }
+  bool empty() const {
+    assert(Names.empty() == ThinLTOToNameIndex.empty());
+    return ThinLTOToNameIndex.empty();
+  }
 };
 
 /// 160 bits SHA1

llvm/include/llvm/IR/ModuleSummaryIndexYAML.h

@@ -361,11 +361,12 @@ template <> struct MappingTraits<ModuleSummaryIndex> {
     } else {
       std::vector<std::string> CfiFunctionDefs;
       io.mapOptional("CfiFunctionDefs", CfiFunctionDefs);
-      index.CfiFunctionDefs = {CfiFunctionDefs.begin(), CfiFunctionDefs.end()};
+      index.CfiFunctionDefs = CfiFunctionIndex::createForTest(
+          CfiFunctionDefs.begin(), CfiFunctionDefs.end());
       std::vector<std::string> CfiFunctionDecls;
       io.mapOptional("CfiFunctionDecls", CfiFunctionDecls);
-      index.CfiFunctionDecls = {CfiFunctionDecls.begin(),
-                                CfiFunctionDecls.end()};
+      index.CfiFunctionDecls = CfiFunctionIndex::createForTest(
+          CfiFunctionDecls.begin(), CfiFunctionDecls.end());
     }
   }
 };

llvm/lib/AsmParser/LLParser.cpp

@@ -477,6 +477,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
 
   UpgradeModuleFlags(*M);
   UpgradeNVVMAnnotations(*M);
+  UpgradeCFIFunctions(*M);
   UpgradeSectionAttributes(*M);
   copyModuleAttrToFunctions(*M);
 

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

@@ -7237,6 +7237,7 @@ Error BitcodeReader::materializeModule() {
   UpgradeNVVMAnnotations(*TheModule);
 
   UpgradeARCRuntime(*TheModule);
+  UpgradeCFIFunctions(*TheModule);
 
   copyModuleAttrToFunctions(*TheModule);
 
@@ -8189,17 +8190,23 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
 
     case bitc::FS_CFI_FUNCTION_DEFS: {
       auto &CfiFunctionDefs = TheIndex.cfiFunctionDefs();
-      for (unsigned I = 0; I != Record.size(); I += 2)
-        CfiFunctionDefs.emplace(Strtab.data() + Record[I],
-                                static_cast<size_t>(Record[I + 1]));
+      for (unsigned I = 0; I != Record.size(); I += 3) {
+        uint64_t ThinLTOGUID = Record[I];
+        StringRef Name(Strtab.data() + Record[I + 1],
+                       static_cast<size_t>(Record[I + 2]));
+        CfiFunctionDefs.addSymbolWithThinLTOGUID(Name, ThinLTOGUID);
+      }
       break;
     }
 
     case bitc::FS_CFI_FUNCTION_DECLS: {
       auto &CfiFunctionDecls = TheIndex.cfiFunctionDecls();
-      for (unsigned I = 0; I != Record.size(); I += 2)
-        CfiFunctionDecls.emplace(Strtab.data() + Record[I],
-                                 static_cast<size_t>(Record[I + 1]));
+      for (unsigned I = 0; I != Record.size(); I += 3) {
+        uint64_t ThinLTOGUID = Record[I];
+        StringRef Name(Strtab.data() + Record[I + 1],
+                       static_cast<size_t>(Record[I + 2]));
+        CfiFunctionDecls.addSymbolWithThinLTOGUID(Name, ThinLTOGUID);
+      }
       break;
     }
 

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -5360,21 +5360,28 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
         getReferencedTypeIds(FS, ReferencedTypeIds);
   }
 
-  SmallVector<StringRef, 4> Functions;
+  struct CfiFunction {
+    GlobalValue::GUID GUID = 0;
+    StringRef Name;
+    bool operator<(const CfiFunction &RHS) const { return Name < RHS.Name; }
+  };
+  std::vector<CfiFunction> Functions;
   auto EmitCfiFunctions = [&](const CfiFunctionIndex &CfiIndex,
                               bitc::GlobalValueSummarySymtabCodes Code) {
     if (CfiIndex.empty())
       return;
     for (GlobalValue::GUID GUID : DefOrUseGUIDs) {
-      auto Defs = CfiIndex.forGuid(GUID);
-      llvm::append_range(Functions, Defs);
+      for (StringRef Name : CfiIndex.getMatchingNamesForThinLTOGUID(GUID)) {
+        Functions.push_back({GUID, Name});
+      }
     }
     if (Functions.empty())
       return;
     llvm::sort(Functions);
-    for (const auto &S : Functions) {
-      NameVals.push_back(StrtabBuilder.add(S));
-      NameVals.push_back(S.size());
+    for (const auto &F : Functions) {
+      NameVals.push_back(F.GUID);
+      NameVals.push_back(StrtabBuilder.add(F.Name));
+      NameVals.push_back(F.Name.size());
     }
     Stream.EmitRecord(Code, NameVals);
     NameVals.clear();

llvm/lib/IR/AutoUpgrade.cpp

@@ -6045,6 +6045,30 @@ static bool upgradeRetainReleaseMarker(Module &M) {
   return Changed;
 }
 
+void llvm::UpgradeCFIFunctions(Module &M) {
+  if (NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions")) {
+    LLVMContext &C = M.getContext();
+    for (unsigned I = 0, E = CfiFunctionsMD->getNumOperands(); I != E; ++I) {
+      MDNode *FuncMD = CfiFunctionsMD->getOperand(I);
+      if (FuncMD->getNumOperands() >= 3 &&
+          isa<ConstantAsMetadata>(FuncMD->getOperand(2)))
+        continue;
+
+      auto *Name = cast<MDString>(FuncMD->getOperand(0));
+      StringRef FunctionName = Name->getString();
+      const GlobalValue::GUID GUID =
+          GlobalValue::getGUIDAssumingExternalLinkage(
+              GlobalValue::dropLLVMManglingEscape(FunctionName));
+
+      SmallVector<Metadata *, 4> Ops(FuncMD->op_begin(), FuncMD->op_end());
+      Ops.insert(Ops.begin() + 2, ConstantAsMetadata::get(ConstantInt::get(
+                                      Type::getInt64Ty(C), GUID)));
+
+      CfiFunctionsMD->setOperand(I, MDTuple::get(C, Ops));
+    }
+  }
+}
+
 void llvm::UpgradeARCRuntime(Module &M) {
   // This lambda converts normal function calls to ARC runtime functions to
   // intrinsic calls.

llvm/lib/IR/Globals.cpp

@@ -106,6 +106,13 @@ GlobalValue::GUID GlobalValue::getGUIDOrFallback() const {
   return getGUIDAssumingExternalLinkage(getGlobalIdentifier());
 }
 
+GlobalValue::GUID GlobalValue::getGUIDOrFallbackDropEscape() const {
+  auto GuidIfPresent = getGUIDIfAssigned();
+  return GuidIfPresent ? *GuidIfPresent
+                       : GlobalValue::getGUIDAssumingExternalLinkage(
+                             GlobalValue::dropLLVMManglingEscape(getName()));
+}
+
 std::optional<GlobalValue::GUID> GlobalValue::getGUIDIfAssigned() const {
   // First check the metadata.
   auto *MD = getGUIDMetadata();

llvm/lib/LTO/LTO.cpp

@@ -1596,9 +1596,9 @@ class CGThinBackend : public ThinBackendProc {
                         OnWrite, ShouldEmitImportsFiles, ThinLTOParallelism),
         ShouldEmitIndexFiles(ShouldEmitIndexFiles) {
     auto &Defs = CombinedIndex.cfiFunctionDefs();
-    CfiFunctionDefs.insert_range(Defs.guids());
+    CfiFunctionDefs.insert_range(Defs.getExportedGuids());
     auto &Decls = CombinedIndex.cfiFunctionDecls();
-    CfiFunctionDecls.insert_range(Decls.guids());
+    CfiFunctionDecls.insert_range(Decls.getExportedGuids());
   }
 };
 
@@ -2171,9 +2171,9 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
   // Any functions referenced by the jump table in the regular LTO object must
   // be exported.
   auto &Defs = ThinLTO.CombinedIndex.cfiFunctionDefs();
-  ExportedGUIDs.insert(Defs.guid_begin(), Defs.guid_end());
+  ExportedGUIDs.insert(Defs.getExportedGuids().begin(), Defs.getExportedGuids().end());
   auto &Decls = ThinLTO.CombinedIndex.cfiFunctionDecls();
-  ExportedGUIDs.insert(Decls.guid_begin(), Decls.guid_end());
+  ExportedGUIDs.insert(Decls.getExportedGuids().begin(), Decls.getExportedGuids().end());
 
   auto isExported = [&](StringRef ModuleIdentifier, ValueInfo VI) {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);

llvm/lib/Transforms/IPO/CrossDSOCFI.cpp

@@ -73,8 +73,8 @@ void CrossDSOCFI::buildCFICheck(Module &M) {
   NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions");
   if (CfiFunctionsMD) {
     for (auto *Func : CfiFunctionsMD->operands()) {
-      assert(Func->getNumOperands() >= 2);
-      for (unsigned I = 2; I < Func->getNumOperands(); ++I)
+      assert(Func->getNumOperands() >= 3);
+      for (unsigned I = 3; I < Func->getNumOperands(); ++I)
         if (ConstantInt *TypeId =
                 extractNumericTypeId(cast<MDNode>(Func->getOperand(I).get())))
           TypeIds.insert(TypeId->getZExtValue());

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

@@ -1788,10 +1788,13 @@ void LowerTypeTestsModule::buildBitSetsFromFunctionsNative(
     }
 
     if (IsExported) {
+      GlobalValue::GUID GUID = F->getGUIDOrFallbackDropEscape();
       if (IsJumpTableCanonical)
-        ExportSummary->cfiFunctionDefs().emplace(F->getName());
+        ExportSummary->cfiFunctionDefs().addSymbolWithThinLTOGUID(F->getName(),
+                                                                  GUID);
       else
-        ExportSummary->cfiFunctionDecls().emplace(F->getName());
+        ExportSummary->cfiFunctionDecls().addSymbolWithThinLTOGUID(F->getName(),
+                                                                   GUID);
     }
 
     if (!IsJumpTableCanonical) {
@@ -2123,9 +2126,9 @@ bool LowerTypeTestsModule::lower() {
       // have the same name, but it's not the one we are looking for.
       if (F.hasLocalLinkage())
         continue;
-      if (ImportSummary->cfiFunctionDefs().count(F.getName()))
+      if (ImportSummary->cfiFunctionDefs().contains(F.getName()))
         Defs.push_back(&F);
-      else if (ImportSummary->cfiFunctionDecls().count(F.getName()))
+      else if (ImportSummary->cfiFunctionDecls().contains(F.getName()))
         Decls.push_back(&F);
     }
 
@@ -2169,7 +2172,7 @@ bool LowerTypeTestsModule::lower() {
 
   struct ExportedFunctionInfo {
     CfiFunctionLinkage Linkage;
-    MDNode *FuncMD; // {name, linkage, type[, type...]}
+    MDNode *FuncMD; // {name, linkage, stable_GUID, type[, type...]}
   };
   MapVector<StringRef, ExportedFunctionInfo> ExportedFunctions;
   if (ExportSummary) {
@@ -2207,9 +2210,14 @@ bool LowerTypeTestsModule::lower() {
                 ->getValue()
                 ->getUniqueInteger()
                 .getZExtValue());
-        const GlobalValue::GUID GUID =
-            GlobalValue::getGUIDAssumingExternalLinkage(
-                GlobalValue::dropLLVMManglingEscape(FunctionName));
+        // Use the stable GUID stored in !cfi.functions (element 2).
+        assert(FuncMD->getNumOperands() >= 3 &&
+               isa<ConstantAsMetadata>(FuncMD->getOperand(2)) &&
+               "GUID metadata missing");
+        GlobalValue::GUID GUID = cast<ConstantAsMetadata>(FuncMD->getOperand(2))
+                                     ->getValue()
+                                     ->getUniqueInteger()
+                                     .getZExtValue();
         // Do not emit jumptable entries for functions that are not-live and
         // have no live references (and are not exported with cross-DSO CFI.)
         if (!ExportSummary->isGUIDLive(GUID))
@@ -2248,11 +2256,15 @@ bool LowerTypeTestsModule::lower() {
           F = nullptr;
         }
 
-        if (!F)
+        if (!F) {
           F = Function::Create(
               FunctionType::get(Type::getVoidTy(M.getContext()), false),
               GlobalVariable::ExternalLinkage,
               M.getDataLayout().getProgramAddressSpace(), FunctionName, &M);
+          F->setMetadata(
+              LLVMContext::MD_unique_id,
+              MDTuple::get(M.getContext(), {FuncMD->getOperand(2).get()}));
+        }
 
         // If the function is available_externally, remove its definition so
         // that it is handled the same way as a declaration. Later we will try
@@ -2280,7 +2292,12 @@ bool LowerTypeTestsModule::lower() {
             F->setLinkage(GlobalValue::ExternalWeakLinkage);
 
           F->eraseMetadata(LLVMContext::MD_type);
-          for (unsigned I = 2; I < FuncMD->getNumOperands(); ++I)
+          // Type metadata starts at operand 3 (operand 2 is the stable GUID).
+          assert(FuncMD->getNumOperands() >= 3 &&
+                 isa<ConstantAsMetadata>(FuncMD->getOperand(2)) &&
+                 "GUID metadata missing");
+          unsigned TypesStart = 3;
+          for (unsigned I = TypesStart; I < FuncMD->getNumOperands(); ++I)
             F->addMetadata(LLVMContext::MD_type,
                            *cast<MDNode>(FuncMD->getOperand(I).get()));
         }