[lldb][NFC] remove the ResolveSDKPathFromDebugInfo method #145744

charles-zablit · 2025-06-25T17:33:49Z

This patch is part of an effort to remove the ResolveSDKPathFromDebugInfo method, and more specifically the variant which takes a Module as argument.

This PR should be merged after #144913.

llvmbot · 2025-06-25T17:34:23Z

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

Changes

This patch is part of an effort to remove the ResolveSDKPathFromDebugInfo method, and more specifically the variant which takes a Module as argument.

This PR should be merged after #144913.

Full diff: https://github.com/llvm/llvm-project/pull/145744.diff

7 Files Affected:

(modified) lldb/include/lldb/Target/Platform.h (-16)
(modified) lldb/include/lldb/Utility/XcodeSDK.h (-13)
(modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+1-50)
(modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (+25-31)
(modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h (-3)
(modified) lldb/source/Utility/XcodeSDK.cpp (-21)
(modified) lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp (-6)

diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 35ffdabf907e7..1a05bdf54332f 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -458,22 +458,6 @@ class Platform : public PluginInterface {
                       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`).
-  ///
-  /// \param[in] module Module whose debug-info to parse for
-  ///                   which SDK it was compiled against.
-  ///
-  /// \returns If successful, returns the full path to an
-  ///          Xcode SDK.
-  virtual llvm::Expected<std::string>
-  ResolveSDKPathFromDebugInfo(Module &module) {
-    return llvm::createStringError(
-        llvm::formatv("{0} not implemented for '{1}' platform.",
-                      LLVM_PRETTY_FUNCTION, GetName()));
-  }
-
   /// Search CU for the SDK path the CUs was compiled against.
   ///
   /// \param[in] unit The CU
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h b/lldb/include/lldb/Utility/XcodeSDK.h
index ceb8abb8c502d..a1a0ec415b90e 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -93,19 +93,6 @@ class XcodeSDK {
   static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
   static bool SDKSupportsModules(Type desired_type, const FileSpec &sdk_path);
 
-  /// Returns true if the SDK for the specified triple supports
-  /// builtin modules in system headers.
-  ///
-  /// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
-  /// Toolchains/Darwin.cpp
-  ///
-  /// FIXME: this function will be removed once LLDB's ClangExpressionParser
-  /// constructs the compiler instance through the driver/toolchain. See \ref
-  /// SetupImportStdModuleLangOpts
-  ///
-  static bool SDKSupportsBuiltinModules(const llvm::Triple &target_triple,
-                                        llvm::VersionTuple sdk_version);
-
   /// Return the canonical SDK name, such as "macosx" for the macOS SDK.
   static std::string GetCanonicalName(Info info);
   /// Return the best-matching SDK type for a specific triple.
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 7aa9cae5a5614..ffc76e6e93498 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -319,49 +319,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
   StringRef m_filename;
 };
 
-/// Returns true if the SDK for the specified triple supports
-/// builtin modules in system headers. This is used to decide
-/// whether to pass -fbuiltin-headers-in-system-modules to
-/// the compiler instance when compiling the `std` module.
-static llvm::Expected<bool>
-sdkSupportsBuiltinModules(lldb_private::Target &target) {
-  auto arch_spec = target.GetArchitecture();
-  auto const &triple = arch_spec.GetTriple();
-  auto module_sp = target.GetExecutableModule();
-  if (!module_sp)
-    return llvm::createStringError("Executable module not found.");
-
-  // Get SDK path that the target was compiled against.
-  auto platform_sp = target.GetPlatform();
-  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)});
-  if (!sdk_path_or_err)
-    return sdk_path_or_err.takeError();
-
-  auto VFS = FileSystem::Instance().GetVirtualFileSystem();
-  if (!VFS)
-    return llvm::createStringError("No virtual filesystem available.");
-
-  // Extract SDK version from the /path/to/some.sdk/SDKSettings.json
-  auto parsed_or_err = clang::parseDarwinSDKInfo(*VFS, *sdk_path_or_err);
-  if (!parsed_or_err)
-    return parsed_or_err.takeError();
-
-  auto maybe_sdk = *parsed_or_err;
-  if (!maybe_sdk)
-    return llvm::createStringError("Couldn't find Darwin SDK info.");
-
-  return XcodeSDK::SDKSupportsBuiltinModules(triple, maybe_sdk->getVersion());
-}
-
 static void SetupModuleHeaderPaths(CompilerInstance *compiler,
                                    std::vector<std::string> include_directories,
                                    lldb::TargetSP target_sp) {
@@ -705,7 +662,6 @@ static void SetupLangOpts(CompilerInstance &compiler,
 
 static void SetupImportStdModuleLangOpts(CompilerInstance &compiler,
                                          lldb_private::Target &target) {
-  Log *log = GetLog(LLDBLog::Expressions);
   LangOptions &lang_opts = compiler.getLangOpts();
   lang_opts.Modules = true;
   // We want to implicitly build modules.
@@ -723,12 +679,7 @@ static void SetupImportStdModuleLangOpts(CompilerInstance &compiler,
   lang_opts.GNUKeywords = true;
   lang_opts.CPlusPlus11 = true;
 
-  if (auto supported_or_err = sdkSupportsBuiltinModules(target))
-    lang_opts.BuiltinHeadersInSystemModules = !*supported_or_err;
-  else
-    LLDB_LOG_ERROR(log, supported_or_err.takeError(),
-                   "Failed to determine BuiltinHeadersInSystemModules when "
-                   "setting up import-std-module: {0}");
+  lang_opts.BuiltinHeadersInSystemModules = false;
 
   // The Darwin libc expects this macro to be set.
   lang_opts.GNUCVersion = 40201;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 262a7dc731713..54d70e6ffb0c2 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1130,14 +1130,33 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
 
   if (target) {
     if (ModuleSP exe_module_sp = target->GetExecutableModule()) {
-      auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp);
-      if (path_or_err) {
-        sysroot_spec = FileSpec(*path_or_err);
+      SymbolFile *sym_file = exe_module_sp->GetSymbolFile();
+      if (!sym_file)
+        return;
+
+      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);
+          merged_sdk.Merge(cu_sdk);
+        }
+      }
+
+      if (FileSystem::Instance().Exists(merged_sdk.GetSysroot())) {
+        sysroot_spec = merged_sdk.GetSysroot();
       } else {
-        LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host),
-                       path_or_err.takeError(),
-                       "Failed to resolve SDK path: {0}");
+        auto path_or_err =
+            HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk});
+        if (path_or_err) {
+          sysroot_spec = FileSpec(*path_or_err);
+        } else {
+          LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host),
+                         path_or_err.takeError(),
+                         "Failed to resolve SDK path: {0}");
+        }
       }
+      // getSDKfortriple()
+      // would not work for internal/public variants.
     }
   }
 
@@ -1384,31 +1403,6 @@ PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) {
   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)
-    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);
-
-  if (FileSystem::Instance().Exists(sdk.GetSysroot()))
-    return sdk.GetSysroot().GetPath();
-
-  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())));
-
-  return path_or_err->str();
-}
-
 llvm::Expected<XcodeSDK>
 PlatformDarwin::GetSDKPathFromDebugInfo(CompileUnit &unit) {
   ModuleSP module_sp = unit.CalculateSymbolContextModule();
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index f8a62ceb958fe..f8a9301002e0d 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -120,9 +120,6 @@ class PlatformDarwin : public PlatformPOSIX {
   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::string>
diff --git a/lldb/source/Utility/XcodeSDK.cpp b/lldb/source/Utility/XcodeSDK.cpp
index 004b4717e315b..eb2047e67c326 100644
--- a/lldb/source/Utility/XcodeSDK.cpp
+++ b/lldb/source/Utility/XcodeSDK.cpp
@@ -266,27 +266,6 @@ bool XcodeSDK::SupportsSwift() const {
   }
 }
 
-bool XcodeSDK::SDKSupportsBuiltinModules(const llvm::Triple &target_triple,
-                                         llvm::VersionTuple sdk_version) {
-  using namespace llvm;
-
-  switch (target_triple.getOS()) {
-  case Triple::OSType::MacOSX:
-    return sdk_version >= VersionTuple(15U);
-  case Triple::OSType::IOS:
-    return sdk_version >= VersionTuple(18U);
-  case Triple::OSType::TvOS:
-    return sdk_version >= VersionTuple(18U);
-  case Triple::OSType::WatchOS:
-    return sdk_version >= VersionTuple(11U);
-  case Triple::OSType::XROS:
-    return sdk_version >= VersionTuple(2U);
-  default:
-    // New SDKs support builtin modules from the start.
-    return true;
-  }
-}
-
 bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type desired_type,
                                   const FileSpec &sdk_path) {
   ConstString last_path_component = sdk_path.GetFilename();
diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
index 50c37dcd4568e..e04183d85e5bd 100644
--- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -161,9 +161,6 @@ TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
 
   auto platform_sp = Platform::GetHostPlatform();
   ASSERT_TRUE(platform_sp);
-  auto path_or_err = platform_sp->ResolveSDKPathFromDebugInfo(*module);
-  EXPECT_FALSE(static_cast<bool>(path_or_err));
-  llvm::consumeError(path_or_err.takeError());
 }
 
 TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
@@ -207,9 +204,6 @@ TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
 
   auto platform_sp = Platform::GetHostPlatform();
   ASSERT_TRUE(platform_sp);
-  auto path_or_err = platform_sp->ResolveSDKPathFromDebugInfo(*module);
-  EXPECT_FALSE(static_cast<bool>(path_or_err));
-  llvm::consumeError(path_or_err.takeError());
 }
 
 TEST_P(SDKPathParsingMultiparamTests, TestSDKPathFromDebugInfo) {

adrian-prantl · 2025-06-25T17:52:36Z

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

      }
+      // getSDKfortriple()


Maybe reword this as:
// TODO: This result of this loop is almost equivalent to deriving the SDK from the target triple, which would be a lot cheaper.

Initially, I did not intend to leave this here. I have now added the TODO comment you suggested.

adrian-prantl

LGTM

…vm#145744)" This reverts commit 7381d81.

…es (#145864) Revert the changes made in the following PRs as they are causing bot failures: - #145744 - #144913

…SystemModules (#145864) Revert the changes made in the following PRs as they are causing bot failures: - llvm/llvm-project#145744 - llvm/llvm-project#144913

This patch is part of an effort to remove the `ResolveSDKPathFromDebugInfo` method, and more specifically the variant which takes a `Module` as argument. This PR should be merged after llvm#144913.

…es (llvm#145864) Revert the changes made in the following PRs as they are causing bot failures: - llvm#145744 - llvm#144913

charles-zablit added 3 commits June 19, 2025 16:17

[lldb][darwin] force BuiltinHeadersInSystemModules to be always false

e28a9e6

remove unused variable

d8f808b

remove ResolveSDKPathFromDebugInfo

8fc573c

charles-zablit requested a review from JDevlieghere as a code owner June 25, 2025 17:33

charles-zablit self-assigned this Jun 25, 2025

charles-zablit requested review from adrian-prantl and Michael137 and removed request for JDevlieghere and adrian-prantl June 25, 2025 17:33

llvmbot added the lldb label Jun 25, 2025

charles-zablit requested a review from adrian-prantl June 25, 2025 17:34

remove comment

5d4b702

adrian-prantl reviewed Jun 25, 2025

View reviewed changes

adrian-prantl approved these changes Jun 25, 2025

View reviewed changes

add TODO

1715c0e

charles-zablit merged commit 7381d81 into llvm:main Jun 25, 2025
7 checks passed

charles-zablit added a commit to charles-zablit/llvm-project that referenced this pull request Jun 26, 2025

Revert "[lldb][NFC] remove the ResolveSDKPathFromDebugInfo method (ll…

a540d68

…vm#145744)" This reverts commit 7381d81.

charles-zablit mentioned this pull request Jun 26, 2025

[lldb][Darwin] revert change to lang_opts.BuiltinHeadersInSystemModules #145864

Merged

Michael137 pushed a commit that referenced this pull request Jun 26, 2025

[lldb][Darwin] revert change to lang_opts.BuiltinHeadersInSystemModul…

9ae41f0

…es (#145864) Revert the changes made in the following PRs as they are causing bot failures: - #145744 - #144913

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][NFC] remove the ResolveSDKPathFromDebugInfo method #145744

[lldb][NFC] remove the ResolveSDKPathFromDebugInfo method #145744

Uh oh!

charles-zablit commented Jun 25, 2025

Uh oh!

llvmbot commented Jun 25, 2025

Uh oh!

adrian-prantl Jun 25, 2025

Uh oh!

charles-zablit Jun 25, 2025

Uh oh!

adrian-prantl left a comment

Uh oh!

Uh oh!

Uh oh!

[lldb][NFC] remove the ResolveSDKPathFromDebugInfo method #145744

[lldb][NFC] remove the ResolveSDKPathFromDebugInfo method #145744

Uh oh!

Conversation

charles-zablit commented Jun 25, 2025

Uh oh!

llvmbot commented Jun 25, 2025

Uh oh!

adrian-prantl Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

charles-zablit Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

adrian-prantl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!