aprantl updated this revision to Diff 478762.
aprantl added a comment.
Herald added a subscriber: fedor.sergeev.
Don't introduce a Host->Core dependency. Cache the errors and remove Progress
reports (since they also depend on Core).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138060/new/
https://reviews.llvm.org/D138060
Files:
lldb/include/lldb/Host/HostInfoBase.h
lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
lldb/source/Core/Module.cpp
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
lldb/unittests/Host/CMakeLists.txt
lldb/unittests/Host/HostInfoTest.cpp
Index: lldb/unittests/Host/HostInfoTest.cpp
===================================================================
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -56,11 +56,21 @@
#if defined(__APPLE__)
TEST_F(HostInfoTest, GetXcodeSDK) {
- EXPECT_FALSE(HostInfo::GetXcodeSDKPath(XcodeSDK("MacOSX.sdk")).empty());
+ auto get_sdk = [](std::string sdk, bool error = false) -> llvm::StringRef {
+ auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(sdk)));
+ if (!error) {
+ EXPECT_TRUE((bool)sdk_path_or_err);
+ return *sdk_path_or_err;
+ }
+ EXPECT_FALSE((bool)sdk_path_or_err);
+ llvm::consumeError(sdk_path_or_err.takeError());
+ return {};
+ };
+ EXPECT_FALSE(get_sdk("MacOSX.sdk").empty());
// These are expected to fall back to an available version.
- EXPECT_FALSE(HostInfo::GetXcodeSDKPath(XcodeSDK("MacOSX9999.sdk")).empty());
+ EXPECT_FALSE(get_sdk("MacOSX9999.sdk").empty());
// This is expected to fail.
- EXPECT_TRUE(HostInfo::GetXcodeSDKPath(XcodeSDK("CeciNestPasUnOS.sdk")).empty());
+ EXPECT_TRUE(get_sdk("CeciNestPasUnOS.sdk", true).empty());
}
#endif
Index: lldb/unittests/Host/CMakeLists.txt
===================================================================
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -32,6 +32,7 @@
${FILES}
LINK_LIBS
lldbHost
+ lldbCore
lldbUtilityHelpers
lldbHostHelpers
LLVMTestingSupport
Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -17,6 +17,7 @@
#include "PlatformRemoteAppleWatch.h"
#endif
#include "lldb/Breakpoint/BreakpointLocation.h"
+#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleList.h"
#include "lldb/Core/ModuleSpec.h"
@@ -123,8 +124,14 @@
}
// Use the default SDK as a fallback.
- FileSpec fspec(
- HostInfo::GetXcodeSDKPath(lldb_private::XcodeSDK::GetAnyMacOS()));
+ auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS());
+ if (!sdk_path_or_err) {
+ Debugger::ReportError("Error while searching for Xcode SDK: " +
+ toString(sdk_path_or_err.takeError()));
+ return {};
+ }
+
+ FileSpec fspec(*sdk_path_or_err);
if (fspec) {
if (FileSystem::Instance().Exists(fspec))
return ConstString(fspec.GetPath());
Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -12,6 +12,7 @@
#include <dlfcn.h>
#endif
+#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Host/HostInfo.h"
@@ -278,9 +279,19 @@
static llvm::StringRef GetXcodeSDKDir(std::string preferred,
std::string secondary) {
llvm::StringRef sdk;
- sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(preferred)));
+ auto get_sdk = [&](std::string sdk) -> llvm::StringRef {
+ auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(sdk)));
+ if (!sdk_path_or_err) {
+ Debugger::ReportError("Error while searching for Xcode SDK: " +
+ toString(sdk_path_or_err.takeError()));
+ return {};
+ }
+ return *sdk_path_or_err;
+ };
+
+ sdk = get_sdk(preferred);
if (sdk.empty())
- sdk = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(secondary)));
+ sdk = get_sdk(secondary);
return sdk;
}
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===================================================================
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -6,15 +6,15 @@
//
//===----------------------------------------------------------------------===//
+#include "lldb/Host/macosx/HostInfoMacOSX.h"
+#include "Utility/UuidCompatibility.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
-#include "lldb/Host/macosx/HostInfoMacOSX.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Timer.h"
-#include "Utility/UuidCompatibility.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
@@ -337,7 +337,14 @@
}
}
- FileSpec fspec(HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS()));
+ auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS());
+ if (!sdk_path_or_err) {
+ Log *log = GetLog(LLDBLog::Host);
+ LLDB_LOGF(log, "Error while searching for Xcode SDK: %s",
+ toString(sdk_path_or_err.takeError()).c_str());
+ return;
+ }
+ FileSpec fspec(*sdk_path_or_err);
if (fspec) {
if (FileSystem::Instance().Exists(fspec)) {
std::string xcode_contents_dir =
@@ -365,12 +372,18 @@
return g_developer_directory;
}
-static std::string GetXcodeSDK(XcodeSDK sdk) {
+llvm::Expected<std::string> GetXcodeSDK(XcodeSDK sdk) {
XcodeSDK::Info info = sdk.Parse();
std::string sdk_name = XcodeSDK::GetCanonicalName(info);
+ if (sdk_name.empty())
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Unrecognized SDK type: " + sdk.GetString());
+
+ Log *log = GetLog(LLDBLog::Host);
auto xcrun = [](const std::string &sdk,
- llvm::StringRef developer_dir = "") -> std::string {
+ llvm::StringRef developer_dir =
+ "") -> llvm::Expected<std::string> {
Args args;
if (!developer_dir.empty()) {
args.AppendArgument("/usr/bin/env");
@@ -391,13 +404,29 @@
int status = 0;
int signo = 0;
std::string output_str;
- lldb_private::Status error =
- Host::RunShellCommand(args, FileSpec(), &status, &signo, &output_str,
- std::chrono::seconds(15));
-
- // Check that xcrun return something useful.
- if (status != 0 || output_str.empty())
- return {};
+ // The first time after Xcode was updated or freshly installed,
+ // xcrun can take surprisingly long to build up its database.
+ auto timeout = std::chrono::seconds(60);
+ bool run_in_shell = false;
+ lldb_private::Status error = Host::RunShellCommand(
+ args, FileSpec(), &status, &signo, &output_str, timeout, run_in_shell);
+
+ // Check that xcrun returned something useful.
+ if (error.Fail()) {
+ // Catastrophic error.
+ LLDB_LOG(log, "xcrun failed to execute: %s", error.AsCString());
+ return error.ToError();
+ }
+ if (status != 0) {
+ // xcrun didn't find a matching SDK. Not an error, we'll try
+ // different spellings.
+ LLDB_LOG(log, "xcrun returned exit code %d", status);
+ return "";
+ }
+ if (output_str.empty()) {
+ LLDB_LOG(log, "xcrun returned no results");
+ return "";
+ }
// Convert to a StringRef so we can manipulate the string without modifying
// the underlying data.
@@ -414,7 +443,8 @@
return output.str();
};
- auto find_sdk = [&xcrun](const std::string &sdk_name) -> std::string {
+ auto find_sdk =
+ [&xcrun](const std::string &sdk_name) -> llvm::Expected<std::string> {
// Invoke xcrun with the developer dir specified in the environment.
std::string developer_dir = GetEnvDeveloperDir();
if (!developer_dir.empty()) {
@@ -430,8 +460,10 @@
llvm::StringRef shlib_developer_dir =
llvm::sys::path::parent_path(contents_dir);
if (!shlib_developer_dir.empty()) {
- std::string sdk = xcrun(sdk_name, std::move(shlib_developer_dir));
- if (!sdk.empty())
+ auto sdk = xcrun(sdk_name, std::move(shlib_developer_dir));
+ if (!sdk)
+ return sdk.takeError();
+ if (!sdk->empty())
return sdk;
}
}
@@ -441,7 +473,10 @@
return xcrun(sdk_name);
};
- std::string path = find_sdk(sdk_name);
+ auto path_or_err = find_sdk(sdk_name);
+ if (!path_or_err)
+ return path_or_err.takeError();
+ std::string path = *path_or_err;
while (path.empty()) {
// Try an alternate spelling of the name ("macosx10.9internal").
if (info.type == XcodeSDK::Type::MacOSX && !info.version.empty() &&
@@ -449,44 +484,68 @@
llvm::StringRef fixed(sdk_name);
if (fixed.consume_back(".internal"))
sdk_name = fixed.str() + "internal";
- path = find_sdk(sdk_name);
+ path_or_err = find_sdk(sdk_name);
+ if (!path_or_err)
+ return path_or_err.takeError();
+ path = *path_or_err;
if (!path.empty())
break;
}
- Log *log = GetLog(LLDBLog::Host);
LLDB_LOGF(log, "Couldn't find SDK %s on host", sdk_name.c_str());
// Try without the version.
if (!info.version.empty()) {
info.version = {};
sdk_name = XcodeSDK::GetCanonicalName(info);
- path = find_sdk(sdk_name);
+ path_or_err = find_sdk(sdk_name);
+ if (!path_or_err)
+ return path_or_err.takeError();
+ path = *path_or_err;
if (!path.empty())
break;
}
LLDB_LOGF(log, "Couldn't find any matching SDK on host");
- return {};
+ return "";
}
// Whatever is left in output should be a valid path.
- if (!FileSystem::Instance().Exists(path))
- return {};
+ if (!FileSystem::Instance().Exists(path)) {
+ LLDB_LOGF(log, "SDK returned by xcrun doesn't exist");
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "SDK returned by xcrun doesn't exist");
+ }
return path;
}
-llvm::StringRef HostInfoMacOSX::GetXcodeSDKPath(XcodeSDK sdk) {
- static llvm::StringMap<std::string> g_sdk_path;
+llvm::Expected<llvm::StringRef> HostInfoMacOSX::GetXcodeSDKPath(XcodeSDK sdk) {
+ struct ErrorOrPath {
+ std::string str;
+ bool is_error;
+ };
+ static llvm::StringMap<ErrorOrPath> g_sdk_path;
static std::mutex g_sdk_path_mutex;
std::lock_guard<std::mutex> guard(g_sdk_path_mutex);
LLDB_SCOPED_TIMER();
- auto it = g_sdk_path.find(sdk.GetString());
- if (it != g_sdk_path.end())
- return it->second;
- auto it_new = g_sdk_path.insert({sdk.GetString(), GetXcodeSDK(sdk)});
- return it_new.first->second;
+ auto key = sdk.GetString();
+ auto it = g_sdk_path.find(key);
+ if (it != g_sdk_path.end()) {
+ if (it->second.is_error)
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ it->second.str);
+ else
+ return it->second.str;
+ }
+ auto path_or_err = GetXcodeSDK(sdk);
+ if (!path_or_err) {
+ std::string error = toString(path_or_err.takeError());
+ g_sdk_path.insert({key, {error, true}});
+ return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
+ }
+ auto it_new = g_sdk_path.insert({key, {*path_or_err, false}});
+ return it_new.first->second.str;
}
namespace {
Index: lldb/source/Core/Module.cpp
===================================================================
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1647,7 +1647,15 @@
void Module::RegisterXcodeSDK(llvm::StringRef sdk_name,
llvm::StringRef sysroot) {
XcodeSDK sdk(sdk_name.str());
- llvm::StringRef sdk_path(HostInfo::GetXcodeSDKPath(sdk));
+ auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(sdk);
+
+ if (!sdk_path_or_err) {
+ Debugger::ReportError("Error while searching for Xcode SDK: " +
+ toString(sdk_path_or_err.takeError()));
+ return;
+ }
+
+ auto sdk_path = *sdk_path_or_err;
if (sdk_path.empty())
return;
// If the SDK changed for a previously registered source path, update it.
Index: lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
===================================================================
--- lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
+++ lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
@@ -30,7 +30,7 @@
static FileSpec GetXcodeDeveloperDirectory();
/// Query xcrun to find an Xcode SDK directory.
- static llvm::StringRef GetXcodeSDKPath(XcodeSDK sdk);
+ static llvm::Expected<llvm::StringRef> GetXcodeSDKPath(XcodeSDK sdk);
/// Shared cache utilities
static SharedCacheImageInfo
Index: lldb/include/lldb/Host/HostInfoBase.h
===================================================================
--- lldb/include/lldb/Host/HostInfoBase.h
+++ lldb/include/lldb/Host/HostInfoBase.h
@@ -108,7 +108,9 @@
static FileSpec GetXcodeDeveloperDirectory() { return {}; }
/// Return the directory containing a specific Xcode SDK.
- static llvm::StringRef GetXcodeSDKPath(XcodeSDK sdk) { return {}; }
+ static llvm::Expected<llvm::StringRef> GetXcodeSDKPath(XcodeSDK sdk) {
+ return "";
+ }
/// Return information about module \p image_name if it is loaded in
/// the current process's address space.
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits