[Lldb-commits] [lldb] 33cbda4 - Improve error logging when xcrun fails to execute successfully
Adrian Prantl via lldb-commits
lldb-commits at lists.llvm.org
Thu Dec 1 16:22:24 PST 2022
Author: Adrian Prantl
Date: 2022-12-01T16:22:14-08:00
New Revision: 33cbda4cacb87103bbb5f4f9f2368bbb442132f4
URL: https://github.com/llvm/llvm-project/commit/33cbda4cacb87103bbb5f4f9f2368bbb442132f4
DIFF: https://github.com/llvm/llvm-project/commit/33cbda4cacb87103bbb5f4f9f2368bbb442132f4.diff
LOG: Improve error logging when xcrun fails to execute successfully
Because Host::RunShellCommand runs commands through $SHELL there is an
opportunity for this to fail spectacularly on systems that use custom
shells with odd behaviors. This patch makes these situations easier to
debug by at least logging the result of the failed xcrun invocation.
It also doesn't run xcrun through a shell any more.
rdar://102389438
Differential Revision: https://reviews.llvm.org/D138060
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/HostInfoBase.h b/lldb/include/lldb/Host/HostInfoBase.h
index eeed881101d03..38c488cc73b83 100644
--- a/lldb/include/lldb/Host/HostInfoBase.h
+++ b/lldb/include/lldb/Host/HostInfoBase.h
@@ -108,7 +108,9 @@ class HostInfoBase {
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.
diff --git a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
index de7ecc1e6d803..913af1ffe70ba 100644
--- a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
+++ b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
@@ -30,7 +30,7 @@ class HostInfoMacOSX : public HostInfoPosix {
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
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index dd2569eb81f8e..63b79b3b71d88 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1645,7 +1645,15 @@ Module::RemapSourceFile(llvm::StringRef path) const {
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.
diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index d0d964b0903be..ddc66c8f5b50b 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/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 @@ static void ParseOSVersion(llvm::VersionTuple &version, NSString *Key) {
}
}
- 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 @@ static void ParseOSVersion(llvm::VersionTuple &version, NSString *Key) {
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 @@ static void ParseOSVersion(llvm::VersionTuple &version, NSString *Key) {
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
+ //
diff erent 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 @@ static void ParseOSVersion(llvm::VersionTuple &version, NSString *Key) {
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 @@ static void ParseOSVersion(llvm::VersionTuple &version, NSString *Key) {
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 @@ static void ParseOSVersion(llvm::VersionTuple &version, NSString *Key) {
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 @@ static void ParseOSVersion(llvm::VersionTuple &version, NSString *Key) {
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 {
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index d610ac1280481..1e117f26dacf8 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ b/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 @@ std::vector<ArchSpec> PlatformAppleSimulator::GetSupportedArchitectures(
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;
}
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
index 0dc669d7e8c73..25b821d12a314 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ b/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 @@ ConstString PlatformMacOSX::GetSDKDirectory(lldb_private::Target &target) {
}
// 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());
diff --git a/lldb/unittests/Host/CMakeLists.txt b/lldb/unittests/Host/CMakeLists.txt
index 23a6aae721e7e..2e9f84314c4a7 100644
--- a/lldb/unittests/Host/CMakeLists.txt
+++ b/lldb/unittests/Host/CMakeLists.txt
@@ -32,6 +32,7 @@ add_lldb_unittest(HostTests
${FILES}
LINK_LIBS
lldbHost
+ lldbCore
lldbUtilityHelpers
lldbHostHelpers
LLVMTestingSupport
diff --git a/lldb/unittests/Host/HostInfoTest.cpp b/lldb/unittests/Host/HostInfoTest.cpp
index daec8b46e5425..50b903b1dc949 100644
--- a/lldb/unittests/Host/HostInfoTest.cpp
+++ b/lldb/unittests/Host/HostInfoTest.cpp
@@ -56,11 +56,21 @@ TEST_F(HostInfoTest, GetHostname) {
#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
More information about the lldb-commits
mailing list