[llvm] 9eb4939 - [ORC] Allow JITDylib::getDFSLinkOrder and friends to fail for defunct JITDylibs.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 19 22:51:48 PST 2022
Author: Lang Hames
Date: 2022-01-20T17:45:32+11:00
New Revision: 9eb4939b862a2c0ab6690c3e8526366594722010
URL: https://github.com/llvm/llvm-project/commit/9eb4939b862a2c0ab6690c3e8526366594722010
DIFF: https://github.com/llvm/llvm-project/commit/9eb4939b862a2c0ab6690c3e8526366594722010.diff
LOG: [ORC] Allow JITDylib::getDFSLinkOrder and friends to fail for defunct JITDylibs.
Calls to JITDylib's getDFSLinkOrder and getReverseDFSLinkOrder methods (both
static an non-static versions) are now valid to make on defunct JITDylibs, but
will return an error if any JITDylib in the link order is defunct.
This means that platforms can safely lookup link orders by name in response to
jit-dlopen calls from the ORC runtime, even if the call names a defunct
JITDylib -- the call will just fail with an error.
Added:
Modified:
llvm/include/llvm/ExecutionEngine/Orc/Core.h
llvm/lib/ExecutionEngine/Orc/Core.cpp
llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/Core.h b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
index c5d1c4a51ef1e..d0168f79e3d82 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/Core.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
@@ -1120,32 +1120,33 @@ class JITDylib : public ThreadSafeRefCountedBase<JITDylib>,
/// DFS order (based on linkage relationships). Each JITDylib will appear
/// only once.
///
- /// It is illegal to call this method on a defunct JITDylib and the client
- /// is responsible for ensuring that they do not do so.
- static std::vector<JITDylibSP> getDFSLinkOrder(ArrayRef<JITDylibSP> JDs);
+ /// If any JITDylib in the order is defunct then this method will return an
+ /// error, otherwise returns the order.
+ static Expected<std::vector<JITDylibSP>>
+ getDFSLinkOrder(ArrayRef<JITDylibSP> JDs);
/// Returns the given JITDylibs and all of their transitive dependencies in
/// reverse DFS order (based on linkage relationships). Each JITDylib will
/// appear only once.
///
- /// It is illegal to call this method on a defunct JITDylib and the client
- /// is responsible for ensuring that they do not do so.
- static std::vector<JITDylibSP>
+ /// If any JITDylib in the order is defunct then this method will return an
+ /// error, otherwise returns the order.
+ static Expected<std::vector<JITDylibSP>>
getReverseDFSLinkOrder(ArrayRef<JITDylibSP> JDs);
/// Return this JITDylib and its transitive dependencies in DFS order
/// based on linkage relationships.
///
- /// It is illegal to call this method on a defunct JITDylib and the client
- /// is responsible for ensuring that they do not do so.
- std::vector<JITDylibSP> getDFSLinkOrder();
+ /// If any JITDylib in the order is defunct then this method will return an
+ /// error, otherwise returns the order.
+ Expected<std::vector<JITDylibSP>> getDFSLinkOrder();
/// Rteurn this JITDylib and its transitive dependencies in reverse DFS order
/// based on linkage relationships.
///
- /// It is illegal to call this method on a defunct JITDylib and the client
- /// is responsible for ensuring that they do not do so.
- std::vector<JITDylibSP> getReverseDFSLinkOrder();
+ /// If any JITDylib in the order is defunct then this method will return an
+ /// error, otherwise returns the order.
+ Expected<std::vector<JITDylibSP>> getReverseDFSLinkOrder();
private:
using AsynchronousSymbolQuerySet =
diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp
index c6933cff40bc5..e5cb8103919a7 100644
--- a/llvm/lib/ExecutionEngine/Orc/Core.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -1958,19 +1958,22 @@ Error ExecutionSession::removeJITDylib(JITDylib &JD) {
return Err;
}
-std::vector<JITDylibSP> JITDylib::getDFSLinkOrder(ArrayRef<JITDylibSP> JDs) {
+Expected<std::vector<JITDylibSP>>
+JITDylib::getDFSLinkOrder(ArrayRef<JITDylibSP> JDs) {
if (JDs.empty())
- return {};
+ return std::vector<JITDylibSP>();
auto &ES = JDs.front()->getExecutionSession();
- return ES.runSessionLocked([&]() {
+ return ES.runSessionLocked([&]() -> Expected<std::vector<JITDylibSP>> {
DenseSet<JITDylib *> Visited;
std::vector<JITDylibSP> Result;
for (auto &JD : JDs) {
- assert(JD->State == Open && "JD is defunct");
-
+ if (JD->State != Open)
+ return make_error<StringError>(
+ "Error building link order: " + JD->getName() + " is defunct",
+ inconvertibleErrorCode());
if (Visited.count(JD.get()))
continue;
@@ -1995,18 +1998,19 @@ std::vector<JITDylibSP> JITDylib::getDFSLinkOrder(ArrayRef<JITDylibSP> JDs) {
});
}
-std::vector<JITDylibSP>
+Expected<std::vector<JITDylibSP>>
JITDylib::getReverseDFSLinkOrder(ArrayRef<JITDylibSP> JDs) {
- auto Tmp = getDFSLinkOrder(JDs);
- std::reverse(Tmp.begin(), Tmp.end());
- return Tmp;
+ auto Result = getDFSLinkOrder(JDs);
+ if (Result)
+ std::reverse(Result->begin(), Result->end());
+ return Result;
}
-std::vector<JITDylibSP> JITDylib::getDFSLinkOrder() {
+Expected<std::vector<JITDylibSP>> JITDylib::getDFSLinkOrder() {
return getDFSLinkOrder({this});
}
-std::vector<JITDylibSP> JITDylib::getReverseDFSLinkOrder() {
+Expected<std::vector<JITDylibSP>> JITDylib::getReverseDFSLinkOrder() {
return getReverseDFSLinkOrder({this});
}
diff --git a/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
index 2552955197316..d02760703f067 100644
--- a/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
@@ -320,9 +320,14 @@ void ELFNixPlatform::getInitializersLookupPhase(
SendInitializerSequenceFn SendResult, JITDylib &JD) {
auto DFSLinkOrder = JD.getDFSLinkOrder();
+ if (!DFSLinkOrder) {
+ SendResult(DFSLinkOrder.takeError());
+ return;
+ }
+
DenseMap<JITDylib *, SymbolLookupSet> NewInitSymbols;
ES.runSessionLocked([&]() {
- for (auto &InitJD : DFSLinkOrder) {
+ for (auto &InitJD : *DFSLinkOrder) {
auto RISItr = RegisteredInitSymbols.find(InitJD.get());
if (RISItr != RegisteredInitSymbols.end()) {
NewInitSymbols[InitJD.get()] = std::move(RISItr->second);
@@ -335,7 +340,7 @@ void ELFNixPlatform::getInitializersLookupPhase(
// phase.
if (NewInitSymbols.empty()) {
getInitializersBuildSequencePhase(std::move(SendResult), JD,
- std::move(DFSLinkOrder));
+ std::move(*DFSLinkOrder));
return;
}
diff --git a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
index cffc340b3b48f..4e6ec445dd2b4 100644
--- a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
@@ -277,17 +277,22 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
std::vector<JITDylibSP> DFSLinkOrder;
- getExecutionSession().runSessionLocked([&]() {
- DFSLinkOrder = JD.getDFSLinkOrder();
-
- for (auto &NextJD : DFSLinkOrder) {
- auto IFItr = InitFunctions.find(NextJD.get());
- if (IFItr != InitFunctions.end()) {
- LookupSymbols[NextJD.get()] = std::move(IFItr->second);
- InitFunctions.erase(IFItr);
- }
- }
- });
+ if (auto Err = getExecutionSession().runSessionLocked([&]() -> Error {
+ if (auto DFSLinkOrderOrErr = JD.getDFSLinkOrder())
+ DFSLinkOrder = std::move(*DFSLinkOrderOrErr);
+ else
+ return DFSLinkOrderOrErr.takeError();
+
+ for (auto &NextJD : DFSLinkOrder) {
+ auto IFItr = InitFunctions.find(NextJD.get());
+ if (IFItr != InitFunctions.end()) {
+ LookupSymbols[NextJD.get()] = std::move(IFItr->second);
+ InitFunctions.erase(IFItr);
+ }
+ }
+ return Error::success();
+ }))
+ return Err;
LLVM_DEBUG({
dbgs() << "JITDylib init order is [ ";
@@ -327,20 +332,25 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
std::vector<JITDylibSP> DFSLinkOrder;
- ES.runSessionLocked([&]() {
- DFSLinkOrder = JD.getDFSLinkOrder();
-
- for (auto &NextJD : DFSLinkOrder) {
- auto &JDLookupSymbols = LookupSymbols[NextJD.get()];
- auto DIFItr = DeInitFunctions.find(NextJD.get());
- if (DIFItr != DeInitFunctions.end()) {
- LookupSymbols[NextJD.get()] = std::move(DIFItr->second);
- DeInitFunctions.erase(DIFItr);
- }
- JDLookupSymbols.add(LLJITRunAtExits,
- SymbolLookupFlags::WeaklyReferencedSymbol);
- }
- });
+ if (auto Err = ES.runSessionLocked([&]() -> Error {
+ if (auto DFSLinkOrderOrErr = JD.getDFSLinkOrder())
+ DFSLinkOrder = std::move(*DFSLinkOrderOrErr);
+ else
+ return DFSLinkOrderOrErr.takeError();
+
+ for (auto &NextJD : DFSLinkOrder) {
+ auto &JDLookupSymbols = LookupSymbols[NextJD.get()];
+ auto DIFItr = DeInitFunctions.find(NextJD.get());
+ if (DIFItr != DeInitFunctions.end()) {
+ LookupSymbols[NextJD.get()] = std::move(DIFItr->second);
+ DeInitFunctions.erase(DIFItr);
+ }
+ JDLookupSymbols.add(LLJITRunAtExits,
+ SymbolLookupFlags::WeaklyReferencedSymbol);
+ }
+ return Error::success();
+ }))
+ return Err;
LLVM_DEBUG({
dbgs() << "JITDylib deinit order is [ ";
@@ -381,17 +391,22 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
DenseMap<JITDylib *, SymbolLookupSet> RequiredInitSymbols;
std::vector<JITDylibSP> DFSLinkOrder;
- getExecutionSession().runSessionLocked([&]() {
- DFSLinkOrder = JD.getDFSLinkOrder();
-
- for (auto &NextJD : DFSLinkOrder) {
- auto ISItr = InitSymbols.find(NextJD.get());
- if (ISItr != InitSymbols.end()) {
- RequiredInitSymbols[NextJD.get()] = std::move(ISItr->second);
- InitSymbols.erase(ISItr);
- }
- }
- });
+ if (auto Err = getExecutionSession().runSessionLocked([&]() -> Error {
+ if (auto DFSLinkOrderOrErr = JD.getDFSLinkOrder())
+ DFSLinkOrder = std::move(*DFSLinkOrderOrErr);
+ else
+ return DFSLinkOrderOrErr.takeError();
+
+ for (auto &NextJD : DFSLinkOrder) {
+ auto ISItr = InitSymbols.find(NextJD.get());
+ if (ISItr != InitSymbols.end()) {
+ RequiredInitSymbols[NextJD.get()] = std::move(ISItr->second);
+ InitSymbols.erase(ISItr);
+ }
+ }
+ return Error::success();
+ }))
+ return Err;
return Platform::lookupInitSymbols(getExecutionSession(),
RequiredInitSymbols)
diff --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index 9df4c8cc3d2df..a364719855b40 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -382,9 +382,14 @@ void MachOPlatform::getInitializersLookupPhase(
SendInitializerSequenceFn SendResult, JITDylib &JD) {
auto DFSLinkOrder = JD.getDFSLinkOrder();
+ if (!DFSLinkOrder) {
+ SendResult(DFSLinkOrder.takeError());
+ return;
+ }
+
DenseMap<JITDylib *, SymbolLookupSet> NewInitSymbols;
ES.runSessionLocked([&]() {
- for (auto &InitJD : DFSLinkOrder) {
+ for (auto &InitJD : *DFSLinkOrder) {
auto RISItr = RegisteredInitSymbols.find(InitJD.get());
if (RISItr != RegisteredInitSymbols.end()) {
NewInitSymbols[InitJD.get()] = std::move(RISItr->second);
@@ -397,7 +402,7 @@ void MachOPlatform::getInitializersLookupPhase(
// phase.
if (NewInitSymbols.empty()) {
getInitializersBuildSequencePhase(std::move(SendResult), JD,
- std::move(DFSLinkOrder));
+ std::move(*DFSLinkOrder));
return;
}
diff --git a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
index 64a61d872e385..2344878417f26 100644
--- a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
@@ -1426,21 +1426,21 @@ TEST(JITDylibTest, GetDFSLinkOrderTree) {
LibB.setLinkOrder(makeJITDylibSearchOrder({&LibD, &LibE}));
LibC.setLinkOrder(makeJITDylibSearchOrder({&LibF}));
- auto DFSOrderFromB = JITDylib::getDFSLinkOrder({&LibB});
+ auto DFSOrderFromB = cantFail(JITDylib::getDFSLinkOrder({&LibB}));
EXPECT_TRUE(linkOrdersEqual(DFSOrderFromB, {&LibB, &LibD, &LibE}))
<< "Incorrect DFS link order for LibB";
- auto DFSOrderFromA = JITDylib::getDFSLinkOrder({&LibA});
+ auto DFSOrderFromA = cantFail(JITDylib::getDFSLinkOrder({&LibA}));
EXPECT_TRUE(linkOrdersEqual(DFSOrderFromA,
{&LibA, &LibB, &LibD, &LibE, &LibC, &LibF}))
<< "Incorrect DFS link order for libA";
- auto DFSOrderFromAB = JITDylib::getDFSLinkOrder({&LibA, &LibB});
+ auto DFSOrderFromAB = cantFail(JITDylib::getDFSLinkOrder({&LibA, &LibB}));
EXPECT_TRUE(linkOrdersEqual(DFSOrderFromAB,
{&LibA, &LibB, &LibD, &LibE, &LibC, &LibF}))
<< "Incorrect DFS link order for { libA, libB }";
- auto DFSOrderFromBA = JITDylib::getDFSLinkOrder({&LibB, &LibA});
+ auto DFSOrderFromBA = cantFail(JITDylib::getDFSLinkOrder({&LibB, &LibA}));
EXPECT_TRUE(linkOrdersEqual(DFSOrderFromBA,
{&LibB, &LibD, &LibE, &LibA, &LibC, &LibF}))
<< "Incorrect DFS link order for { libB, libA }";
@@ -1463,7 +1463,7 @@ TEST(JITDylibTest, GetDFSLinkOrderDiamond) {
LibB.setLinkOrder(makeJITDylibSearchOrder({&LibD}));
LibC.setLinkOrder(makeJITDylibSearchOrder({&LibD}));
- auto DFSOrderFromA = JITDylib::getDFSLinkOrder({&LibA});
+ auto DFSOrderFromA = cantFail(JITDylib::getDFSLinkOrder({&LibA}));
EXPECT_TRUE(linkOrdersEqual(DFSOrderFromA, {&LibA, &LibB, &LibD, &LibC}))
<< "Incorrect DFS link order for libA";
}
@@ -1483,15 +1483,15 @@ TEST(JITDylibTest, GetDFSLinkOrderCycle) {
LibB.setLinkOrder(makeJITDylibSearchOrder({&LibC}));
LibC.setLinkOrder(makeJITDylibSearchOrder({&LibA}));
- auto DFSOrderFromA = JITDylib::getDFSLinkOrder({&LibA});
+ auto DFSOrderFromA = cantFail(JITDylib::getDFSLinkOrder({&LibA}));
EXPECT_TRUE(linkOrdersEqual(DFSOrderFromA, {&LibA, &LibB, &LibC}))
<< "Incorrect DFS link order for libA";
- auto DFSOrderFromB = JITDylib::getDFSLinkOrder({&LibB});
+ auto DFSOrderFromB = cantFail(JITDylib::getDFSLinkOrder({&LibB}));
EXPECT_TRUE(linkOrdersEqual(DFSOrderFromB, {&LibB, &LibC, &LibA}))
<< "Incorrect DFS link order for libB";
- auto DFSOrderFromC = JITDylib::getDFSLinkOrder({&LibC});
+ auto DFSOrderFromC = cantFail(JITDylib::getDFSLinkOrder({&LibC}));
EXPECT_TRUE(linkOrdersEqual(DFSOrderFromC, {&LibC, &LibA, &LibB}))
<< "Incorrect DFS link order for libC";
}
@@ -1546,6 +1546,8 @@ TEST_F(CoreAPIsStandardTest, RemoveJITDylibs) {
EXPECT_THAT_ERROR(BazMR->notifyResolved({{Baz, BazSym}}), Failed());
+ EXPECT_THAT_EXPECTED(JD.getDFSLinkOrder(), Failed());
+
BazMR->failMaterialization();
}
More information about the llvm-commits
mailing list