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

Tim Gymnich via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 3 12:41:29 PDT 2024


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

>From a14102af307c10c6880735eb43e9c37532419446 Mon Sep 17 00:00:00 2001
From: Tim Gymnich <tgymnich at icloud.com>
Date: Wed, 3 Jul 2024 20:26:27 +0200
Subject: [PATCH 1/2] make InterfaceFile::symbols iteration order deterministic

---
 lld/MachO/InputFiles.cpp              | 12 +++---------
 llvm/include/llvm/TextAPI/SymbolSet.h | 15 ++++++++++++++-
 llvm/lib/TextAPI/SymbolSet.cpp        | 22 +++++++++++++++-------
 3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 0cee1a84d0b55..7823e00718467 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 6ccabb9077208..41c2fcfdae074 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 Kind < Other.Kind || Name < 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 2e0b4160c9b46..5206a12740d11 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;
 }

>From 492856764ae4b0a3829efd68f24f66eeefddf5ca Mon Sep 17 00:00:00 2001
From: Tim Gymnich <tim at gymni.ch>
Date: Wed, 3 Jul 2024 21:41:22 +0200
Subject: [PATCH 2/2] Update llvm/include/llvm/TextAPI/SymbolSet.h

Co-authored-by: Ellis Hoag <ellis.sparky.hoag at gmail.com>
---
 llvm/include/llvm/TextAPI/SymbolSet.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/TextAPI/SymbolSet.h b/llvm/include/llvm/TextAPI/SymbolSet.h
index 41c2fcfdae074..8e38bb570f913 100644
--- a/llvm/include/llvm/TextAPI/SymbolSet.h
+++ b/llvm/include/llvm/TextAPI/SymbolSet.h
@@ -39,7 +39,7 @@ struct SymbolsMapKey {
   }
 
   bool operator<(const SymbolsMapKey &Other) const {
-    return Kind < Other.Kind || Name < Other.Name;
+    return std::make_pair(Kind, Name) < std::make_pair(Other.Kind, Other.Name);
   }
 };
 template <> struct DenseMapInfo<SymbolsMapKey> {



More information about the llvm-commits mailing list