-
Notifications
You must be signed in to change notification settings - Fork 15.8k
[clang-repl] Use more precise search to find the orc runtime. #175805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang-repl] Use more precise search to find the orc runtime. #175805
Conversation
The new mechanism relies on the path in the toolchain which should be the autoritative answer. This patch tweaks the discovery of the orc runtime from unittests where the resource directory is hard to deduce.
|
@llvm/pr-subscribers-clang Author: Vassil Vassilev (vgvassilev) ChangesThe new mechanism relies on the path in the toolchain which should be the autoritative answer. This patch tweaks the discovery of the orc runtime from unittests where the resource directory is hard to deduce. Should address the issue raised in #175435 and #175322 Full diff: https://github.com/llvm/llvm-project/pull/175805.diff 1 Files Affected:
diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp
index 74b751d0e2dae..71a54dc7d3809 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -420,90 +420,95 @@ llvm::Error IncrementalExecutorBuilder::UpdateOrcRuntimePath(
if (!IsOutOfProcess)
return llvm::Error::success();
- static constexpr std::array<const char *, 3> OrcRTLibNames = {
- "liborc_rt.a",
- "liborc_rt_osx.a",
- "liborc_rt-x86_64.a",
- };
+ const clang::driver::Driver &D = C.getDriver();
+ const clang::driver::ToolChain &TC = C.getDefaultToolChain();
+
+ llvm::SmallVector<std::string, 2> OrcRTLibNames;
+
+ // Get canonical compiler-rt path
+ std::string CompilerRTPath = TC.getCompilerRT(C.getArgs(), "orc_rt");
+ llvm::StringRef CanonicalFilename = llvm::sys::path::filename(CompilerRTPath);
+
+ if (CanonicalFilename.empty()) {
+ return llvm::make_error<llvm::StringError>(
+ "Could not determine OrcRuntime filename from ToolChain",
+ llvm::inconvertibleErrorCode());
+ }
- auto findInDir = [&](llvm::StringRef Base) -> std::optional<std::string> {
- if (Base.empty() || !llvm::sys::fs::exists(Base))
- return std::nullopt;
- for (const char *LibName : OrcRTLibNames) {
- llvm::SmallString<256> Candidate(Base);
- llvm::sys::path::append(Candidate, LibName);
- if (llvm::sys::fs::exists(Candidate))
- return std::string(Candidate.str());
+ OrcRTLibNames.push_back(CanonicalFilename.str());
+
+ // Derive legacy spelling (libclang_rt.orc_rt -> orc_rt)
+ llvm::StringRef LegacySuffix = CanonicalFilename;
+ if (LegacySuffix.consume_front("libclang_rt.")) {
+ OrcRTLibNames.push_back(("lib" + LegacySuffix).str());
+ }
+
+ // Extract directory
+ llvm::SmallString<256> OrcRTDir(CompilerRTPath);
+ llvm::sys::path::remove_filename(OrcRTDir);
+
+ llvm::SmallVector<std::string, 8> triedPaths;
+
+ auto findInDir = [&](llvm::StringRef Dir) -> std::optional<std::string> {
+ for (const auto &LibName : OrcRTLibNames) {
+ llvm::SmallString<256> FullPath = Dir;
+ llvm::sys::path::append(FullPath, LibName);
+ if (llvm::sys::fs::exists(FullPath))
+ return std::string(FullPath.str());
+ triedPaths.push_back(std::string(FullPath.str()));
}
return std::nullopt;
};
- const clang::driver::Driver &D = C.getDriver();
- const clang::driver::ToolChain &TC = C.getDefaultToolChain();
- llvm::SmallVector<std::string, 8> triedPaths;
+ // Try the primary directory first
+ if (auto Found = findInDir(OrcRTDir)) {
+ OrcRuntimePath = *Found;
+ return llvm::Error::success();
+ }
- llvm::SmallString<256> Resource(D.ResourceDir);
- if (llvm::sys::fs::exists(Resource)) {
- // Ask the ToolChain for its runtime paths first (most authoritative).
- for (auto RuntimePath :
- {TC.getRuntimePath(), std::make_optional(TC.getCompilerRTPath())}) {
- if (RuntimePath) {
- if (auto Found = findInDir(*RuntimePath)) {
- OrcRuntimePath = *Found;
- return llvm::Error::success();
- }
- triedPaths.emplace_back(*RuntimePath);
- }
- }
+ // We want to find the relative path from the Driver to the OrcRTDir
+ // to replicate that structure elsewhere if needed.
+ llvm::StringRef Rel = OrcRTDir.str();
+ if (!Rel.consume_front(llvm::sys::path::parent_path(D.Dir))) {
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv("OrcRuntime library path ({0}) is not located within the "
+ "Clang resource directory ({1}). Check your installation "
+ "or provide an explicit path via -resource-dir.",
+ OrcRTDir, D.Dir)
+ .str(),
+ llvm::inconvertibleErrorCode());
+ }
- // Check ResourceDir and ResourceDir/lib
- for (auto P : {Resource.str().str(), (Resource + "/lib").str()}) {
- if (auto F = findInDir(P)) {
- OrcRuntimePath = *F;
- return llvm::Error::success();
- }
- triedPaths.emplace_back(P);
- }
- } else {
- // The binary was misplaced. Generic Backward Search (Climbing the tree)
- // This allows unit tests in tools/clang/unittests to find the real lib/
- llvm::SmallString<256> Cursor = Resource;
- // ResourceDir-derived locations
- llvm::StringRef Version = llvm::sys::path::filename(Resource);
- llvm::StringRef OSName = TC.getOSLibName();
- while (llvm::sys::path::has_parent_path(Cursor)) {
- Cursor = llvm::sys::path::parent_path(Cursor).str();
- // At each level, try standard relative layouts
- for (auto Rel :
- {(llvm::Twine("lib/clang/") + Version + "/lib/" + OSName).str(),
- (llvm::Twine("lib/clang/") + Version + "/lib").str(),
- (llvm::Twine("lib/") + OSName).str(), std::string("lib/clang")}) {
- llvm::SmallString<256> Candidate = Cursor;
- llvm::sys::path::append(Candidate, Rel);
- if (auto F = findInDir(Candidate)) {
- OrcRuntimePath = *F;
- return llvm::Error::success();
- }
- triedPaths.emplace_back(std::string(Candidate.str()));
- }
- // Stop if we hit the root or go too far (safety check)
- if (triedPaths.size() > 32)
- break;
+ // Generic Backward Search (Climbing the tree)
+ // This is useful for unit tests or relocated toolchains
+ llvm::SmallString<256> Cursor(D.Dir); // Start from the driver directory
+ while (llvm::sys::path::has_parent_path(Cursor)) {
+ Cursor = llvm::sys::path::parent_path(Cursor).str();
+ llvm::SmallString<256> Candidate = Cursor;
+ llvm::sys::path::append(Candidate, Rel);
+
+ if (auto Found = findInDir(Candidate)) {
+ OrcRuntimePath = *Found;
+ return llvm::Error::success();
}
+
+ // Safety check
+ if (triedPaths.size() > 32)
+ break;
}
- // Build a helpful error string if everything failed.
+ // Build a helpful error string
std::string Joined;
for (size_t i = 0; i < triedPaths.size(); ++i) {
- if (i)
- Joined += ", ";
+ if (i > 0)
+ Joined += "\n "; // Use newlines for better readability
Joined += triedPaths[i];
}
- if (Joined.empty())
- Joined = "<no candidate paths available>";
return llvm::make_error<llvm::StringError>(
- llvm::formatv("OrcRuntime library not found in: {0}", Joined).str(),
+ llvm::formatv("OrcRuntime library not found. Checked:\n {0}",
+ Joined.empty() ? "<none>" : Joined)
+ .str(),
llvm::inconvertibleErrorCode());
}
|
chapuni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've confirmed this satisfies my case. Thanks!
|
Thank you, @chapuni! |
|
This should be fed to the release 22. |
|
Adding the PR to the release milestone is good enough nowadays, right? |
|
Ah, okay, done: #175915 |
The new mechanism relies on the path in the toolchain which should be the autoritative answer. This patch tweaks the discovery of the orc runtime from unittests where the resource directory is hard to deduce.
Should address the issue raised in #175435 and #175322