Cherry pick 1013

lldb/include/lldb/Host/HostInfoBase.h

@@ -116,7 +116,9 @@ class HostInfoBase {
 #endif
 
   /// 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/include/lldb/Host/macosx/HostInfoMacOSX.h

@@ -52,7 +52,7 @@ class HostInfoMacOSX : public HostInfoPosix {
 #endif
 
   /// 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

lldb/source/Core/Module.cpp

@@ -1713,7 +1713,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.

lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

@@ -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
+      // 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 @@ 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,52 +473,79 @@ 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() &&
         info.internal) {
       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 {

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;
 }
 

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());

lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp

@@ -1354,6 +1354,22 @@ static llvm::Optional<StringRef> GetDSYMBundle(Module &module) {
   return dsym;
 }
 
+static std::string GetSDKPath(std::string m_description, XcodeSDK 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()));
+    HEALTH_LOG_PRINTF("Error while searching for Xcode SDK %s.",
+                      sdk.GetString().str().c_str());
+    return {};
+  }
+
+  std::string sdk_path = sdk_path_or_err->str();
+  LOG_PRINTF(GetLog(LLDBLog::Types), "Host SDK path for sdk %s is %s.",
+             sdk.GetString().str().c_str(), sdk_path.c_str());
+  return sdk_path;
+}
+
 /// Force parsing of the CUs to extract the SDK info.
 static std::string GetSDKPathFromDebugInfo(std::string m_description,
                                            Module &module) {
@@ -1376,11 +1392,7 @@ static std::string GetSDKPathFromDebugInfo(std::string m_description,
                       "'%s'. Mixed use of SDKs indicates use of different "
                       "toolchains, which is not supported.",
                       module.GetFileSpec().GetFilename().GetCString());
-
-  std::string sdk_path = HostInfo::GetXcodeSDKPath(sdk).str();
-  LOG_PRINTF(GetLog(LLDBLog::Types), "Host SDK path for sdk %s is %s.",
-             sdk.GetString().str().c_str(), sdk_path.c_str());
-  return sdk_path;
+  return GetSDKPath(m_description, sdk);
 }
 
 /// Detect whether a Swift module was "imported" by DWARFImporter.
@@ -2464,15 +2476,15 @@ void SwiftASTContext::InitializeSearchPathOptions(
       XcodeSDK::Info info;
       info.type = XcodeSDK::GetSDKTypeForTriple(triple);
       XcodeSDK sdk(info);
-      sdk_path = HostInfo::GetXcodeSDKPath(sdk).str();
+      sdk_path = GetSDKPath(m_description, sdk);
     }
     if (sdk_path.empty()) {
       // This fallback is questionable. Perhaps it should be removed.
       XcodeSDK::Info info;
       info.type = XcodeSDK::GetSDKTypeForTriple(
           HostInfo::GetArchitecture().GetTriple());
       XcodeSDK sdk(info);
-      sdk_path = std::string(HostInfo::GetXcodeSDKPath(sdk));
+      sdk_path = GetSDKPath(m_description, sdk);
     }
     if (!sdk_path.empty()) {
       // Note that calling setSDKPath() also recomputes all paths that

lldb/unittests/Host/CMakeLists.txt

@@ -39,6 +39,7 @@ add_lldb_unittest(HostTests
   ${FILES}
   LINK_LIBS
     lldbHost
+    lldbCore
     lldbUtilityHelpers
     lldbHostHelpers
     LLVMTestingSupport

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