[llvm] r359521 - [ORC] Allow JITDylib definition generators to return Errors.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 29 17:03:27 PDT 2019
Author: lhames
Date: Mon Apr 29 17:03:26 2019
New Revision: 359521
URL: http://llvm.org/viewvc/llvm-project?rev=359521&view=rev
Log:
[ORC] Allow JITDylib definition generators to return Errors.
Background: A definition generator can be attached to a JITDylib to generate
new definitions in response to queries. For example: a generator that forwards
calls to dlsym can map symbols from a dynamic library into the JIT process on
demand.
If definition generation fails then the generator should be able to return an
error. This allows the JIT API to distinguish between the case where a
generator does not provide a definition, and the case where it was not able to
determine whether it provided a definition due to an error.
The immediate motivation for this is cross-process symbol lookups: If the
remote-lookup generator is attached to a JITDylib early in the search list, and
if a generator failure is misinterpreted as "no definition in this JITDylib" then
lookup may continue and bind to a different definition in a later JITDylib, which
is a bug.
Modified:
llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h
llvm/trunk/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h
llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
llvm/trunk/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h?rev=359521&r1=359520&r2=359521&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h Mon Apr 29 17:03:26 2019
@@ -418,7 +418,7 @@ public:
ReexportsGenerator(JITDylib &SourceJD, bool MatchNonExported = false,
SymbolPredicate Allow = SymbolPredicate());
- SymbolNameSet operator()(JITDylib &JD, const SymbolNameSet &Names);
+ Expected<SymbolNameSet> operator()(JITDylib &JD, const SymbolNameSet &Names);
private:
JITDylib &SourceJD;
@@ -497,7 +497,7 @@ class JITDylib {
friend class ExecutionSession;
friend class MaterializationResponsibility;
public:
- using GeneratorFunction = std::function<SymbolNameSet(
+ using GeneratorFunction = std::function<Expected<SymbolNameSet>(
JITDylib &Parent, const SymbolNameSet &Names)>;
using AsynchronousSymbolQuerySet =
@@ -595,7 +595,7 @@ public:
/// Search the given JITDylib for the symbols in Symbols. If found, store
/// the flags for each symbol in Flags. Returns any unresolved symbols.
- SymbolFlagsMap lookupFlags(const SymbolNameSet &Names);
+ Expected<SymbolFlagsMap> lookupFlags(const SymbolNameSet &Names);
/// Dump current JITDylib state to OS.
void dump(raw_ostream &OS);
@@ -608,8 +608,8 @@ public:
/// and the query will not be applied. The Query is not failed and can be
/// re-used in a subsequent lookup once the symbols have been added, or
/// manually failed.
- SymbolNameSet legacyLookup(std::shared_ptr<AsynchronousSymbolQuery> Q,
- SymbolNameSet Names);
+ Expected<SymbolNameSet>
+ legacyLookup(std::shared_ptr<AsynchronousSymbolQuery> Q, SymbolNameSet Names);
private:
using AsynchronousSymbolQueryList =
@@ -645,12 +645,12 @@ private:
Error defineImpl(MaterializationUnit &MU);
- SymbolNameSet lookupFlagsImpl(SymbolFlagsMap &Flags,
- const SymbolNameSet &Names);
+ Expected<SymbolNameSet> lookupFlagsImpl(SymbolFlagsMap &Flags,
+ const SymbolNameSet &Names);
- void lodgeQuery(std::shared_ptr<AsynchronousSymbolQuery> &Q,
- SymbolNameSet &Unresolved, bool MatchNonExported,
- MaterializationUnitList &MUs);
+ Error lodgeQuery(std::shared_ptr<AsynchronousSymbolQuery> &Q,
+ SymbolNameSet &Unresolved, bool MatchNonExported,
+ MaterializationUnitList &MUs);
void lodgeQueryImpl(std::shared_ptr<AsynchronousSymbolQuery> &Q,
SymbolNameSet &Unresolved, bool MatchNonExported,
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h?rev=359521&r1=359520&r2=359521&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h Mon Apr 29 17:03:26 2019
@@ -239,7 +239,7 @@ public:
return Load(nullptr, GlobalPrefix, std::move(Allow));
}
- SymbolNameSet operator()(JITDylib &JD, const SymbolNameSet &Names);
+ Expected<SymbolNameSet> operator()(JITDylib &JD, const SymbolNameSet &Names);
private:
sys::DynamicLibrary Dylib;
Modified: llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp?rev=359521&r1=359520&r2=359521&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp Mon Apr 29 17:03:26 2019
@@ -696,17 +696,20 @@ Expected<SymbolAliasMap>
buildSimpleReexportsAliasMap(JITDylib &SourceJD, const SymbolNameSet &Symbols) {
auto Flags = SourceJD.lookupFlags(Symbols);
- if (Flags.size() != Symbols.size()) {
+ if (!Flags)
+ return Flags.takeError();
+
+ if (Flags->size() != Symbols.size()) {
SymbolNameSet Unresolved = Symbols;
- for (auto &KV : Flags)
+ for (auto &KV : *Flags)
Unresolved.erase(KV.first);
return make_error<SymbolsNotFound>(std::move(Unresolved));
}
SymbolAliasMap Result;
for (auto &Name : Symbols) {
- assert(Flags.count(Name) && "Missing entry in flags map");
- Result[Name] = SymbolAliasMapEntry(Name, Flags[Name]);
+ assert(Flags->count(Name) && "Missing entry in flags map");
+ Result[Name] = SymbolAliasMapEntry(Name, (*Flags)[Name]);
}
return Result;
@@ -718,14 +721,17 @@ ReexportsGenerator::ReexportsGenerator(J
: SourceJD(SourceJD), MatchNonExported(MatchNonExported),
Allow(std::move(Allow)) {}
-SymbolNameSet ReexportsGenerator::operator()(JITDylib &JD,
- const SymbolNameSet &Names) {
+Expected<SymbolNameSet>
+ReexportsGenerator::operator()(JITDylib &JD, const SymbolNameSet &Names) {
orc::SymbolNameSet Added;
orc::SymbolAliasMap AliasMap;
auto Flags = SourceJD.lookupFlags(Names);
- for (auto &KV : Flags) {
+ if (!Flags)
+ return Flags.takeError();
+
+ for (auto &KV : *Flags) {
if (Allow && !Allow(KV.first))
continue;
AliasMap[KV.first] = SymbolAliasMapEntry(KV.first, KV.second);
@@ -1171,16 +1177,23 @@ Error JITDylib::remove(const SymbolNameS
});
}
-SymbolFlagsMap JITDylib::lookupFlags(const SymbolNameSet &Names) {
- return ES.runSessionLocked([&, this]() {
+Expected<SymbolFlagsMap> JITDylib::lookupFlags(const SymbolNameSet &Names) {
+ return ES.runSessionLocked([&, this]() -> Expected<SymbolFlagsMap> {
SymbolFlagsMap Result;
auto Unresolved = lookupFlagsImpl(Result, Names);
- if (DefGenerator && !Unresolved.empty()) {
- auto NewDefs = DefGenerator(*this, Unresolved);
- if (!NewDefs.empty()) {
- auto Unresolved2 = lookupFlagsImpl(Result, NewDefs);
+ if (!Unresolved)
+ return Unresolved.takeError();
+
+ if (DefGenerator && !Unresolved->empty()) {
+ auto NewDefs = DefGenerator(*this, *Unresolved);
+ if (!NewDefs)
+ return NewDefs.takeError();
+ if (!NewDefs->empty()) {
+ auto Unresolved2 = lookupFlagsImpl(Result, *NewDefs);
+ if (!Unresolved2)
+ return Unresolved2.takeError();
(void)Unresolved2;
- assert(Unresolved2.empty() &&
+ assert(Unresolved2->empty() &&
"All fallback defs should have been found by lookupFlagsImpl");
}
};
@@ -1188,8 +1201,8 @@ SymbolFlagsMap JITDylib::lookupFlags(con
});
}
-SymbolNameSet JITDylib::lookupFlagsImpl(SymbolFlagsMap &Flags,
- const SymbolNameSet &Names) {
+Expected<SymbolNameSet> JITDylib::lookupFlagsImpl(SymbolFlagsMap &Flags,
+ const SymbolNameSet &Names) {
SymbolNameSet Unresolved;
for (auto &Name : Names) {
@@ -1207,22 +1220,26 @@ SymbolNameSet JITDylib::lookupFlagsImpl(
return Unresolved;
}
-void JITDylib::lodgeQuery(std::shared_ptr<AsynchronousSymbolQuery> &Q,
- SymbolNameSet &Unresolved, bool MatchNonExported,
- MaterializationUnitList &MUs) {
+Error JITDylib::lodgeQuery(std::shared_ptr<AsynchronousSymbolQuery> &Q,
+ SymbolNameSet &Unresolved, bool MatchNonExported,
+ MaterializationUnitList &MUs) {
assert(Q && "Query can not be null");
lodgeQueryImpl(Q, Unresolved, MatchNonExported, MUs);
if (DefGenerator && !Unresolved.empty()) {
auto NewDefs = DefGenerator(*this, Unresolved);
- if (!NewDefs.empty()) {
- for (auto &D : NewDefs)
+ if (!NewDefs)
+ return NewDefs.takeError();
+ if (!NewDefs->empty()) {
+ for (auto &D : *NewDefs)
Unresolved.erase(D);
- lodgeQueryImpl(Q, NewDefs, MatchNonExported, MUs);
- assert(NewDefs.empty() &&
+ lodgeQueryImpl(Q, *NewDefs, MatchNonExported, MUs);
+ assert(NewDefs->empty() &&
"All fallback defs should have been found by lookupImpl");
}
}
+
+ return Error::success();
}
void JITDylib::lodgeQueryImpl(
@@ -1294,8 +1311,9 @@ void JITDylib::lodgeQueryImpl(
Unresolved.erase(Name);
}
-SymbolNameSet JITDylib::legacyLookup(std::shared_ptr<AsynchronousSymbolQuery> Q,
- SymbolNameSet Names) {
+Expected<SymbolNameSet>
+JITDylib::legacyLookup(std::shared_ptr<AsynchronousSymbolQuery> Q,
+ SymbolNameSet Names) {
assert(Q && "Query can not be null");
ES.runOutstandingMUs();
@@ -1304,22 +1322,28 @@ SymbolNameSet JITDylib::legacyLookup(std
std::vector<std::unique_ptr<MaterializationUnit>> MUs;
SymbolNameSet Unresolved = std::move(Names);
- ES.runSessionLocked([&, this]() {
+ auto Err = ES.runSessionLocked([&, this]() -> Error {
ActionFlags = lookupImpl(Q, MUs, Unresolved);
if (DefGenerator && !Unresolved.empty()) {
assert(ActionFlags == None &&
"ActionFlags set but unresolved symbols remain?");
auto NewDefs = DefGenerator(*this, Unresolved);
- if (!NewDefs.empty()) {
- for (auto &D : NewDefs)
+ if (!NewDefs)
+ return NewDefs.takeError();
+ if (!NewDefs->empty()) {
+ for (auto &D : *NewDefs)
Unresolved.erase(D);
- ActionFlags = lookupImpl(Q, MUs, NewDefs);
- assert(NewDefs.empty() &&
+ ActionFlags = lookupImpl(Q, MUs, *NewDefs);
+ assert(NewDefs->empty() &&
"All fallback defs should have been found by lookupImpl");
}
}
+ return Error::success();
});
+ if (Err)
+ return std::move(Err);
+
assert((MUs.empty() || ActionFlags == None) &&
"If action flags are set, there should be no work to do (so no MUs)");
@@ -1761,34 +1785,29 @@ void ExecutionSession::lookup(
Unresolved, std::move(OnResolve), std::move(OnReady));
bool QueryIsFullyResolved = false;
bool QueryIsFullyReady = false;
- bool QueryFailed = false;
- runSessionLocked([&]() {
- for (auto &KV : SearchOrder) {
- assert(KV.first && "JITDylibList entries must not be null");
- assert(!CollectedMUsMap.count(KV.first) &&
- "JITDylibList should not contain duplicate entries");
-
- auto &JD = *KV.first;
- auto MatchNonExported = KV.second;
- JD.lodgeQuery(Q, Unresolved, MatchNonExported, CollectedMUsMap[&JD]);
- }
-
- if (Unresolved.empty()) {
- // Query lodged successfully.
-
- // Record whether this query is fully ready / resolved. We will use
- // this to call handleFullyResolved/handleFullyReady outside the session
- // lock.
- QueryIsFullyResolved = Q->isFullyResolved();
- QueryIsFullyReady = Q->isFullyReady();
-
- // Call the register dependencies function.
- if (RegisterDependencies && !Q->QueryRegistrations.empty())
- RegisterDependencies(Q->QueryRegistrations);
- } else {
- // Query failed due to unresolved symbols.
- QueryFailed = true;
+ auto LodgingErr = runSessionLocked([&]() -> Error {
+ auto LodgeQuery = [&]() -> Error {
+ for (auto &KV : SearchOrder) {
+ assert(KV.first && "JITDylibList entries must not be null");
+ assert(!CollectedMUsMap.count(KV.first) &&
+ "JITDylibList should not contain duplicate entries");
+
+ auto &JD = *KV.first;
+ auto MatchNonExported = KV.second;
+ if (auto Err = JD.lodgeQuery(Q, Unresolved, MatchNonExported,
+ CollectedMUsMap[&JD]))
+ return Err;
+ }
+
+ if (!Unresolved.empty())
+ return make_error<SymbolsNotFound>(std::move(Unresolved));
+
+ return Error::success();
+ };
+
+ if (auto Err = LodgeQuery()) {
+ // Query failed.
// Disconnect the query from its dependencies.
Q->detach();
@@ -1797,11 +1816,27 @@ void ExecutionSession::lookup(
for (auto &KV : CollectedMUsMap)
for (auto &MU : KV.second)
KV.first->replace(std::move(MU));
+
+ return Err;
}
+
+ // Query lodged successfully.
+
+ // Record whether this query is fully ready / resolved. We will use
+ // this to call handleFullyResolved/handleFullyReady outside the session
+ // lock.
+ QueryIsFullyResolved = Q->isFullyResolved();
+ QueryIsFullyReady = Q->isFullyReady();
+
+ // Call the register dependencies function.
+ if (RegisterDependencies && !Q->QueryRegistrations.empty())
+ RegisterDependencies(Q->QueryRegistrations);
+
+ return Error::success();
});
- if (QueryFailed) {
- Q->handleFailed(make_error<SymbolsNotFound>(std::move(Unresolved)));
+ if (LodgingErr) {
+ Q->handleFailed(std::move(LodgingErr));
return;
} else {
if (QueryIsFullyResolved)
Modified: llvm/trunk/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/ExecutionUtils.cpp?rev=359521&r1=359520&r2=359521&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/ExecutionUtils.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/ExecutionUtils.cpp Mon Apr 29 17:03:26 2019
@@ -193,8 +193,9 @@ DynamicLibrarySearchGenerator::Load(cons
std::move(Allow));
}
-SymbolNameSet DynamicLibrarySearchGenerator::
-operator()(JITDylib &JD, const SymbolNameSet &Names) {
+Expected<SymbolNameSet>
+DynamicLibrarySearchGenerator::operator()(JITDylib &JD,
+ const SymbolNameSet &Names) {
orc::SymbolNameSet Added;
orc::SymbolMap NewSymbols;
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp?rev=359521&r1=359520&r2=359521&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp Mon Apr 29 17:03:26 2019
@@ -216,7 +216,7 @@ TEST_F(CoreAPIsStandardTest, ChainedJITD
OnReadyRun = true;
});
- JD2.legacyLookup(Q, JD.legacyLookup(Q, {Foo}));
+ cantFail(JD2.legacyLookup(Q, cantFail(JD.legacyLookup(Q, {Foo}))));
EXPECT_TRUE(OnResolvedRun) << "OnResolved was not run for empty query";
EXPECT_TRUE(OnReadyRun) << "OnReady was not run for empty query";
@@ -260,7 +260,7 @@ TEST_F(CoreAPIsStandardTest, LookupFlags
SymbolNameSet Names({Foo, Bar, Baz});
- auto SymbolFlags = JD.lookupFlags(Names);
+ auto SymbolFlags = cantFail(JD.lookupFlags(Names));
EXPECT_EQ(SymbolFlags.size(), 2U)
<< "Returned symbol flags contains unexpected results";
@@ -273,6 +273,26 @@ TEST_F(CoreAPIsStandardTest, LookupFlags
<< "Incorrect flags returned for Bar";
}
+TEST_F(CoreAPIsStandardTest, LookupWithGeneratorFailure) {
+
+ class BadGenerator {
+ public:
+ Expected<SymbolNameSet> operator()(JITDylib &, const SymbolNameSet &) {
+ return make_error<StringError>("BadGenerator", inconvertibleErrorCode());
+ }
+ };
+
+ JD.setGenerator(BadGenerator());
+
+ EXPECT_THAT_ERROR(JD.lookupFlags({Foo}).takeError(), Failed<StringError>())
+ << "Generator failure did not propagate through lookupFlags";
+
+ EXPECT_THAT_ERROR(
+ ES.lookup(JITDylibSearchList({{&JD, false}}), {Foo}).takeError(),
+ Failed<StringError>())
+ << "Generator failure did not propagate through lookup";
+}
+
TEST_F(CoreAPIsStandardTest, TestBasicAliases) {
cantFail(JD.define(absoluteSymbols({{Foo, FooSym}, {Bar, BarSym}})));
cantFail(JD.define(symbolAliases({{Baz, {Foo, JITSymbolFlags::Exported}},
@@ -356,7 +376,7 @@ TEST_F(CoreAPIsStandardTest, TestReexpor
JD.setGenerator(ReexportsGenerator(JD2, false, Filter));
- auto Flags = JD.lookupFlags({Foo, Bar, Baz});
+ auto Flags = cantFail(JD.lookupFlags({Foo, Bar, Baz}));
EXPECT_EQ(Flags.size(), 1U) << "Unexpected number of results";
EXPECT_EQ(Flags[Foo], FooSym.getFlags()) << "Unexpected flags for Foo";
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp?rev=359521&r1=359520&r2=359521&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp Mon Apr 29 17:03:26 2019
@@ -24,7 +24,7 @@ TEST_F(LegacyAPIsStandardTest, TestLambd
auto Resolver = createSymbolResolver(
[&](const SymbolNameSet &Symbols) {
- auto FlagsMap = JD.lookupFlags(Symbols);
+ auto FlagsMap = cantFail(JD.lookupFlags(Symbols));
SymbolNameSet Result;
for (auto &KV : FlagsMap)
if (!KV.second.isStrong())
@@ -32,7 +32,7 @@ TEST_F(LegacyAPIsStandardTest, TestLambd
return Result;
},
[&](std::shared_ptr<AsynchronousSymbolQuery> Q, SymbolNameSet Symbols) {
- return JD.legacyLookup(std::move(Q), Symbols);
+ return cantFail(JD.legacyLookup(std::move(Q), Symbols));
});
auto RS = Resolver->getResponsibilitySet(SymbolNameSet({Bar, Baz}));
More information about the llvm-commits
mailing list