[JITLink] Introduce new weakly-referenced concept separate from linkage.

Introduces two new methods on Symbol: isWeaklyReferenced and
setWeaklyReferenced. These are now used to track/set whether an external symbol
is weakly referenced, rather than having the Symbol's linkage set to weak.

This change is a first step towards proper handling of weak defs used across
JITDylib boundaries: It frees up the Linkage field on external symbols so that
it can be used to represent the linkage of the definition that the symbol resolves
to. It is expected that Platform plugins will use this information to track
locations that need to be updated if the selected weak definition changes (e.g.
because JITDylibs were dlclosed and then dlopened again in a different order).

diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index 3b759f1..54dbd6c 100644 (file)
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -405,7 +405,7 @@ private:
   Symbol(Addressable &Base, orc::ExecutorAddrDiff Offset, StringRef Name,
          orc::ExecutorAddrDiff Size, Linkage L, Scope S, bool IsLive,
          bool IsCallable)
-      : Name(Name), Base(&Base), Offset(Offset), Size(Size) {
+      : Name(Name), Base(&Base), Offset(Offset), WeakRef(0), Size(Size) {
     assert(Offset <= MaxOffset && "Offset out of range");
     setLinkage(L);
     setScope(S);
@@ -428,12 +428,14 @@ private:
 
   static Symbol &constructExternal(BumpPtrAllocator &Allocator,
                                    Addressable &Base, StringRef Name,
-                                   orc::ExecutorAddrDiff Size, Linkage L) {
+                                   orc::ExecutorAddrDiff Size, Linkage L,
+                                   bool WeaklyReferenced) {
     assert(!Base.isDefined() &&
            "Cannot create external symbol from defined block");
     assert(!Name.empty() && "External symbol name cannot be empty");
     auto *Sym = Allocator.Allocate<Symbol>();
     new (Sym) Symbol(Base, 0, Name, Size, L, Scope::Default, false, false);
+    Sym->setWeaklyReferenced(WeaklyReferenced);
     return *Sym;
   }
 
@@ -615,6 +617,20 @@ public:
     this->S = static_cast<uint8_t>(S);
   }
 
+  /// Returns true if this is a weakly referenced external symbol.
+  /// This method may only be called on external symbols.
+  bool isWeaklyReferenced() const {
+    assert(isExternal() && "isWeaklyReferenced called on non-external");
+    return WeakRef;
+  }
+
+  /// Set the WeaklyReferenced value for this symbol.
+  /// This method may only be called on external symbols.
+  void setWeaklyReferenced(bool WeakRef) {
+    assert(isExternal() && "setWeaklyReferenced called on non-external");
+    this->WeakRef = WeakRef;
+  }
+
 private:
   void makeExternal(Addressable &A) {
     assert(!A.isDefined() && !A.isAbsolute() &&
@@ -645,11 +661,12 @@ private:
   // FIXME: A char* or SymbolStringPtr may pack better.
   StringRef Name;
   Addressable *Base = nullptr;
-  uint64_t Offset : 59;
+  uint64_t Offset : 58;
   uint64_t L : 1;
   uint64_t S : 2;
   uint64_t IsLive : 1;
   uint64_t IsCallable : 1;
+  uint64_t WeakRef : 1;
   size_t Size = 0;
 };
 
@@ -1076,12 +1093,13 @@ public:
   /// Add an external symbol.
   /// Some formats (e.g. ELF) allow Symbols to have sizes. For Symbols whose
   /// size is not known, you should substitute '0'.
-  /// For external symbols Linkage determines whether the symbol must be
-  /// present during lookup: Externals with strong linkage must be found or
-  /// an error will be emitted. Externals with weak linkage are permitted to
-  /// be undefined, in which case they are assigned a value of 0.
+  /// The IsWeaklyReferenced argument determines whether the symbol must be
+  /// present during lookup: Externals that are strongly referenced must be
+  /// found or an error will be emitted. Externals that are weakly referenced
+  /// are permitted to be undefined, in which case they are assigned an address
+  /// of 0.
   Symbol &addExternalSymbol(StringRef Name, orc::ExecutorAddrDiff Size,
-                            Linkage L) {
+                            bool IsWeaklyReferenced) {
     assert(llvm::count_if(ExternalSymbols,
                           [&](const Symbol *Sym) {
                             return Sym->getName() == Name;
@@ -1089,7 +1107,7 @@ public:
            "Duplicate external symbol");
     auto &Sym = Symbol::constructExternal(
         Allocator, createAddressable(orc::ExecutorAddr(), false), Name, Size,
-        L);
+        Linkage::Strong, IsWeaklyReferenced);
     ExternalSymbols.insert(&Sym);
     return Sym;
   }

diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
index 6e1801c..5198a9a 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
@@ -289,8 +289,7 @@ Error COFFLinkGraphBuilder::handleDirectiveSection(StringRef Str) {
     case COFF_OPT_incl: {
       auto DataCopy = G->allocateString(S);
       StringRef StrCopy(DataCopy.data(), DataCopy.size());
-      ExternalSymbols[StrCopy] =
-          &G->addExternalSymbol(StrCopy, 0, Linkage::Strong);
+      ExternalSymbols[StrCopy] = &G->addExternalSymbol(StrCopy, 0, false);
       ExternalSymbols[StrCopy]->setLive(true);
       break;
     }
@@ -361,7 +360,7 @@ Symbol *COFFLinkGraphBuilder::createExternalSymbol(
     object::COFFSymbolRef Symbol, const object::coff_section *Section) {
   if (!ExternalSymbols.count(SymbolName))
     ExternalSymbols[SymbolName] =
-        &G->addExternalSymbol(SymbolName, Symbol.getValue(), Linkage::Strong);
+        &G->addExternalSymbol(SymbolName, Symbol.getValue(), false);
 
   LLVM_DEBUG({
     dbgs() << "    " << SymIndex

diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
index 2ab7ed6..b9e0c29 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
+++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
@@ -409,19 +409,19 @@ template <typename ELFT> Error ELFLinkGraphBuilder<ELFT>::graphifySymbols() {
       continue;
     }
 
-    // Map Visibility and Binding to Scope and Linkage:
-    Linkage L;
-    Scope S;
-
-    if (auto LSOrErr = getSymbolLinkageAndScope(Sym, *Name))
-      std::tie(L, S) = *LSOrErr;
-    else
-      return LSOrErr.takeError();
-
     if (Sym.isDefined() &&
         (Sym.getType() == ELF::STT_NOTYPE || Sym.getType() == ELF::STT_FUNC ||
          Sym.getType() == ELF::STT_OBJECT ||
          Sym.getType() == ELF::STT_SECTION || Sym.getType() == ELF::STT_TLS)) {
+
+      // Map Visibility and Binding to Scope and Linkage:
+      Linkage L;
+      Scope S;
+      if (auto LSOrErr = getSymbolLinkageAndScope(Sym, *Name))
+        std::tie(L, S) = *LSOrErr;
+      else
+        return LSOrErr.takeError();
+
       // Handle extended tables.
       unsigned Shndx = Sym.st_shndx;
       if (Shndx == ELF::SHN_XINDEX) {
@@ -460,7 +460,25 @@ template <typename ELFT> Error ELFLinkGraphBuilder<ELFT>::graphifySymbols() {
                << ": Creating external graph symbol for ELF symbol \"" << *Name
                << "\"\n";
       });
-      auto &GSym = G->addExternalSymbol(*Name, Sym.st_size, L);
+
+      if (Sym.getBinding() != ELF::STB_GLOBAL &&
+          Sym.getBinding() != ELF::STB_WEAK)
+        return make_error<StringError>(
+            "Invalid symbol binding " +
+                Twine(static_cast<int>(Sym.getBinding())) +
+                " for external symbol " + *Name,
+            inconvertibleErrorCode());
+
+      if (Sym.getVisibility() != ELF::STV_DEFAULT)
+        return make_error<StringError>(
+            "Invalid symbol visibility " +
+                Twine(static_cast<int>(Sym.getVisibility())) +
+                " for external symbol " + *Name,
+            inconvertibleErrorCode());
+
+      // If L is Linkage::Weak that means this is a weakly referenced symbol.
+      auto &GSym = G->addExternalSymbol(*Name, Sym.st_size,
+                                        Sym.getBinding() == ELF::STB_WEAK);
       setGraphSymbol(SymIndex, GSym);
     } else {
       LLVM_DEBUG({

diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp
index 7d67e5e..b1fe9c1 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp
@@ -487,8 +487,7 @@ private:
 
   Symbol &getTLSDescResolver(LinkGraph &G) {
     if (!TLSDescResolver)
-      TLSDescResolver =
-          &G.addExternalSymbol("__tlsdesc_resolver", 8, Linkage::Strong);
+      TLSDescResolver = &G.addExternalSymbol("__tlsdesc_resolver", 8, false);
     return *TLSDescResolver;
   }
 

diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
index 7ad1a10..37d28b1 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
@@ -203,9 +203,8 @@ JITLinkContext::LookupMap JITLinkerBase::getExternalSymbolNames() const {
     assert(Sym->getName() != StringRef() && Sym->getName() != "" &&
            "Externals must be named");
     SymbolLookupFlags LookupFlags =
-        Sym->getLinkage() == Linkage::Weak
-            ? SymbolLookupFlags::WeaklyReferencedSymbol
-            : SymbolLookupFlags::RequiredSymbol;
+        Sym->isWeaklyReferenced() ? SymbolLookupFlags::WeaklyReferencedSymbol
+                                  : SymbolLookupFlags::RequiredSymbol;
     UnresolvedExternals[Sym->getName()] = LookupFlags;
   }
   return UnresolvedExternals;
@@ -222,7 +221,7 @@ void JITLinkerBase::applyLookupResult(AsyncLookupResult Result) {
       Sym->getAddressable().setAddress(
           orc::ExecutorAddr(ResultI->second.getAddress()));
     else
-      assert(Sym->getLinkage() == Linkage::Weak &&
+      assert(Sym->isWeaklyReferenced() &&
              "Failed to resolve non-weak reference");
   }
 

diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
index 1315654..80908d1 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
@@ -361,8 +361,7 @@ Error MachOLinkGraphBuilder::graphifyRegularSymbols() {
                                           "index " +
                                           Twine(KV.first));
         NSym.GraphSymbol = &G->addExternalSymbol(
-            *NSym.Name, 0,
-            NSym.Desc & MachO::N_WEAK_REF ? Linkage::Weak : Linkage::Strong);
+            *NSym.Name, 0, (NSym.Desc & MachO::N_WEAK_REF) != 0);
       }
       break;
     case MachO::N_ABS: