caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[ORC] Allow JITDylib::getDFSLinkOrder and friends to fail for defunct JITDylibs.

Browse Source 
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.
This commit is contained in:
Lang Hames
2022-01-20 14:21:36 +11:00
parent a7a4115bf3
commit 9eb4939b86
6 changed files with 100 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
llvm/include/llvm/ExecutionEngine/Orc/Core.h
View File

@@ -1120,32 +1120,33 @@ public:
/// 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 =

26
llvm/lib/ExecutionEngine/Orc/Core.cpp
View File

@@ -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});
}

9
llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
View File

@@ -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;
}

81
llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
View File

@@ -277,17 +277,22 @@ private:
DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
std::vector<JITDylibSP> DFSLinkOrder;
getExecutionSession().runSessionLocked([&]() {
DFSLinkOrder = JD.getDFSLinkOrder();
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);
}
}
});
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 @@ private:
DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
std::vector<JITDylibSP> DFSLinkOrder;
ES.runSessionLocked([&]() {
DFSLinkOrder = JD.getDFSLinkOrder();
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);
}
});
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 @@ private:
DenseMap<JITDylib *, SymbolLookupSet> RequiredInitSymbols;
std::vector<JITDylibSP> DFSLinkOrder;
getExecutionSession().runSessionLocked([&]() {
DFSLinkOrder = JD.getDFSLinkOrder();
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);
}
}
});
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)

9
llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
View File

@@ -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;
}

18
llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
View File

@@ -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();
}