[lld] [llvm] [LLD][MachO] make InterfaceFile::symbols iteration order deterministic (PR #97615)

via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 4 04:36:18 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lld-macho

Author: Tim Gymnich (tgymnich)

<details>
<summary>Changes</summary>

make InterfaceFile::symbols iteration order deterministic using `std::map`

fixes #<!-- -->97127

---
Full diff: https://github.com/llvm/llvm-project/pull/97615.diff


3 Files Affected:

- (modified) lld/MachO/InputFiles.cpp (+3-9) 
- (modified) llvm/include/llvm/TextAPI/SymbolSet.h (+14-1) 
- (modified) llvm/lib/TextAPI/SymbolSet.cpp (+15-7) 


``````````diff
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 0cee1a84d0b55a..7823e00718467c 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -1896,8 +1896,6 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
                                        symbol.isThreadLocalValue()));
   };
 
-  std::vector<const llvm::MachO::Symbol *> normalSymbols;
-  normalSymbols.reserve(interface.symbolsCount());
   for (const auto *symbol : interface.symbols()) {
     if (!isArchABICompatible(symbol->getArchitectures(), config->arch()))
       continue;
@@ -1909,15 +1907,11 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
     case EncodeKind::ObjectiveCClass:
     case EncodeKind::ObjectiveCClassEHType:
     case EncodeKind::ObjectiveCInstanceVariable:
-      normalSymbols.push_back(symbol);
+      break;
+    default:
+      continue;
     }
-  }
-  // interface.symbols() order is non-deterministic.
-  llvm::sort(normalSymbols,
-             [](auto *l, auto *r) { return l->getName() < r->getName(); });
 
-  // TODO(compnerd) filter out symbols based on the target platform
-  for (const auto *symbol : normalSymbols) {
     switch (symbol->getKind()) {
     case EncodeKind::GlobalSymbol:
       addSymbol(*symbol, symbol->getName());
diff --git a/llvm/include/llvm/TextAPI/SymbolSet.h b/llvm/include/llvm/TextAPI/SymbolSet.h
index 6ccabb90772087..8e38bb570f9131 100644
--- a/llvm/include/llvm/TextAPI/SymbolSet.h
+++ b/llvm/include/llvm/TextAPI/SymbolSet.h
@@ -18,6 +18,7 @@
 #include "llvm/TextAPI/Architecture.h"
 #include "llvm/TextAPI/ArchitectureSet.h"
 #include "llvm/TextAPI/Symbol.h"
+#include <map>
 #include <stddef.h>
 
 namespace llvm {
@@ -28,6 +29,18 @@ struct SymbolsMapKey {
 
   SymbolsMapKey(MachO::EncodeKind Kind, StringRef Name)
       : Kind(Kind), Name(Name) {}
+
+  bool operator==(const SymbolsMapKey &Other) const {
+    return Kind == Other.Kind && Name == Other.Name;
+  }
+
+  bool operator!=(const SymbolsMapKey &Other) const {
+    return operator==(Other);
+  }
+
+  bool operator<(const SymbolsMapKey &Other) const {
+    return std::make_pair(Kind, Name) < std::make_pair(Other.Kind, Other.Name);
+  }
 };
 template <> struct DenseMapInfo<SymbolsMapKey> {
   static inline SymbolsMapKey getEmptyKey() {
@@ -84,7 +97,7 @@ class SymbolSet {
     return StringRef(reinterpret_cast<const char *>(Ptr), String.size());
   }
 
-  using SymbolsMapType = llvm::DenseMap<SymbolsMapKey, Symbol *>;
+  using SymbolsMapType = std::map<SymbolsMapKey, Symbol *>;
   SymbolsMapType Symbols;
 
   Symbol *addGlobalImpl(EncodeKind, StringRef Name, SymbolFlags Flags);
diff --git a/llvm/lib/TextAPI/SymbolSet.cpp b/llvm/lib/TextAPI/SymbolSet.cpp
index 2e0b4160c9b468..5206a12740d11b 100644
--- a/llvm/lib/TextAPI/SymbolSet.cpp
+++ b/llvm/lib/TextAPI/SymbolSet.cpp
@@ -30,19 +30,27 @@ Symbol *SymbolSet::addGlobal(EncodeKind Kind, StringRef Name, SymbolFlags Flags,
 
 const Symbol *SymbolSet::findSymbol(EncodeKind Kind, StringRef Name,
                                     ObjCIFSymbolKind ObjCIF) const {
-  if (auto result = Symbols.lookup({Kind, Name}))
-    return result;
+  auto Iter = Symbols.find({Kind, Name});
+  if (Iter != Symbols.end())
+    return Iter->second;
   if ((ObjCIF == ObjCIFSymbolKind::None) || (ObjCIF > ObjCIFSymbolKind::EHType))
     return nullptr;
   assert(ObjCIF <= ObjCIFSymbolKind::EHType &&
          "expected single ObjCIFSymbolKind enum value");
   // Non-complete ObjC Interfaces are represented as global symbols.
-  if (ObjCIF == ObjCIFSymbolKind::Class)
-    return Symbols.lookup(
+  if (ObjCIF == ObjCIFSymbolKind::Class) {
+    auto Iter = Symbols.find(
         {EncodeKind::GlobalSymbol, (ObjC2ClassNamePrefix + Name).str()});
-  if (ObjCIF == ObjCIFSymbolKind::MetaClass)
-    return Symbols.lookup(
+    return Iter != Symbols.end() ? Iter->second : nullptr;
+  }
+
+  if (ObjCIF == ObjCIFSymbolKind::MetaClass) {
+    auto Iter = Symbols.find(
         {EncodeKind::GlobalSymbol, (ObjC2MetaClassNamePrefix + Name).str()});
-  return Symbols.lookup(
+    return Iter != Symbols.end() ? Iter->second : nullptr;
+  }
+
+  Iter = Symbols.find(
       {EncodeKind::GlobalSymbol, (ObjC2EHTypePrefix + Name).str()});
+  return Iter != Symbols.end() ? Iter->second : nullptr;
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/97615


More information about the llvm-commits mailing list