[ORC] Switch to shared_ptr ownership for AsynchronousSymbolQueries.

Queries need to stay alive until each owner has set the values they are responsible for. llvm-svn: 325179
2025-01-31 20:51:52 +01:00 · 2018-02-14 22:12:56 +00:00 · 2018-02-14 22:12:56 +00:00 · df05abc39f
commit df05abc39f
parent 1184db4a1c
13 changed files with 81 additions and 92 deletions
--- a/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
+++ b/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
@ -492,10 +492,10 @@ private:
          return LD.BackingResolver->lookupFlags(SymbolFlags,
                                                 *NotFoundViaLegacyLookup);
        },
-        [&LD, LegacyLookup](AsynchronousSymbolQuery &Query,
+        [&LD, LegacyLookup](std::shared_ptr<AsynchronousSymbolQuery> Query,
                            SymbolNameSet Symbols) {
          auto NotFoundViaLegacyLookup =
-              lookupWithLegacyFn(Query, Symbols, LegacyLookup);
+              lookupWithLegacyFn(*Query, Symbols, LegacyLookup);
          return LD.BackingResolver->lookup(Query, NotFoundViaLegacyLookup);
        });

@ -649,9 +649,10 @@ private:
          return LD.BackingResolver->lookupFlags(SymbolFlags,
                                                 *NotFoundViaLegacyLookup);
        },
-        [&LD, LegacyLookup](AsynchronousSymbolQuery &Q, SymbolNameSet Symbols) {
+        [&LD, LegacyLookup](std::shared_ptr<AsynchronousSymbolQuery> Q,
+                            SymbolNameSet Symbols) {
          auto NotFoundViaLegacyLookup =
-              lookupWithLegacyFn(Q, Symbols, LegacyLookup);
+              lookupWithLegacyFn(*Q, Symbols, LegacyLookup);
          return LD.BackingResolver->lookup(Q,
                                            std::move(NotFoundViaLegacyLookup));
        });
--- a/include/llvm/ExecutionEngine/Orc/Core.h
+++ b/include/llvm/ExecutionEngine/Orc/Core.h
@ -107,7 +107,7 @@ public:

  /// @brief For each symbol in Symbols that can be found, assigns that symbols
  ///        value in Query. Returns the set of symbols that could not be found.
-  virtual SymbolNameSet lookup(AsynchronousSymbolQuery &Query,
+  virtual SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
                               SymbolNameSet Symbols) = 0;

 private:
@ -129,9 +129,9 @@ public:
    return LookupFlags(Flags, Symbols);
  }

-  SymbolNameSet lookup(AsynchronousSymbolQuery &Query,
+  SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
                       SymbolNameSet Symbols) final {
-    return Lookup(Query, std::move(Symbols));
+    return Lookup(std::move(Query), std::move(Symbols));
  }

 private:
@ -258,23 +258,26 @@ public:
  ///
  /// Any symbols not found in this VSO will be returned in the
  /// UnresolvedSymbols field of the LookupResult.
-  LookupResult lookup(AsynchronousSymbolQuery &Query, SymbolNameSet Symbols);
+  LookupResult lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
+                      SymbolNameSet Symbols);

 private:
  class MaterializationInfo {
  public:
-    MaterializationInfo(JITSymbolFlags Flags, AsynchronousSymbolQuery &Query);
+    MaterializationInfo(JITSymbolFlags Flags,
+                        std::shared_ptr<AsynchronousSymbolQuery> Query);
    JITSymbolFlags getFlags() const;
    JITTargetAddress getAddress() const;
-    void query(SymbolStringPtr Name, AsynchronousSymbolQuery &Query);
+    void query(SymbolStringPtr Name,
+               std::shared_ptr<AsynchronousSymbolQuery> Query);
    void resolve(SymbolStringPtr Name, JITEvaluatedSymbol Sym);
    void finalize();

  private:
    JITSymbolFlags Flags;
    JITTargetAddress Address = 0;
-    std::vector<AsynchronousSymbolQuery *> PendingResolution;
-    std::vector<AsynchronousSymbolQuery *> PendingFinalization;
+    std::vector<std::shared_ptr<AsynchronousSymbolQuery>> PendingResolution;
+    std::vector<std::shared_ptr<AsynchronousSymbolQuery>> PendingFinalization;
  };

  class SymbolTableEntry {
@ -286,7 +289,8 @@ private:
    JITSymbolFlags getFlags() const;
    void replaceWithSource(VSO &V, SymbolStringPtr Name, JITSymbolFlags Flags,
                           SymbolSource &NewSource);
-    SymbolSource *query(SymbolStringPtr Name, AsynchronousSymbolQuery &Query);
+    SymbolSource *query(SymbolStringPtr Name,
+                        std::shared_ptr<AsynchronousSymbolQuery> Query);
    void resolve(VSO &V, SymbolStringPtr Name, JITEvaluatedSymbol Sym);
    void finalize();

--- a/include/llvm/ExecutionEngine/Orc/Legacy.h
+++ b/include/llvm/ExecutionEngine/Orc/Legacy.h
@ -114,9 +114,9 @@ public:
    }
  }

-  SymbolNameSet lookup(AsynchronousSymbolQuery &Query,
+  SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
                       SymbolNameSet Symbols) final {
-    return lookupWithLegacyFn(Query, Symbols, LegacyLookup);
+    return lookupWithLegacyFn(*Query, Symbols, LegacyLookup);
  }

 private:
--- a/include/llvm/ExecutionEngine/Orc/NullResolver.h
+++ b/include/llvm/ExecutionEngine/Orc/NullResolver.h
@ -26,7 +26,7 @@ public:
  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
                            const SymbolNameSet &Symbols) override;

-  SymbolNameSet lookup(AsynchronousSymbolQuery &Query,
+  SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
                       SymbolNameSet Symbols) override;
 };

--- a/lib/ExecutionEngine/Orc/Core.cpp
+++ b/lib/ExecutionEngine/Orc/Core.cpp
@ -64,9 +64,9 @@ void AsynchronousSymbolQuery::notifySymbolFinalized() {
    NotifySymbolsReady(Error::success());
 }

-VSO::MaterializationInfo::MaterializationInfo(JITSymbolFlags Flags,
-                                              AsynchronousSymbolQuery &Query)
-    : Flags(std::move(Flags)), PendingResolution({&Query}) {}
+VSO::MaterializationInfo::MaterializationInfo(
+    JITSymbolFlags Flags, std::shared_ptr<AsynchronousSymbolQuery> Query)
+    : Flags(std::move(Flags)), PendingResolution({std::move(Query)}) {}

 JITSymbolFlags VSO::MaterializationInfo::getFlags() const { return Flags; }

@ -74,13 +74,13 @@ JITTargetAddress VSO::MaterializationInfo::getAddress() const {
  return Address;
 }

-void VSO::MaterializationInfo::query(SymbolStringPtr Name,
-                                     AsynchronousSymbolQuery &Query) {
+void VSO::MaterializationInfo::query(
+    SymbolStringPtr Name, std::shared_ptr<AsynchronousSymbolQuery> Query) {
  if (Address != 0) {
-    Query.setDefinition(Name, JITEvaluatedSymbol(Address, Flags));
-    PendingFinalization.push_back(&Query);
+    Query->setDefinition(Name, JITEvaluatedSymbol(Address, Flags));
+    PendingFinalization.push_back(std::move(Query));
  } else
-    PendingResolution.push_back(&Query);
+    PendingResolution.push_back(std::move(Query));
 }

 void VSO::MaterializationInfo::resolve(SymbolStringPtr Name,
@ -88,15 +88,15 @@ void VSO::MaterializationInfo::resolve(SymbolStringPtr Name,
  // FIXME: Sanity check flags?
  Flags = Sym.getFlags();
  Address = Sym.getAddress();
-  for (auto *Query : PendingResolution) {
+  for (auto &Query : PendingResolution) {
    Query->setDefinition(Name, std::move(Sym));
-    PendingFinalization.push_back(Query);
+    PendingFinalization.push_back(std::move(Query));
  }
  PendingResolution = {};
 }

 void VSO::MaterializationInfo::finalize() {
-  for (auto *Query : PendingFinalization)
+  for (auto &Query : PendingFinalization)
    Query->notifySymbolFinalized();
  PendingFinalization = {};
 }
@ -140,19 +140,20 @@ void VSO::SymbolTableEntry::replaceWithSource(VSO &V, SymbolStringPtr Name,
  this->Source = &NewSource;
 }

-SymbolSource *VSO::SymbolTableEntry::query(SymbolStringPtr Name,
-                                           AsynchronousSymbolQuery &Query) {
+SymbolSource *
+VSO::SymbolTableEntry::query(SymbolStringPtr Name,
+                             std::shared_ptr<AsynchronousSymbolQuery> Query) {
  if (Flags.isMaterializing()) {
-    MatInfo->query(std::move(Name), Query);
+    MatInfo->query(std::move(Name), std::move(Query));
    return nullptr;
  } else if (Flags.isMaterialized()) {
-    Query.setDefinition(std::move(Name), JITEvaluatedSymbol(Address, Flags));
-    Query.notifySymbolFinalized();
+    Query->setDefinition(std::move(Name), JITEvaluatedSymbol(Address, Flags));
+    Query->notifySymbolFinalized();
    return nullptr;
  }
  SymbolSource *S = Source;
  new (&MatInfo) std::unique_ptr<MaterializationInfo>(
-      llvm::make_unique<MaterializationInfo>(Flags, Query));
+      llvm::make_unique<MaterializationInfo>(Flags, std::move(Query)));
  Flags |= JITSymbolFlags::Materializing;
  return S;
 }
@ -307,7 +308,7 @@ SymbolNameSet VSO::lookupFlags(SymbolFlagsMap &Flags, SymbolNameSet Names) {
  return Names;
 }

-VSO::LookupResult VSO::lookup(AsynchronousSymbolQuery &Query,
+VSO::LookupResult VSO::lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
                              SymbolNameSet Names) {
  SourceWorkMap MaterializationWork;

--- a/lib/ExecutionEngine/Orc/Legacy.cpp
+++ b/lib/ExecutionEngine/Orc/Legacy.cpp
@ -41,9 +41,10 @@ JITSymbolResolverAdapter::lookup(const LookupSet &Symbols) {
                          "legacy resolver received on-ready error:\n");
  };

-  AsynchronousSymbolQuery Query(InternedSymbols, OnResolve, OnReady);
+  auto Query = std::make_shared<AsynchronousSymbolQuery>(InternedSymbols,
+                                                         OnResolve, OnReady);

-  auto UnresolvedSymbols = R.lookup(Query, InternedSymbols);
+  auto UnresolvedSymbols = R.lookup(std::move(Query), InternedSymbols);

  if (!UnresolvedSymbols.empty()) {
    std::string ErrorMsg = "Unresolved symbols: ";
--- a/lib/ExecutionEngine/Orc/NullResolver.cpp
+++ b/lib/ExecutionEngine/Orc/NullResolver.cpp
@ -19,8 +19,9 @@ SymbolNameSet NullResolver::lookupFlags(SymbolFlagsMap &Flags,
  return Symbols;
 }

-SymbolNameSet NullResolver::lookup(AsynchronousSymbolQuery &Query,
-                                   SymbolNameSet Symbols) {
+SymbolNameSet
+NullResolver::lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
+                     SymbolNameSet Symbols) {
  assert(Symbols.empty() && "Null resolver: Symbols must be empty");
  return Symbols;
 }
--- a/lib/ExecutionEngine/Orc/OrcCBindingsStack.h
+++ b/lib/ExecutionEngine/Orc/OrcCBindingsStack.h
@ -147,20 +147,21 @@ private:
      return SymbolsNotFound;
    }

-    orc::SymbolNameSet lookup(orc::AsynchronousSymbolQuery &Query,
-                              orc::SymbolNameSet Symbols) override {
+    orc::SymbolNameSet
+    lookup(std::shared_ptr<orc::AsynchronousSymbolQuery> Query,
+           orc::SymbolNameSet Symbols) override {
      orc::SymbolNameSet UnresolvedSymbols;

      for (auto &S : Symbols) {
        if (auto Sym = findSymbol(*S)) {
          if (auto Addr = Sym.getAddress())
-            Query.setDefinition(S, JITEvaluatedSymbol(*Addr, Sym.getFlags()));
+            Query->setDefinition(S, JITEvaluatedSymbol(*Addr, Sym.getFlags()));
          else {
-            Query.setFailed(Addr.takeError());
+            Query->setFailed(Addr.takeError());
            return orc::SymbolNameSet();
          }
        } else if (auto Err = Sym.takeError()) {
-          Query.setFailed(std::move(Err));
+          Query->setFailed(std::move(Err));
          return orc::SymbolNameSet();
        } else
          UnresolvedSymbols.insert(S);
--- a/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
+++ b/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
@ -166,32 +166,32 @@ class OrcMCJITReplacement : public ExecutionEngine {
      return UnresolvedSymbols;
    }

-    SymbolNameSet lookup(AsynchronousSymbolQuery &Query,
+    SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
                         SymbolNameSet Symbols) override {
      SymbolNameSet UnresolvedSymbols;

      for (auto &S : Symbols) {
        if (auto Sym = M.findMangledSymbol(*S)) {
          if (auto Addr = Sym.getAddress())
-            Query.setDefinition(S, JITEvaluatedSymbol(*Addr, Sym.getFlags()));
+            Query->setDefinition(S, JITEvaluatedSymbol(*Addr, Sym.getFlags()));
          else {
-            Query.setFailed(Addr.takeError());
+            Query->setFailed(Addr.takeError());
            return SymbolNameSet();
          }
        } else if (auto Err = Sym.takeError()) {
-          Query.setFailed(std::move(Err));
+          Query->setFailed(std::move(Err));
          return SymbolNameSet();
        } else {
          if (auto Sym2 = M.ClientResolver->findSymbol(*S)) {
            if (auto Addr = Sym2.getAddress())
-              Query.setDefinition(S,
-                                  JITEvaluatedSymbol(*Addr, Sym2.getFlags()));
+              Query->setDefinition(S,
+                                   JITEvaluatedSymbol(*Addr, Sym2.getFlags()));
            else {
-              Query.setFailed(Addr.takeError());
+              Query->setFailed(Addr.takeError());
              return SymbolNameSet();
            }
          } else if (auto Err = Sym2.takeError()) {
-            Query.setFailed(std::move(Err));
+            Query->setFailed(std::move(Err));
            return SymbolNameSet();
          } else
            UnresolvedSymbols.insert(S);
--- a/tools/lli/OrcLazyJIT.h
+++ b/tools/lli/OrcLazyJIT.h
@ -174,9 +174,9 @@ public:
            }
            return std::move(*NotFoundViaLegacyLookup);
          },
-          [LegacyLookup](orc::AsynchronousSymbolQuery &Query,
+          [LegacyLookup](std::shared_ptr<orc::AsynchronousSymbolQuery> Query,
                         orc::SymbolNameSet Symbols) {
-            return lookupWithLegacyFn(Query, Symbols, LegacyLookup);
+            return lookupWithLegacyFn(*Query, Symbols, LegacyLookup);
          });

      // Add the module to the JIT.
--- a/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
+++ b/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
@ -118,7 +118,8 @@ TEST(CoreAPIsTest, SimpleAsynchronousSymbolQueryAgainstVSO) {
    OnReadyRun = true;
  };

-  AsynchronousSymbolQuery Q(Names, OnResolution, OnReady);
+  auto Q =
+      std::make_shared<AsynchronousSymbolQuery>(Names, OnResolution, OnReady);
  VSO V;

  SymbolMap Defs;
@ -241,9 +242,10 @@ TEST(CoreAPIsTest, AddAndMaterializeLazySymbol) {
    OnReadyRun = true;
  };

-  AsynchronousSymbolQuery Q(Names, OnResolution, OnReady);
+  auto Q =
+      std::make_shared<AsynchronousSymbolQuery>(Names, OnResolution, OnReady);

-  auto LR = V.lookup(Q, Names);
+  auto LR = V.lookup(std::move(Q), Names);

  for (auto &SWKV : LR.MaterializationWork)
    cantFail(SWKV.first->materialize(V, std::move(SWKV.second)));
@ -271,8 +273,8 @@ TEST(CoreAPIsTest, TestLambdaSymbolResolver) {
      [&](SymbolFlagsMap &SymbolFlags, const SymbolNameSet &Symbols) {
        return V.lookupFlags(SymbolFlags, Symbols);
      },
-      [&](AsynchronousSymbolQuery &Q, SymbolNameSet Symbols) {
-        auto LR = V.lookup(Q, Symbols);
+      [&](std::shared_ptr<AsynchronousSymbolQuery> Q, SymbolNameSet Symbols) {
+        auto LR = V.lookup(std::move(Q), Symbols);
        assert(LR.MaterializationWork.empty() &&
               "Test generated unexpected materialization "
               "work?");
@ -314,8 +316,9 @@ TEST(CoreAPIsTest, TestLambdaSymbolResolver) {
    EXPECT_FALSE(!!Err) << "Finalization should never fail in this test";
  };

-  AsynchronousSymbolQuery Q({Foo, Bar}, OnResolved, OnReady);
-  auto Unresolved = Resolver->lookup(Q, Symbols);
+  auto Q = std::make_shared<AsynchronousSymbolQuery>(SymbolNameSet({Foo, Bar}),
+                                                     OnResolved, OnReady);
+  auto Unresolved = Resolver->lookup(std::move(Q), Symbols);

  EXPECT_EQ(Unresolved.size(), 1U) << "Expected one unresolved symbol";
  EXPECT_EQ(Unresolved.count(Baz), 1U) << "Expected baz to not be resolved";
--- a/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
+++ b/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
@ -14,31 +14,6 @@
 using namespace llvm;
 using namespace llvm::orc;

-class SimpleORCResolver : public SymbolResolver {
-public:
-  using LookupFlagsFn = std::function<SymbolNameSet(SymbolFlagsMap &SymbolFlags,
-                                                    const SymbolNameSet &)>;
-  using LookupFn = std::function<SymbolNameSet(AsynchronousSymbolQuery &Q,
-                                               SymbolNameSet Symbols)>;
-
-  SimpleORCResolver(LookupFlagsFn LookupFlags, LookupFn Lookup)
-      : LookupFlags(std::move(LookupFlags)), Lookup(std::move(Lookup)) {}
-
-  SymbolNameSet lookupFlags(SymbolFlagsMap &SymbolFlags,
-                            const SymbolNameSet &Symbols) override {
-    return LookupFlags(SymbolFlags, Symbols);
-  }
-
-  SymbolNameSet lookup(AsynchronousSymbolQuery &Query,
-                       SymbolNameSet Symbols) override {
-    return Lookup(Query, std::move(Symbols));
-  };
-
-private:
-  LookupFlagsFn LookupFlags;
-  LookupFn Lookup;
-};
-
 namespace {

 TEST(LegacyAPIInteropTest, QueryAgainstVSO) {
@ -58,16 +33,17 @@ TEST(LegacyAPIInteropTest, QueryAgainstVSO) {
    return V.lookupFlags(SymbolFlags, Names);
  };

-  auto Lookup = [&](AsynchronousSymbolQuery &Query, SymbolNameSet Symbols) {
-    auto R = V.lookup(Query, Symbols);
+  auto Lookup = [&](std::shared_ptr<AsynchronousSymbolQuery> Query,
+                    SymbolNameSet Symbols) {
+    auto R = V.lookup(std::move(Query), Symbols);
    EXPECT_TRUE(R.MaterializationWork.empty())
        << "Query resulted in unexpected materialization work";
    return std::move(R.UnresolvedSymbols);
  };

-  SimpleORCResolver UnderlyingResolver(std::move(LookupFlags),
-                                       std::move(Lookup));
-  JITSymbolResolverAdapter Resolver(ES, UnderlyingResolver);
+  auto UnderlyingResolver =
+      createSymbolResolver(std::move(LookupFlags), std::move(Lookup));
+  JITSymbolResolverAdapter Resolver(ES, *UnderlyingResolver);

  JITSymbolResolver::LookupSet Names{StringRef("foo")};

--- a/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp
+++ b/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp
@ -196,8 +196,9 @@ TEST_F(RTDyldObjectLinkingLayerExecutionTest, NoDuplicateFinalization) {
        return cantFail(
            lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup));
      },
-      [&](AsynchronousSymbolQuery &Query, const SymbolNameSet &Symbols) {
-        return lookupWithLegacyFn(Query, Symbols, LegacyLookup);
+      [&](std::shared_ptr<AsynchronousSymbolQuery> Query,
+          const SymbolNameSet &Symbols) {
+        return lookupWithLegacyFn(*Query, Symbols, LegacyLookup);
      });

  cantFail(ObjLayer.addObject(K2, std::move(Obj2)));