Skip to content

Commit cc8f7cd

Browse files
authored
[ORC][LibraryResolver] Fix ensureFilterBuilt assertion failure and concurrency issue. (#166510)
- Fixed architecture compatibility check. Previously, we used `sys::getDefaultTriple()`, which caused issues on build bots using cross-compilation. We now ensure that the target architecture where the shared library (.so) is run or loaded matches the architecture it was built for. - Fixed ensureFilterBuilt assertion failure. - Replaced use of FilteredView with a safer alternative for concurrent environments. The old FilteredView approach iterated over shared state without sufficient synchronization, which could lead to invalid accesses when libraries were being added or removed concurrently.
1 parent 5e7f7a4 commit cc8f7cd

File tree

3 files changed

+33
-21
lines changed

3 files changed

+33
-21
lines changed

llvm/include/llvm/ExecutionEngine/Orc/TargetProcess/LibraryResolver.h

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,21 @@ class LibraryManager {
211211
return FilteredView(Libraries.begin(), Libraries.end(), S, K);
212212
}
213213

214+
using LibraryFilterFn = std::function<bool(const LibraryInfo &)>;
215+
void getLibraries(LibState S, PathType K,
216+
std::vector<std::shared_ptr<LibraryInfo>> &Outs,
217+
LibraryFilterFn Filter = nullptr) const {
218+
std::shared_lock<std::shared_mutex> Lock(Mtx);
219+
for (const auto &[_, Entry] : Libraries) {
220+
const auto &Info = *Entry;
221+
if (Info.getKind() != K || Info.getState() != S)
222+
continue;
223+
if (Filter && !Filter(Info))
224+
continue;
225+
Outs.push_back(Entry);
226+
}
227+
}
228+
214229
void forEachLibrary(const LibraryVisitor &visitor) const {
215230
std::unique_lock<std::shared_mutex> Lock(Mtx);
216231
for (const auto &[_, entry] : Libraries) {
@@ -220,14 +235,14 @@ class LibraryManager {
220235
}
221236

222237
bool isLoaded(StringRef Path) const {
223-
std::unique_lock<std::shared_mutex> Lock(Mtx);
238+
std::shared_lock<std::shared_mutex> Lock(Mtx);
224239
if (auto It = Libraries.find(Path.str()); It != Libraries.end())
225240
return It->second->getState() == LibState::Loaded;
226241
return false;
227242
}
228243

229244
bool isQueried(StringRef Path) const {
230-
std::unique_lock<std::shared_mutex> Lock(Mtx);
245+
std::shared_lock<std::shared_mutex> Lock(Mtx);
231246
if (auto It = Libraries.find(Path.str()); It != Libraries.end())
232247
return It->second->getState() == LibState::Queried;
233248
return false;

llvm/lib/ExecutionEngine/Orc/TargetProcess/LibraryResolver.cpp

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -184,17 +184,17 @@ class SymbolSearchContext {
184184
public:
185185
SymbolSearchContext(SymbolQuery &Q) : Q(Q) {}
186186

187-
bool hasSearched(LibraryInfo *Lib) const { return Searched.count(Lib); }
187+
bool hasSearched(const LibraryInfo *Lib) const { return Searched.count(Lib); }
188188

189-
void markSearched(LibraryInfo *Lib) { Searched.insert(Lib); }
189+
void markSearched(const LibraryInfo *Lib) { Searched.insert(Lib); }
190190

191191
inline bool allResolved() const { return Q.allResolved(); }
192192

193193
SymbolQuery &query() { return Q; }
194194

195195
private:
196196
SymbolQuery &Q;
197-
DenseSet<LibraryInfo *> Searched;
197+
DenseSet<const LibraryInfo *> Searched;
198198
};
199199

200200
void LibraryResolver::resolveSymbolsInLibrary(
@@ -226,19 +226,18 @@ void LibraryResolver::resolveSymbolsInLibrary(
226226
return EnumerateResult::Continue;
227227
},
228228
Opts);
229+
};
229230

231+
if (!Lib.hasFilter()) {
232+
LLVM_DEBUG(dbgs() << "Building filter for library: " << Lib.getFullPath()
233+
<< "\n";);
234+
enumerateSymbolsIfNeeded();
230235
if (DiscoveredSymbols.empty()) {
231236
LLVM_DEBUG(dbgs() << " No symbols and remove library : "
232237
<< Lib.getFullPath() << "\n";);
233238
LibMgr.removeLibrary(Lib.getFullPath());
234239
return;
235240
}
236-
};
237-
238-
if (!Lib.hasFilter()) {
239-
LLVM_DEBUG(dbgs() << "Building filter for library: " << Lib.getFullPath()
240-
<< "\n";);
241-
enumerateSymbolsIfNeeded();
242241
SmallVector<StringRef> SymbolVec;
243242
SymbolVec.reserve(DiscoveredSymbols.size());
244243
for (const auto &KV : DiscoveredSymbols)
@@ -288,24 +287,22 @@ void LibraryResolver::searchSymbolsInLibraries(
288287

289288
SymbolSearchContext Ctx(Q);
290289
while (!Ctx.allResolved()) {
290+
std::vector<std::shared_ptr<LibraryInfo>> Libs;
291+
LibMgr.getLibraries(S, K, Libs, [&](const LibraryInfo &Lib) {
292+
return !Ctx.hasSearched(&Lib);
293+
});
291294

292-
for (auto &Lib : LibMgr.getView(S, K)) {
293-
if (Ctx.hasSearched(Lib.get()))
294-
continue;
295+
if (Libs.empty() && !scanLibrariesIfNeeded(K, scanBatchSize))
296+
break; // no more new libs to scan
295297

298+
for (auto &Lib : Libs) {
296299
// can use Async here?
297300
resolveSymbolsInLibrary(*Lib, Ctx.query(), Config.Options);
298301
Ctx.markSearched(Lib.get());
299302

300303
if (Ctx.allResolved())
301304
return;
302305
}
303-
304-
if (Ctx.allResolved())
305-
return;
306-
307-
if (!scanLibrariesIfNeeded(K, scanBatchSize))
308-
break; // no more new libs to scan
309306
}
310307
};
311308

llvm/lib/ExecutionEngine/Orc/TargetProcess/LibraryScanner.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ void handleError(Error Err, StringRef context = "") {
5050
}
5151

5252
bool ObjectFileLoader::isArchitectureCompatible(const object::ObjectFile &Obj) {
53-
Triple HostTriple(sys::getDefaultTargetTriple());
53+
Triple HostTriple(sys::getProcessTriple());
5454
Triple ObjTriple = Obj.makeTriple();
5555

5656
LLVM_DEBUG({

0 commit comments

Comments
 (0)