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

charles-zablit · 2025-06-19T15:20:38Z

SDKSupportsBuiltinModules always returns true on newer versions of Darwin based OS. The only way for this call to return false would be to have a version mismatch between lldb and the SDK (recent lldb manually installed on macOS 14 for instance).

This patch removes this check and hardcodes the value of BuiltinHeadersInSystemModules to false.

llvmbot · 2025-06-19T15:21:13Z

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

Changes

SDKSupportsBuiltinModules always returns true on newer versions of Darwin based OS. The only way for this call to return false would be to have a version mismatch between lldb and the SDK (recent lldb manually installed on macOS 14 for instance).

This patch removes this check and hardcodes the value of BuiltinHeadersInSystemModules to false.

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

3 Files Affected:

(modified) lldb/include/lldb/Utility/XcodeSDK.h (-13)
(modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+1-49)
(modified) lldb/source/Utility/XcodeSDK.cpp (-21)

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..3caf30c5822a2 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) {
@@ -723,12 +680,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/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();

Michael137 · 2025-06-20T08:13:56Z

@ian-twilightcoder to chime in on the details again. Happy to give it a shot if nothing breaks (I assume you tried running the API test-suite locally?)

charles-zablit · 2025-06-20T17:31:32Z

I only get one test failures when running the check-lldb-api target: commands/platform/sdk/TestPlatformSDK.py but it also happens on the main branch, so there's probably something wrong with the SDK configuration on my system.

ian-twilightcoder · 2025-06-20T18:29:46Z

This really shouldn't be assumed or copy/pasted, we should just be constructing the invocation/instance through the driver. IMO I'd rather have some logic copy/pasted than have it hard coded again.

adrian-prantl

This LGTM. There is no good reason to support debugging with modules against an older SDK, since you can't actually install one easily either.

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.

…(#145744) 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/llvm-project#144913.

Michael137 · 2025-06-26T09:11:47Z

Looks like this does break all the modules tests: https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/as-lldb-cmake/28395/execution/node/106/log/

09:51:43  ********************
09:51:43  Failed Tests (19):
09:51:43    lldb-api :: commands/expression/import-std-module/array/TestArrayFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/basic/TestImportStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/conflicts/TestStdModuleWithConflicts.py
09:51:43    lldb-api :: commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/iterator/TestIteratorFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/list/TestListFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
09:51:43    lldb-api :: commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/shared_ptr-dbg-info-content/TestSharedPtrDbgInfoContentFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
09:51:43    lldb-api :: commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
09:51:43    lldb-api :: commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
09:51:43  
09:51:43

Could we revert for now to unblock bots?

…ys false (llvm#144913)" This reverts commit 487581b.

charles-zablit · 2025-06-26T10:06:34Z

Looks like this does break all the modules tests: https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/as-lldb-cmake/28395/execution/node/106/log/
...
Could we revert for now to unblock bots?

Just opened a revert PR for this commit and a child commit.

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

…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

adrian-prantl · 2025-06-26T14:35:25Z

I think the one thing I did not consider is that our own bots are running on an older version of the tools:
From green dragon:

2025-06-26T14:14:59.504Z] ProductName:		macOS
[2025-06-26T14:14:59.504Z] ProductVersion:		14.1
[2025-06-26T14:14:59.504Z] BuildVersion:		23B74
[2025-06-26T14:14:59.504Z] + xcodebuild -version
[2025-06-26T14:14:59.504Z] Xcode 15.2
[2025-06-26T14:14:59.504Z] Build version 15C5500c
[2025-06-26T14:14:59.504Z] + cmake --version
[2025-06-26T14:14:59.761Z] cmake version 3.30.2

adrian-prantl · 2025-06-26T14:36:06Z

I'll check into getting them updated.

adrian-prantl · 2025-06-26T14:39:06Z

We could mark these tests as requiring macOS 15+, but given how often they break I am not sure we want to do that?

Michael137 · 2025-06-26T16:11:25Z

We could mark these tests as requiring macOS 15+, but given how often they break I am not sure we want to do that?

Happy to mark them as unsupported on older SDKs if it's blocking other work.

ian-twilightcoder · 2025-06-26T16:54:27Z

It's not just BuiltinHeadersInSystemModules either. There are lots of things that the driver toolchain does that you shouldn't be skipping.

…llvm#144913) `SDKSupportsBuiltinModules` always returns true on newer versions of Darwin based OS. The only way for this call to return `false` would be to have a version mismatch between lldb and the SDK (recent lldb manually installed on macOS 14 for instance). This patch removes this check and hardcodes the value of `BuiltinHeadersInSystemModules` to `false`.

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

adrian-prantl · 2025-06-26T21:58:43Z

It's not just BuiltinHeadersInSystemModules either. There are lots of things that the driver toolchain does that you shouldn't be skipping.

We should definitely also find a way to factor that into a function LLDB can call.

ian-twilightcoder · 2025-06-26T22:04:02Z

It's not just BuiltinHeadersInSystemModules either. There are lots of things that the driver toolchain does that you shouldn't be skipping.

We should definitely also find a way to factor that into a function LLDB can call.

LLDB can just use the driver to construct their compiler invocation/instance the same way other llvm/clang tools do. There's a few different ways, and I forget the one we thought would be the best match for LLDB, but I want to say it's what tapi does.

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

e28a9e6

charles-zablit requested a review from JDevlieghere as a code owner June 19, 2025 15:20

llvmbot added the lldb label Jun 19, 2025

remove unused variable

d8f808b

charles-zablit self-assigned this Jun 23, 2025

charles-zablit mentioned this pull request Jun 25, 2025

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

Merged

adrian-prantl approved these changes Jun 25, 2025

View reviewed changes

adrian-prantl merged commit 487581b 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][darwin] force BuiltinHeadersInSystemModules to be alwa…

4ee304f

…ys false (llvm#144913)" This reverts commit 487581b.

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

charles-zablit mentioned this pull request Jun 27, 2025

[lldb][NFC] Inline ResolveSDKPathFromDebugInfo in one of its call site #146062

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

charles-zablit commented Jun 19, 2025

Uh oh!

llvmbot commented Jun 19, 2025

Uh oh!

Michael137 commented Jun 20, 2025

Uh oh!

charles-zablit commented Jun 20, 2025

Uh oh!

ian-twilightcoder commented Jun 20, 2025

Uh oh!

adrian-prantl left a comment

Uh oh!

Uh oh!

Michael137 commented Jun 26, 2025

Uh oh!

charles-zablit commented Jun 26, 2025 •

edited

Loading

Uh oh!

adrian-prantl commented Jun 26, 2025

Uh oh!

adrian-prantl commented Jun 26, 2025

Uh oh!

adrian-prantl commented Jun 26, 2025

Uh oh!

Michael137 commented Jun 26, 2025

Uh oh!

ian-twilightcoder commented Jun 26, 2025

Uh oh!

adrian-prantl commented Jun 26, 2025

Uh oh!

ian-twilightcoder commented Jun 26, 2025

Uh oh!

Uh oh!

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

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

Conversation

charles-zablit commented Jun 19, 2025

Uh oh!

llvmbot commented Jun 19, 2025

Uh oh!

Michael137 commented Jun 20, 2025

Uh oh!

charles-zablit commented Jun 20, 2025

Uh oh!

ian-twilightcoder commented Jun 20, 2025

Uh oh!

adrian-prantl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Michael137 commented Jun 26, 2025

Uh oh!

charles-zablit commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adrian-prantl commented Jun 26, 2025

Uh oh!

adrian-prantl commented Jun 26, 2025

Uh oh!

adrian-prantl commented Jun 26, 2025

Uh oh!

Michael137 commented Jun 26, 2025

Uh oh!

ian-twilightcoder commented Jun 26, 2025

Uh oh!

adrian-prantl commented Jun 26, 2025

Uh oh!

ian-twilightcoder commented Jun 26, 2025

Uh oh!

Uh oh!

charles-zablit commented Jun 26, 2025 •

edited

Loading