fixup! if path at DW_AT_LLVM_sysroot exists, use it

Author: Michael137

lldb/include/lldb/Host/FileSystem.h

@@ -183,9 +183,8 @@ class FileSystem {
     eEnumerateDirectoryResultQuit
   };
 
-  typedef std::function<EnumerateDirectoryResult(
-      void *baton, llvm::sys::fs::file_type file_type, llvm::StringRef)>
-      EnumerateDirectoryCallbackType;
+  typedef EnumerateDirectoryResult (*EnumerateDirectoryCallbackType)(
+      void *baton, llvm::sys::fs::file_type file_type, llvm::StringRef);
 
   typedef std::function<EnumerateDirectoryResult(
       llvm::sys::fs::file_type file_type, llvm::StringRef)>

lldb/include/lldb/Symbol/SymbolFile.h

@@ -32,6 +32,7 @@
 #include <mutex>
 #include <optional>
 #include <unordered_map>
+#include <utility>
 
 #if defined(LLDB_CONFIGURATION_DEBUG)
 #define ASSERT_MODULE_LOCK(expr) (expr->AssertModuleLock())
@@ -148,7 +149,10 @@ class SymbolFile : public PluginInterface {
 
   virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
   /// Return the Xcode SDK comp_unit was compiled against.
-  virtual XcodeSDK ParseXcodeSDK(CompileUnit &comp_unit) { return {}; }
+  virtual std::pair<XcodeSDK, std::string>
+  ParseXcodeSDK(CompileUnit &comp_unit) {
+    return {};
+  }
 
   /// This function exists because SymbolFileDWARFDebugMap may extra compile
   /// units which aren't exposed as "real" compile units. In every other

lldb/include/lldb/Symbol/SymbolFileOnDemand.h

@@ -65,7 +65,7 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
   lldb::LanguageType
   ParseLanguage(lldb_private::CompileUnit &comp_unit) override;
 
-  lldb_private::XcodeSDK
+  std::pair<lldb_private::XcodeSDK, std::string>
   ParseXcodeSDK(lldb_private::CompileUnit &comp_unit) override;
 
   void InitializeObject() override;

lldb/include/lldb/Target/Platform.h

@@ -438,25 +438,6 @@ class Platform : public PluginInterface {
     return lldb_private::ConstString();
   }
 
-  /// Search each CU associated with the specified 'module' for
-  /// the SDK paths the CUs were compiled against. In the presence
-  /// of different SDKs, we try to pick the most appropriate one
-  /// using \ref XcodeSDK::Merge.
-  ///
-  /// \param[in] module Module whose debug-info CUs to parse for
-  ///                   which SDK they were compiled against.
-  ///
-  /// \returns If successful, returns a pair of a parsed XcodeSDK
-  ///          object and a boolean that is 'true' if we encountered
-  ///          a conflicting combination of SDKs when parsing the CUs
-  ///          (e.g., a public and internal SDK).
-  virtual llvm::Expected<std::pair<XcodeSDK, bool>>
-  GetSDKPathFromDebugInfo(Module &module) {
-    return llvm::createStringError(
-        llvm::formatv("{0} not implemented for '{1}' platform.",
-                      LLVM_PRETTY_FUNCTION, GetName()));
-  }
-
   /// Returns the full path of the most appropriate SDK for the
   /// specified 'module'. This function gets this path by parsing
   /// debug-info (see \ref `GetSDKPathFromDebugInfo`).
@@ -477,8 +458,9 @@ class Platform : public PluginInterface {
   ///
   /// \param[in] unit The CU
   ///
-  /// \returns A parsed XcodeSDK object if successful, an Error otherwise. 
-  virtual llvm::Expected<XcodeSDK> GetSDKPathFromDebugInfo(CompileUnit &unit) {
+  /// \returns A parsed XcodeSDK object if successful, an Error otherwise.
+  virtual llvm::Expected<std::pair<XcodeSDK, std::string>>
+  GetSDKPathFromDebugInfo(CompileUnit &unit) {
     return llvm::createStringError(
         llvm::formatv("{0} not implemented for '{1}' platform.",
                       LLVM_PRETTY_FUNCTION, GetName()));

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

@@ -15,14 +15,11 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 
-#include "clang/Basic/DarwinSDKInfo.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/raw_ostream.h"
 
 // C++ Includes
@@ -572,52 +569,10 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec &path) {
     cache.insert({key, {error, true}});
     return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
   }
-
-  if (path_or_err->empty())
-    return llvm::createStringError("Empty path determined for '%s'",
-                                   key.data());
-
   auto it_new = cache.insert({key, {*path_or_err, false}});
   return it_new.first->second.str;
 }
 
-static llvm::Expected<std::string>
-GetCommandLineToolsSDKRoot(llvm::VersionTuple version) {
-  std::string clt_root_dir;
-  FileSystem::Instance().EnumerateDirectory(
-      "/Library/Developer/CommandLineTools/SDKs/", /*find_directories=*/true,
-      /*find_files=*/false, /*find_other=*/false,
-      [&](void *baton, llvm::sys::fs::file_type file_type,
-          llvm::StringRef name) {
-        assert(file_type == llvm::sys::fs::file_type::directory_file);
-
-        if (!name.ends_with(".sdk"))
-          return FileSystem::eEnumerateDirectoryResultNext;
-
-        llvm::Expected<std::optional<clang::DarwinSDKInfo>> sdk_info =
-            clang::parseDarwinSDKInfo(
-                *FileSystem::Instance().GetVirtualFileSystem(), name);
-        if (!sdk_info) {
-          LLDB_LOG_ERROR(GetLog(LLDBLog::Expressions), sdk_info.takeError(),
-                         "Error while parsing {1}: {0}", name);
-          return FileSystem::eEnumerateDirectoryResultNext;
-        }
-
-        if (!*sdk_info)
-          return FileSystem::eEnumerateDirectoryResultNext;
-
-        if (version == (*sdk_info)->getVersion()) {
-          clt_root_dir = name;
-          return FileSystem::eEnumerateDirectoryResultQuit;
-        }
-
-        return FileSystem::eEnumerateDirectoryResultNext;
-      },
-      /*baton=*/nullptr);
-
-  return clt_root_dir;
-}
-
 llvm::Expected<llvm::StringRef> HostInfoMacOSX::GetSDKRoot(SDKOptions options) {
   static llvm::StringMap<ErrorOrPath> g_sdk_path;
   static std::mutex g_sdk_path_mutex;
@@ -626,21 +581,6 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec &path) {
                                    "XcodeSDK not specified");
   XcodeSDK sdk = *options.XcodeSDKSelection;
   auto key = sdk.GetString();
-
-  // xcrun doesn't search SDKs in the CommandLineTools (CLT) directory. So if
-  // a program was compiled against a CLT SDK, but that SDK wasn't present in
-  // any of the Xcode installations, then xcrun would fail to find the SDK
-  // (which is expensive). To avoid this we first try to find the specified SDK
-  // in the CLT directory.
-  auto clt_root_dir = find_cached_path(g_sdk_path, g_sdk_path_mutex, key, [&] {
-    return GetCommandLineToolsSDKRoot(sdk.GetVersion());
-  });
-
-  if (clt_root_dir)
-    return clt_root_dir;
-  else
-    llvm::consumeError(clt_root_dir.takeError());
-
   return find_cached_path(g_sdk_path, g_sdk_path_mutex, key, [&](){
     return GetXcodeSDK(sdk);
   });

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

@@ -336,13 +336,7 @@ sdkSupportsBuiltinModules(lldb_private::Target &target) {
   if (!platform_sp)
     return llvm::createStringError("No Platform plugin found on target.");
 
-  auto sdk_or_err = platform_sp->GetSDKPathFromDebugInfo(*module_sp);
-  if (!sdk_or_err)
-    return sdk_or_err.takeError();
-
-  // Use the SDK path from debug-info to find a local matching SDK directory.
-  auto sdk_path_or_err =
-      HostInfo::GetSDKRoot(HostInfo::SDKOptions{std::move(sdk_or_err->first)});
+  auto sdk_path_or_err = platform_sp->ResolveSDKPathFromDebugInfo(*module_sp);
   if (!sdk_path_or_err)
     return sdk_path_or_err.takeError();
 

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

@@ -1386,57 +1386,18 @@ llvm::Triple::OSType PlatformDarwin::GetHostOSType() {
 #endif // __APPLE__
 }
 
-llvm::Expected<std::pair<XcodeSDK, bool>>
-PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) {
-  SymbolFile *sym_file = module.GetSymbolFile();
-  if (!sym_file)
-    return llvm::createStringError(
-        llvm::inconvertibleErrorCode(),
-        llvm::formatv("No symbol file available for module '{0}'",
-                      module.GetFileSpec().GetFilename().AsCString("")));
-
-  bool found_public_sdk = false;
-  bool found_internal_sdk = false;
-  XcodeSDK merged_sdk;
-  for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) {
-    if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) {
-      auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp);
-      bool is_internal_sdk = cu_sdk.IsAppleInternalSDK();
-      found_public_sdk |= !is_internal_sdk;
-      found_internal_sdk |= is_internal_sdk;
-
-      merged_sdk.Merge(cu_sdk);
-    }
-  }
-
-  const bool found_mismatch = found_internal_sdk && found_public_sdk;
-
-  return std::pair{std::move(merged_sdk), found_mismatch};
-}
-
 llvm::Expected<std::string>
 PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) {
-  auto sdk_or_err = GetSDKPathFromDebugInfo(module);
-  if (!sdk_or_err)
+  auto cu_sp = module.GetCompileUnitAtIndex(0);
+  if (!cu_sp)
     return llvm::createStringError(
-        llvm::inconvertibleErrorCode(),
-        llvm::formatv("Failed to parse SDK path from debug-info: {0}",
-                      llvm::toString(sdk_or_err.takeError())));
-
-  auto [sdk, _] = std::move(*sdk_or_err);
-
-  auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
-  if (!path_or_err)
-    return llvm::createStringError(
-        llvm::inconvertibleErrorCode(),
-        llvm::formatv("Error while searching for SDK (XcodeSDK '{0}'): {1}",
-                      sdk.GetString(),
-                      llvm::toString(path_or_err.takeError())));
+        "Couldn't retrieve compile-unit for module '%s'.",
+        module.GetObjectName().AsCString("<null>"));
 
-  return path_or_err->str();
+  return ResolveSDKPathFromDebugInfo(*cu_sp);
 }
 
-llvm::Expected<XcodeSDK>
+llvm::Expected<std::pair<XcodeSDK, std::string>>
 PlatformDarwin::GetSDKPathFromDebugInfo(CompileUnit &unit) {
   ModuleSP module_sp = unit.CalculateSymbolContextModule();
   if (!module_sp)
@@ -1459,7 +1420,10 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(CompileUnit &unit) {
         llvm::formatv("Failed to parse SDK path from debug-info: {0}",
                       llvm::toString(sdk_or_err.takeError())));
 
-  auto sdk = std::move(*sdk_or_err);
+  auto [sdk, sysroot] = std::move(*sdk_or_err);
+
+  if (FileSystem::Instance().Exists(sysroot))
+    return sysroot;
 
   auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
   if (!path_or_err)

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h

@@ -124,13 +124,11 @@ class PlatformDarwin : public PlatformPOSIX {
   /// located in.
   static FileSpec GetCurrentCommandLineToolsDirectory();
 
-  llvm::Expected<std::pair<XcodeSDK, bool>>
-  GetSDKPathFromDebugInfo(Module &module) override;
-
   llvm::Expected<std::string>
   ResolveSDKPathFromDebugInfo(Module &module) override;
 
-  llvm::Expected<XcodeSDK> GetSDKPathFromDebugInfo(CompileUnit &unit) override;
+  llvm::Expected<std::pair<XcodeSDK, std::string>>
+  GetSDKPathFromDebugInfo(CompileUnit &unit) override;
 
   llvm::Expected<std::string>
   ResolveSDKPathFromDebugInfo(CompileUnit &unit) override;

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

@@ -982,7 +982,8 @@ lldb::LanguageType SymbolFileDWARF::ParseLanguage(CompileUnit &comp_unit) {
     return eLanguageTypeUnknown;
 }
 
-XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) {
+std::pair<XcodeSDK, std::string>
+SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
   if (!dwarf_cu)
@@ -993,21 +994,26 @@ XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) {
   const char *sdk = cu_die.GetAttributeValueAsString(DW_AT_APPLE_sdk, nullptr);
   if (!sdk)
     return {};
-  const char *sysroot =
+  std::string sysroot =
       cu_die.GetAttributeValueAsString(DW_AT_LLVM_sysroot, "");
-  // Register the sysroot path remapping with the module belonging to
-  // the CU as well as the one belonging to the symbol file. The two
-  // would be different if this is an OSO object and module is the
-  // corresponding debug map, in which case both should be updated.
-  ModuleSP module_sp = comp_unit.GetModule();
-  if (module_sp)
-    module_sp->RegisterXcodeSDK(sdk, sysroot);
 
-  ModuleSP local_module_sp = m_objfile_sp->GetModule();
-  if (local_module_sp && local_module_sp != module_sp)
-    local_module_sp->RegisterXcodeSDK(sdk, sysroot);
+  // RegisterXcodeSDK calls into xcrun which is not aware of CLT, which is
+  // expensive.
+  if (sysroot.find("/Library/Developer/CommandLineTools/SDKs") != 0) {
+    // Register the sysroot path remapping with the module belonging to
+    // the CU as well as the one belonging to the symbol file. The two
+    // would be different if this is an OSO object and module is the
+    // corresponding debug map, in which case both should be updated.
+    ModuleSP module_sp = comp_unit.GetModule();
+    if (module_sp)
+      module_sp->RegisterXcodeSDK(sdk, sysroot);
+
+    ModuleSP local_module_sp = m_objfile_sp->GetModule();
+    if (local_module_sp && local_module_sp != module_sp)
+      local_module_sp->RegisterXcodeSDK(sdk, sysroot);
+  }
 
-  return {sdk};
+  return std::pair{XcodeSDK{sdk}, std::move(sysroot)};
 }
 
 size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) {

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

@@ -110,7 +110,8 @@ class SymbolFileDWARF : public SymbolFileCommon {
 
   lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) override;
 
-  XcodeSDK ParseXcodeSDK(CompileUnit &comp_unit) override;
+  std::pair<XcodeSDK, std::string>
+  ParseXcodeSDK(CompileUnit &comp_unit) override;
 
   size_t ParseFunctions(CompileUnit &comp_unit) override;
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

@@ -673,7 +673,8 @@ SymbolFileDWARFDebugMap::ParseLanguage(CompileUnit &comp_unit) {
   return eLanguageTypeUnknown;
 }
 
-XcodeSDK SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) {
+std::pair<XcodeSDK, std::string>
+SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   SymbolFileDWARF *oso_dwarf = GetSymbolFile(comp_unit);
   if (oso_dwarf)

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

@@ -64,7 +64,8 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
 
   // Compile Unit function calls
   lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) override;
-  XcodeSDK ParseXcodeSDK(CompileUnit &comp_unit) override;
+  std::pair<XcodeSDK, std::string>
+  ParseXcodeSDK(CompileUnit &comp_unit) override;
   llvm::SmallSet<lldb::LanguageType, 4>
   ParseAllLanguages(CompileUnit &comp_unit) override;
   size_t ParseFunctions(CompileUnit &comp_unit) override;

lldb/source/Symbol/SymbolFileOnDemand.cpp

@@ -58,17 +58,18 @@ lldb::LanguageType SymbolFileOnDemand::ParseLanguage(CompileUnit &comp_unit) {
   return m_sym_file_impl->ParseLanguage(comp_unit);
 }
 
-XcodeSDK SymbolFileOnDemand::ParseXcodeSDK(CompileUnit &comp_unit) {
+std::pair<XcodeSDK, std::string>
+SymbolFileOnDemand::ParseXcodeSDK(CompileUnit &comp_unit) {
   if (!m_debug_info_enabled) {
     Log *log = GetLog();
     LLDB_LOG(log, "[{0}] {1} is skipped", GetSymbolFileName(), __FUNCTION__);
     XcodeSDK defaultValue{};
     if (log) {
-      XcodeSDK sdk = m_sym_file_impl->ParseXcodeSDK(comp_unit);
+      auto [sdk, sysroot] = m_sym_file_impl->ParseXcodeSDK(comp_unit);
       if (!(sdk == defaultValue))
         LLDB_LOG(log, "SDK {0} would return if hydrated.", sdk.GetString());
     }
-    return defaultValue;
+    return {defaultValue, {}};
   }
   return m_sym_file_impl->ParseXcodeSDK(comp_unit);
 }

lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

@@ -118,7 +118,7 @@ TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   ASSERT_TRUE(static_cast<bool>(comp_unit.get()));
   ModuleSP module = t.GetModule();
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 0u);
-  XcodeSDK sdk = sym_file.ParseXcodeSDK(*comp_unit);
+  auto [sdk, sysroot] = sym_file.ParseXcodeSDK(*comp_unit);
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }