[Lldb-commits] [lldb] Reland "[lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (#102309)" (PR #102497)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Thu Aug 8 09:03:56 PDT 2024


https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/102497

Depends on https://github.com/llvm/llvm-project/pull/102488

This reverts commit https://github.com/llvm/llvm-project/commit/cf56e265e4b74799aee72a04b634fcc261078084.

The original change was reverted because it was causing linker failures
in the unit-tests:
```
Undefined symbols for architecture arm64:
  "lldb_private::PlatformDarwin::GetSDKPathFromDebugInfo(lldb_private::Module&)",
referenced from:
      lldb_private::ClangExpressionParser::ClangExpressionParser(lldb_private::ExecutionContextScope*,
lldb_private::Expression&, bool,
std::__1::vector<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char>>,
std::__1::allocator<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char>>>>,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char>>) in
liblldbPluginExpressionParserClang.a[11](ClangExpressionParser.cpp.o)
ld: symbol(s) not found for architecture arm64
c++: error: linker command failed with exit code 1 (use -v to see
invocation)
```

The relanded version differs only in the fact that we now use the
generic `Platform` abstraction to get to `GetSDKPathFromDebugInfo`.

>From d2f0b68afe6ebfffe5a2ef3ce14a77a5e1025c90 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 8 Aug 2024 16:07:21 +0100
Subject: [PATCH 1/2] [lldb][Platform] Move the GetSDKPathFromDebugInfo helpers
 from PlatformDarwin into Platform

This will soon be needed for
https://github.com/llvm/llvm-project/pull/102309, where we
plan on calling these APIs from generic ExpressionParser code.
---
 lldb/include/lldb/Target/Platform.h           | 34 +++++++++++++++++++
 .../Plugins/Platform/MacOSX/PlatformDarwin.h  | 31 +++--------------
 .../SymbolFile/DWARF/XcodeSDKModuleTests.cpp  | 12 +++++--
 3 files changed, 48 insertions(+), 29 deletions(-)

diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 5ed2fc33356d9d..8cf52a486e2c87 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -27,6 +27,7 @@
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/Utility/Timeout.h"
 #include "lldb/Utility/UserIDResolver.h"
+#include "lldb/Utility/XcodeSDK.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-public.h"
 
@@ -436,6 +437,39 @@ 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.", __func__, 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.", __func__, GetName()));
+  }
+
   const std::string &GetRemoteURL() const { return m_remote_url; }
 
   bool IsHost() const {
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index ff7087da6825d9..66a26d2f496776 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -124,32 +124,11 @@ class PlatformDarwin : public PlatformPOSIX {
   /// located in.
   static FileSpec GetCurrentCommandLineToolsDirectory();
 
-  /// 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).
-  static llvm::Expected<std::pair<XcodeSDK, bool>>
-  GetSDKPathFromDebugInfo(Module &module);
-
-  /// 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.
-  static llvm::Expected<std::string>
-  ResolveSDKPathFromDebugInfo(Module &module);
+  llvm::Expected<std::pair<XcodeSDK, bool>>
+  GetSDKPathFromDebugInfo(Module &module) override;
+
+  llvm::Expected<std::string>
+  ResolveSDKPathFromDebugInfo(Module &module) override;
 
 protected:
   static const char *GetCompatibleArch(ArchSpec::Core core, size_t idx);
diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
index c37da89ff79ce7..dc5680522e1836 100644
--- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -162,7 +162,9 @@ TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
   ModuleSP module = t.GetModule();
   ASSERT_NE(module, nullptr);
 
-  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  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());
 }
@@ -206,7 +208,9 @@ TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
   ModuleSP module = t.GetModule();
   ASSERT_NE(module, nullptr);
 
-  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  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());
 }
@@ -254,7 +258,9 @@ TEST_P(SDKPathParsingMultiparamTests, TestSDKPathFromDebugInfo) {
   ModuleSP module = t.GetModule();
   ASSERT_NE(module, nullptr);
 
-  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  auto platform_sp = Platform::GetHostPlatform();
+  ASSERT_TRUE(platform_sp);
+  auto sdk_or_err = platform_sp->GetSDKPathFromDebugInfo(*module);
   ASSERT_TRUE(static_cast<bool>(sdk_or_err));
 
   auto [sdk, found_mismatch] = *sdk_or_err;

>From 4b73f06eb085435cdcdfbee0179fb08581d939ab Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 8 Aug 2024 15:14:46 +0100
Subject: [PATCH 2/2] Reland "[lldb][ClangExpressionParser] Set
 BuiltinHeadersInSystemModules depending on SDK version (#102309)"

Depends on https://github.com/llvm/llvm-project/pull/102488

This reverts commit cf56e265e4b74799aee72a04b634fcc261078084.

The original change was reverted because it was causing linker failures
in the unit-tests:
```
Undefined symbols for architecture arm64:
  "lldb_private::PlatformDarwin::GetSDKPathFromDebugInfo(lldb_private::Module&)",
referenced from:
      lldb_private::ClangExpressionParser::ClangExpressionParser(lldb_private::ExecutionContextScope*,
lldb_private::Expression&, bool,
std::__1::vector<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char>>,
std::__1::allocator<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char>>>>,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char>>) in
liblldbPluginExpressionParserClang.a[11](ClangExpressionParser.cpp.o)
ld: symbol(s) not found for architecture arm64
c++: error: linker command failed with exit code 1 (use -v to see
invocation)
```

The relanded version differs only in the fact that we now use the
generic `Platform` abstraction to get to `GetSDKPathFromDebugInfo`.
---
 lldb/include/lldb/Utility/XcodeSDK.h          | 14 ++++
 .../Clang/ClangExpressionParser.cpp           | 66 ++++++++++++++++++-
 lldb/source/Utility/XcodeSDK.cpp              | 21 ++++++
 3 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/lldb/include/lldb/Utility/XcodeSDK.h b/lldb/include/lldb/Utility/XcodeSDK.h
index 673ea578ffce85..2720d0d8a44a11 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -85,6 +85,20 @@ class XcodeSDK {
   /// Whether LLDB feels confident importing Clang modules from this SDK.
   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 f41323d32ac863..aa57179f0e79cd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/Basic/DarwinSDKInfo.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
@@ -39,6 +40,7 @@
 #include "llvm/ExecutionEngine/ExecutionEngine.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/TargetSelect.h"
 
@@ -91,6 +93,8 @@
 #include "lldb/Utility/StringList.h"
 
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwin.h"
+#include "lldb/Utility/XcodeSDK.h"
 
 #include <cctype>
 #include <memory>
@@ -279,6 +283,53 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
   std::string m_output;
 };
 
+/// 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) {
+#ifndef __APPLE__
+  return false;
+#else
+  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());
+#endif
+}
+
 static void SetupModuleHeaderPaths(CompilerInstance *compiler,
                                    std::vector<std::string> include_directories,
                                    lldb::TargetSP target_sp) {
@@ -561,7 +612,9 @@ static void SetupLangOpts(CompilerInstance &compiler,
   lang_opts.NoBuiltin = true;
 }
 
-static void SetupImportStdModuleLangOpts(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.
@@ -578,7 +631,14 @@ static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) {
   lang_opts.GNUMode = true;
   lang_opts.GNUKeywords = true;
   lang_opts.CPlusPlus11 = true;
-  lang_opts.BuiltinHeadersInSystemModules = true;
+
+  auto supported_or_err = sdkSupportsBuiltinModules(target);
+  if (supported_or_err)
+    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}");
 
   // The Darwin libc expects this macro to be set.
   lang_opts.GNUCVersion = 40201;
@@ -659,7 +719,7 @@ ClangExpressionParser::ClangExpressionParser(
   if (auto *clang_expr = dyn_cast<ClangUserExpression>(&m_expr);
       clang_expr && clang_expr->DidImportCxxModules()) {
     LLDB_LOG(log, "Adding lang options for importing C++ modules");
-    SetupImportStdModuleLangOpts(*m_compiler);
+    SetupImportStdModuleLangOpts(*m_compiler, *target_sp);
     SetupModuleHeaderPaths(m_compiler.get(), m_include_directories, target_sp);
   }
 
diff --git a/lldb/source/Utility/XcodeSDK.cpp b/lldb/source/Utility/XcodeSDK.cpp
index 712d611db28f83..b7d51f7e0827ac 100644
--- a/lldb/source/Utility/XcodeSDK.cpp
+++ b/lldb/source/Utility/XcodeSDK.cpp
@@ -259,6 +259,27 @@ 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();



More information about the lldb-commits mailing list