aprantl updated this revision to Diff 475630.
aprantl edited the summary of this revision.
aprantl added a comment.
Implemented error handling of sorts. Because the computation result is cached
and most uses are nested deep in places that have no proper error handling
themselves I'm logging to the global error stream.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138060/new/
https://reviews.llvm.org/D138060
Files:
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/unittests/Host/CMakeLists.txt
lldb/unittests/Host/HostTest.cpp
Index: lldb/unittests/Host/HostTest.cpp
===================================================================
--- lldb/unittests/Host/HostTest.cpp
+++ lldb/unittests/Host/HostTest.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "lldb/Host/Host.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfoBase.h"
#include "gtest/gtest.h"
using namespace lldb_private;
@@ -25,3 +27,25 @@
ASSERT_EQ("Host::GetEnvironment",
Host::GetEnvironment().lookup("LLDB_TEST_ENVIRONMENT_VAR"));
}
+
+#if defined(LLVM_ON_UNIX)
+TEST(Host, RunShellCommand) {
+ HostInfoBase::Initialize();
+ FileSystem::Initialize();
+ std::string shell = std::string("SHELL=") + getenv("SHELL");
+ putenv(const_cast<char *>("SHELL=/bin/LLDB_TEST_this-file-does-not-exist"));
+ int status, signo;
+ std::string out;
+ auto timeout = std::chrono::seconds(60);
+ Status error = Host::RunShellCommand("/usr/bin/true", FileSpec(), &status,
+ &signo, &out, timeout);
+ ASSERT_TRUE(error.Fail());
+ putenv(const_cast<char *>(shell.c_str()));
+
+ error = Host::RunShellCommand("/usr/bin/false", FileSpec(), &status, &signo,
+ &out, timeout);
+ ASSERT_FALSE(error.Fail());
+ HostInfoBase::Terminate();
+ FileSystem::Terminate();
+}
+#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/Host/macosx/objcxx/HostInfoMacOSX.mm
===================================================================
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -10,6 +10,7 @@
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Host/macosx/HostInfoMacOSX.h"
+#include "lldb/Core/Debugger.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
@@ -365,12 +366,14 @@
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);
+ 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 +394,25 @@
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);
+ lldb_private::Status error = Host::RunShellCommand(
+ args, FileSpec(), &status, &signo, &output_str, timeout);
+
+ // Check that xcrun returned something useful.
+ if (error.Fail()) {
+ LLDB_LOG(log, "xcrun failed to execute: %s", error.AsCString());
+ return error.ToError();
+ }
+ if (status != 0) {
+ 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 +429,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 +446,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 +459,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,29 +470,36 @@
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 "";
+ }
return path;
}
@@ -485,7 +513,20 @@
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)});
+ auto path_or_err = GetXcodeSDK(sdk);
+ std::string path;
+ if (path_or_err)
+ path = *path_or_err;
+ else {
+ std::string msg;
+ {
+ llvm::raw_string_ostream os(msg);
+ llvm::logAllUnhandledErrors(path_or_err.takeError(), os,
+ "Error trying to get Xcode SDK path: ");
+ }
+ Debugger::ReportError(msg);
+ }
+ auto it_new = g_sdk_path.insert({sdk.GetString(), path});
return it_new.first->second;
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
