[llvm] 02fc8d5 - [ORC] Add custom object interface support to StaticLibaryDefinitionGenerator.

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 16 00:47:01 PST 2021


Author: Lang Hames
Date: 2021-12-16T19:46:51+11:00
New Revision: 02fc8d5c9eb0703bb1863f22c2d27ff7a580f537

URL: https://github.com/llvm/llvm-project/commit/02fc8d5c9eb0703bb1863f22c2d27ff7a580f537
DIFF: https://github.com/llvm/llvm-project/commit/02fc8d5c9eb0703bb1863f22c2d27ff7a580f537.diff

LOG: [ORC] Add custom object interface support to StaticLibaryDefinitionGenerator.

This adds a GetObjectFileInterface callback member to
StaticLibraryDefinitionGenerator, and adds an optional argument for initializing
that member to StaticLibraryDefinitionGenerator's named constructors. If not
supplied, it will default to getObjectFileInterface from ObjectFileInterface.h.

To enable testing a `-hidden-l<x>` option is added to the llvm-jitlink tool.
This allows archives to be loaded with all contained symbol visibilities demoted
to hidden.

The ObjectLinkingLayer::setOverrideObjectFlagsWithResponsibilityFlags method is
(belatedly) hooked up, and enabled in llvm-jitlink when `-hidden-l<x>` is used
so that the demotion is also applied at symbol resolution time (avoiding any
"mismatched symbol flags" crashes).

Added: 
    llvm/test/ExecutionEngine/JITLink/X86/MachO_archive_load_hidden_expect_failure.s
    llvm/test/ExecutionEngine/JITLink/X86/MachO_archive_load_hidden_expect_success.s

Modified: 
    llvm/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h
    llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
    llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
    llvm/tools/llvm-jitlink/llvm-jitlink.cpp
    llvm/tools/llvm-jitlink/llvm-jitlink.h

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h b/llvm/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h
index 1946aed9733ed..8e572ea1d0c1d 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h
@@ -259,12 +259,18 @@ class DynamicLibrarySearchGenerator : public DefinitionGenerator {
 /// the containing object being added to the JITDylib.
 class StaticLibraryDefinitionGenerator : public DefinitionGenerator {
 public:
+  // Interface builder function for objects loaded from this archive.
+  using GetObjectFileInterface =
+      unique_function<Expected<MaterializationUnit::Interface>(
+          ExecutionSession &ES, MemoryBufferRef ObjBuffer)>;
+
   /// Try to create a StaticLibraryDefinitionGenerator from the given path.
   ///
   /// This call will succeed if the file at the given path is a static library
   /// is a valid archive, otherwise it will return an error.
   static Expected<std::unique_ptr<StaticLibraryDefinitionGenerator>>
-  Load(ObjectLayer &L, const char *FileName);
+  Load(ObjectLayer &L, const char *FileName,
+       GetObjectFileInterface GetObjFileInterface = GetObjectFileInterface());
 
   /// Try to create a StaticLibraryDefinitionGenerator from the given path.
   ///
@@ -272,13 +278,15 @@ class StaticLibraryDefinitionGenerator : public DefinitionGenerator {
   /// or a MachO universal binary containing a static library that is compatible
   /// with the given triple. Otherwise it will return an error.
   static Expected<std::unique_ptr<StaticLibraryDefinitionGenerator>>
-  Load(ObjectLayer &L, const char *FileName, const Triple &TT);
+  Load(ObjectLayer &L, const char *FileName, const Triple &TT,
+       GetObjectFileInterface GetObjFileInterface = GetObjectFileInterface());
 
   /// Try to create a StaticLibrarySearchGenerator from the given memory buffer.
   /// This call will succeed if the buffer contains a valid archive, otherwise
   /// it will return an error.
   static Expected<std::unique_ptr<StaticLibraryDefinitionGenerator>>
-  Create(ObjectLayer &L, std::unique_ptr<MemoryBuffer> ArchiveBuffer);
+  Create(ObjectLayer &L, std::unique_ptr<MemoryBuffer> ArchiveBuffer,
+         GetObjectFileInterface GetObjFileInterface = GetObjectFileInterface());
 
   Error tryToGenerate(LookupState &LS, LookupKind K, JITDylib &JD,
                       JITDylibLookupFlags JDLookupFlags,
@@ -287,9 +295,11 @@ class StaticLibraryDefinitionGenerator : public DefinitionGenerator {
 private:
   StaticLibraryDefinitionGenerator(ObjectLayer &L,
                                    std::unique_ptr<MemoryBuffer> ArchiveBuffer,
+                                   GetObjectFileInterface GetObjFileInterface,
                                    Error &Err);
 
   ObjectLayer &L;
+  GetObjectFileInterface GetObjFileInterface;
   std::unique_ptr<MemoryBuffer> ArchiveBuffer;
   std::unique_ptr<object::Archive> Archive;
 };

diff  --git a/llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp b/llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
index 2ab9ed4f856b3..ae2d47fb8c5eb 100644
--- a/llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
 #include "llvm/ExecutionEngine/Orc/Layer.h"
+#include "llvm/ExecutionEngine/Orc/ObjectFileInterface.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
@@ -269,25 +270,30 @@ Error DynamicLibrarySearchGenerator::tryToGenerate(
 }
 
 Expected<std::unique_ptr<StaticLibraryDefinitionGenerator>>
-StaticLibraryDefinitionGenerator::Load(ObjectLayer &L, const char *FileName) {
+StaticLibraryDefinitionGenerator::Load(
+    ObjectLayer &L, const char *FileName,
+    GetObjectFileInterface GetObjFileInterface) {
   auto ArchiveBuffer = errorOrToExpected(MemoryBuffer::getFile(FileName));
 
   if (!ArchiveBuffer)
     return ArchiveBuffer.takeError();
 
-  return Create(L, std::move(*ArchiveBuffer));
+  return Create(L, std::move(*ArchiveBuffer), std::move(GetObjFileInterface));
 }
 
 Expected<std::unique_ptr<StaticLibraryDefinitionGenerator>>
-StaticLibraryDefinitionGenerator::Load(ObjectLayer &L, const char *FileName,
-                                       const Triple &TT) {
+StaticLibraryDefinitionGenerator::Load(
+    ObjectLayer &L, const char *FileName, const Triple &TT,
+    GetObjectFileInterface GetObjFileInterface) {
+
   auto B = object::createBinary(FileName);
   if (!B)
     return B.takeError();
 
   // If this is a regular archive then create an instance from it.
   if (isa<object::Archive>(B->getBinary()))
-    return Create(L, std::move(B->takeBinary().second));
+    return Create(L, std::move(B->takeBinary().second),
+                  std::move(GetObjFileInterface));
 
   // If this is a universal binary then search for a slice matching the given
   // Triple.
@@ -309,7 +315,8 @@ StaticLibraryDefinitionGenerator::Load(ObjectLayer &L, const char *FileName,
                   " .. " + formatv("{0:x}", Obj.getOffset() + Obj.getSize()) +
                   ": " + SliceBuffer.getError().message(),
               SliceBuffer.getError());
-        return Create(L, std::move(*SliceBuffer));
+        return Create(L, std::move(*SliceBuffer),
+                      std::move(GetObjFileInterface));
       }
     }
 
@@ -326,11 +333,13 @@ StaticLibraryDefinitionGenerator::Load(ObjectLayer &L, const char *FileName,
 
 Expected<std::unique_ptr<StaticLibraryDefinitionGenerator>>
 StaticLibraryDefinitionGenerator::Create(
-    ObjectLayer &L, std::unique_ptr<MemoryBuffer> ArchiveBuffer) {
+    ObjectLayer &L, std::unique_ptr<MemoryBuffer> ArchiveBuffer,
+    GetObjectFileInterface GetObjFileInterface) {
   Error Err = Error::success();
 
   std::unique_ptr<StaticLibraryDefinitionGenerator> ADG(
-      new StaticLibraryDefinitionGenerator(L, std::move(ArchiveBuffer), Err));
+      new StaticLibraryDefinitionGenerator(
+          L, std::move(ArchiveBuffer), std::move(GetObjFileInterface), Err));
 
   if (Err)
     return std::move(Err);
@@ -371,7 +380,12 @@ Error StaticLibraryDefinitionGenerator::tryToGenerate(
     MemoryBufferRef ChildBufferRef(ChildBufferInfo.first,
                                    ChildBufferInfo.second);
 
-    if (auto Err = L.add(JD, MemoryBuffer::getMemBuffer(ChildBufferRef, false)))
+    auto I = GetObjFileInterface(L.getExecutionSession(), ChildBufferRef);
+    if (!I)
+      return I.takeError();
+
+    if (auto Err = L.add(JD, MemoryBuffer::getMemBuffer(ChildBufferRef, false),
+                         std::move(*I)))
       return Err;
   }
 
@@ -379,9 +393,15 @@ Error StaticLibraryDefinitionGenerator::tryToGenerate(
 }
 
 StaticLibraryDefinitionGenerator::StaticLibraryDefinitionGenerator(
-    ObjectLayer &L, std::unique_ptr<MemoryBuffer> ArchiveBuffer, Error &Err)
-    : L(L), ArchiveBuffer(std::move(ArchiveBuffer)),
-      Archive(std::make_unique<object::Archive>(*this->ArchiveBuffer, Err)) {}
+    ObjectLayer &L, std::unique_ptr<MemoryBuffer> ArchiveBuffer,
+    GetObjectFileInterface GetObjFileInterface, Error &Err)
+    : L(L), GetObjFileInterface(std::move(GetObjFileInterface)),
+      ArchiveBuffer(std::move(ArchiveBuffer)),
+      Archive(std::make_unique<object::Archive>(*this->ArchiveBuffer, Err)) {
+
+  if (!this->GetObjFileInterface)
+    this->GetObjFileInterface = getObjectFileInterface;
+}
 
 } // End namespace orc.
 } // End namespace llvm.

diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index a6d3ca7d94e2a..0d6a33c5685ee 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -249,7 +249,8 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
 
     {
 
-      // Check that InternedResult matches up with MR->getSymbols().
+      // Check that InternedResult matches up with MR->getSymbols(), overriding
+      // flags if requested.
       // This guards against faulty transformations / compilers / object caches.
 
       // First check that there aren't any missing symbols.
@@ -258,16 +259,20 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
       SymbolNameVector MissingSymbols;
       for (auto &KV : MR->getSymbols()) {
 
+        auto I = InternedResult.find(KV.first);
+
         // If this is a materialization-side-effects only symbol then bump
         // the counter and make sure it's *not* defined, otherwise make
         // sure that it is defined.
         if (KV.second.hasMaterializationSideEffectsOnly()) {
           ++NumMaterializationSideEffectsOnlySymbols;
-          if (InternedResult.count(KV.first))
+          if (I != InternedResult.end())
             ExtraSymbols.push_back(KV.first);
           continue;
-        } else if (!InternedResult.count(KV.first))
+        } else if (I == InternedResult.end())
           MissingSymbols.push_back(KV.first);
+        else if (Layer.OverrideObjectFlags)
+          I->second.setFlags(KV.second);
       }
 
       // If there were missing symbols then report the error.

diff  --git a/llvm/test/ExecutionEngine/JITLink/X86/MachO_archive_load_hidden_expect_failure.s b/llvm/test/ExecutionEngine/JITLink/X86/MachO_archive_load_hidden_expect_failure.s
new file mode 100644
index 0000000000000..26fae52aa8763
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/MachO_archive_load_hidden_expect_failure.s
@@ -0,0 +1,25 @@
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: llvm-mc -triple x86_64-apple-macosx10.9 -filetype=obj \
+# RUN:   -o %t/MachO_extra_def_strong.o %S/Inputs/MachO_extra_def_strong.s
+# RUN: llvm-ar crs %t/libExtraDef.a %t/MachO_extra_def_strong.o
+# RUN: llvm-mc -triple x86_64-apple-macosx10.9 -filetype=obj \
+# RUN:   -o %t/MachO_archive_load_hidden_support.o %s
+# RUN: not llvm-jitlink -noexec %t/MachO_archive_load_hidden_support.o -lFoo \
+# RUN:   -jd Foo -L%t -hidden-lExtraDef
+#
+# Expect this test to fail when we try to reference ExtraDef, which should have
+# be hidden in JITDylib Foo. This tests that we're correctly overriding the
+# object interface when we load the object.
+
+        .section  __TEXT,__text,regular,pure_instructions
+
+        .globl  _main
+        .p2align  4, 0x90
+_main:
+        retq
+
+	.section	__DATA,__data
+	.globl	ExtraDefRef
+	.p2align	3
+ExtraDefRef:
+	.quad	ExtraDef

diff  --git a/llvm/test/ExecutionEngine/JITLink/X86/MachO_archive_load_hidden_expect_success.s b/llvm/test/ExecutionEngine/JITLink/X86/MachO_archive_load_hidden_expect_success.s
new file mode 100644
index 0000000000000..249519356f8cb
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/MachO_archive_load_hidden_expect_success.s
@@ -0,0 +1,25 @@
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: llvm-mc -triple x86_64-apple-macosx10.9 -filetype=obj \
+# RUN:   -o %t/MachO_extra_def_strong.o %S/Inputs/MachO_extra_def_strong.s
+# RUN: llvm-ar crs %t/libExtraDef.a %t/MachO_extra_def_strong.o
+# RUN: llvm-mc -triple x86_64-apple-macosx10.9 -filetype=obj \
+# RUN:   -o %t/MachO_archive_load_hidden_support.o %s
+# RUN: llvm-jitlink -noexec %t/MachO_archive_load_hidden_support.o \
+# RUN:   -L%t -hidden-lExtraDef
+#
+# Expect this test to succeed -- ExtraDef should be hidden, but visible to
+# ExtraDefRef as they're linked in the same JITDylib. This tests that we're
+# correctly handling the change in object interface when linking ExtraDef.
+
+        .section  __TEXT,__text,regular,pure_instructions
+
+        .globl  _main
+        .p2align  4, 0x90
+_main:
+        retq
+
+	.section	__DATA,__data
+	.globl	ExtraDefRef
+	.p2align	3
+ExtraDefRef:
+	.quad	ExtraDef

diff  --git a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
index aa70c794405c7..3638569343ba8 100644
--- a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
+++ b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
@@ -82,6 +82,10 @@ static cl::list<std::string>
               cl::desc("Link against library X in the library search paths"),
               cl::Prefix, cl::cat(JITLinkCategory));
 
+static cl::list<std::string> LibrariesHidden(
+    "hidden-l", cl::desc("Link against library X in the library search paths"),
+    cl::Prefix, cl::cat(JITLinkCategory));
+
 static cl::opt<bool> NoExec("noexec", cl::desc("Do not execute loaded code"),
                             cl::init(false), cl::cat(JITLinkCategory));
 
@@ -1413,9 +1417,20 @@ static Error addObjects(Session &S,
   return Error::success();
 }
 
+static Expected<MaterializationUnit::Interface>
+getObjectFileInterfaceHidden(ExecutionSession &ES, MemoryBufferRef ObjBuffer) {
+  auto I = getObjectFileInterface(ES, ObjBuffer);
+  if (I) {
+    for (auto &KV : I->SymbolFlags)
+      KV.second &= ~JITSymbolFlags::Exported;
+  }
+  return I;
+}
+
 static Error addLibraries(Session &S,
                           const std::map<unsigned, JITDylib *> &IdxToJD) {
 
+  // 1. Collect search paths for each JITDylib.
   DenseMap<const JITDylib *, SmallVector<StringRef, 2>> JDSearchPaths;
 
   for (auto LSPItr = LibrarySearchPaths.begin(),
@@ -1447,16 +1462,58 @@ static Error addLibraries(Session &S,
     }
   });
 
+  // 2. Collect library loads from -lx, -hidden-lx.
+  struct LibraryLoad {
+    StringRef LibName;
+    unsigned Position;
+    StringRef *CandidateExtensions;
+    enum { Standard, Hidden } Modifier;
+  };
+  std::vector<LibraryLoad> LibraryLoads;
+  StringRef StandardExtensions[] = {".so", ".dylib", ".a"};
+  StringRef ArchiveExtensionsOnly[] = {".a"};
+
+  // Add -lx arguments to LibraryLoads.
   for (auto LibItr = Libraries.begin(), LibEnd = Libraries.end();
        LibItr != LibEnd; ++LibItr) {
+    LibraryLoad LL;
+    LL.LibName = *LibItr;
+    LL.Position = Libraries.getPosition(LibItr - Libraries.begin());
+    LL.CandidateExtensions = StandardExtensions;
+    LL.Modifier = LibraryLoad::Standard;
+    LibraryLoads.push_back(std::move(LL));
+  }
+
+  // Add -hidden-lx arguments to LibraryLoads.
+  for (auto LibHiddenItr = LibrariesHidden.begin(),
+            LibHiddenEnd = LibrariesHidden.end();
+       LibHiddenItr != LibHiddenEnd; ++LibHiddenItr) {
+    LibraryLoad LL;
+    LL.LibName = *LibHiddenItr;
+    LL.Position =
+        LibrariesHidden.getPosition(LibHiddenItr - LibrariesHidden.begin());
+    LL.CandidateExtensions = ArchiveExtensionsOnly;
+    LL.Modifier = LibraryLoad::Hidden;
+    LibraryLoads.push_back(std::move(LL));
+  }
+
+  // If there are any load-<modified> options then turn on flag overrides
+  // to avoid flag mismatch errors.
+  if (!LibrariesHidden.empty())
+    S.ObjLayer.setOverrideObjectFlagsWithResponsibilityFlags(true);
+
+  // Sort library loads by position in the argument list.
+  llvm::sort(LibraryLoads, [](const LibraryLoad &LHS, const LibraryLoad &RHS) {
+    return LHS.Position < RHS.Position;
+  });
 
+  // 3. Process library loads.
+  for (auto &LL : LibraryLoads) {
     bool LibFound = false;
-    StringRef LibName(*LibItr);
-    unsigned LibIdx = Libraries.getPosition(LibItr - Libraries.begin());
-    auto &JD = *std::prev(IdxToJD.lower_bound(LibIdx))->second;
+    auto &JD = *std::prev(IdxToJD.lower_bound(LL.Position))->second;
 
     // If this is the name of a JITDylib then link against that.
-    if (auto *LJD = S.ES.getJITDylibByName(LibName)) {
+    if (auto *LJD = S.ES.getJITDylibByName(LL.LibName)) {
       JD.addToLinkOrder(*LJD);
       continue;
     }
@@ -1467,10 +1524,11 @@ static Error addLibraries(Session &S,
       for (StringRef SearchPath : JDSearchPathsItr->second) {
         for (const char *LibExt : {".dylib", ".so", ".a"}) {
           SmallVector<char, 256> LibPath;
-          LibPath.reserve(SearchPath.size() + strlen("lib") + LibName.size() +
-                          strlen(LibExt) + 2); // +2 for pathsep, null term.
+          LibPath.reserve(SearchPath.size() + strlen("lib") +
+                          LL.LibName.size() + strlen(LibExt) +
+                          2); // +2 for pathsep, null term.
           llvm::copy(SearchPath, std::back_inserter(LibPath));
-          sys::path::append(LibPath, "lib" + LibName + LibExt);
+          sys::path::append(LibPath, "lib" + LL.LibName + LibExt);
           LibPath.push_back('\0');
 
           // Skip missing or non-regular paths.
@@ -1513,9 +1571,21 @@ static Error addLibraries(Session &S,
           }
           case file_magic::archive:
           case file_magic::macho_universal_binary: {
+            unique_function<Expected<MaterializationUnit::Interface>(
+                ExecutionSession & ES, MemoryBufferRef ObjBuffer)>
+                GetObjFileInterface;
+            switch (LL.Modifier) {
+            case LibraryLoad::Standard:
+              GetObjFileInterface = getObjectFileInterface;
+              break;
+            case LibraryLoad::Hidden:
+              GetObjFileInterface = getObjectFileInterfaceHidden;
+              break;
+            }
             auto G = StaticLibraryDefinitionGenerator::Load(
                 S.ObjLayer, LibPath.data(),
-                S.ES.getExecutorProcessControl().getTargetTriple());
+                S.ES.getExecutorProcessControl().getTargetTriple(),
+                std::move(GetObjFileInterface));
             if (!G)
               return G.takeError();
             JD.addGenerator(std::move(*G));
@@ -1545,7 +1615,7 @@ static Error addLibraries(Session &S,
     if (!LibFound)
       return make_error<StringError>("While linking " + JD.getName() +
                                          ", could not find library for -l" +
-                                         LibName,
+                                         LL.LibName,
                                      inconvertibleErrorCode());
   }
 

diff  --git a/llvm/tools/llvm-jitlink/llvm-jitlink.h b/llvm/tools/llvm-jitlink/llvm-jitlink.h
index 7873bcaa13d4c..71bdab7862c6e 100644
--- a/llvm/tools/llvm-jitlink/llvm-jitlink.h
+++ b/llvm/tools/llvm-jitlink/llvm-jitlink.h
@@ -32,6 +32,7 @@ namespace llvm {
 struct Session;
 
 struct Session {
+
   orc::ExecutionSession ES;
   orc::JITDylib *MainJD = nullptr;
   orc::ObjectLinkingLayer ObjLayer;


        


More information about the llvm-commits mailing list