Reland "[lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (#102309)" #102497
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rmDarwin into Platform This will soon be needed for llvm#102309, where we plan on calling these APIs from generic ExpressionParser code.
…es depending on SDK version (llvm#102309)" Depends on llvm#102488 This reverts commit cf56e26. 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`.
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesDepends on #102488 This reverts commit cf56e26. The original change was reverted because it was causing linker failures The relanded version differs only in the fact that we now use the Full diff: https://github.com/llvm/llvm-project/pull/102497.diff 6 Files Affected:
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/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/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/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();
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;
|
adrian-prantl
approved these changes
Aug 8, 2024
adrian-prantl
pushed a commit
to adrian-prantl/llvm-project
that referenced
this pull request
Oct 11, 2024
…es depending on SDK version (llvm#102309)" (llvm#102497) Depends on llvm#102488 This reverts commit llvm@cf56e26. 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`. (cherry picked from commit 3fffa6d)
adrian-prantl
pushed a commit
to adrian-prantl/llvm-project
that referenced
this pull request
Oct 11, 2024
…rmDarwin into Platform (llvm#102488) This is needed for relanding llvm#102497, where we plan on calling these APIs from generic ExpressionParser code. (cherry picked from commit 4bb1396)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #102488
This reverts commit cf56e26.
The original change was reverted because it was causing linker failures
in the unit-tests:
The relanded version differs only in the fact that we now use the
generic
Platformabstraction to get toGetSDKPathFromDebugInfo.