From 2665e747d5866480f2f704d5a72fe016d258f200 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 25 Feb 2025 13:11:33 +0000 Subject: [PATCH 1/8] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths `GetSDKRoot` uses `xcrun` to find an SDK root path for a given SDK version string. But if the SDK doesn't exist in the Xcode installations, but instead lives in the `CommandLineTools`, `xcrun` will fail to find it. Negative searches for an SDK path cost a lot (a few seconds) each time `xcrun` is invoked. We do cache negative results in `find_cached_path` inside LLDB, but we would still pay the price on every new debug session the first time we evaluate an expression. This doesn't only cause a noticable delay in running the expression, but also generates following error: ``` error: Error while searching for Xcode SDK: timed out waiting for shell command to complete (int) $0 = 42 ``` To avoid this `xcrun` penalty, we search `CommandLineTools` for a matching SDK ourselves, and only if we don't find it, do we fall back to calling `xcrun`. rdar://113619904 rdar://113619723 --- lldb/include/lldb/Host/FileSystem.h | 5 +- .../Host/macosx/objcxx/HostInfoMacOSX.mm | 60 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h index 640f3846e448c..4128d7b012041 100644 --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -183,8 +183,9 @@ class FileSystem { eEnumerateDirectoryResultQuit }; - typedef EnumerateDirectoryResult (*EnumerateDirectoryCallbackType)( - void *baton, llvm::sys::fs::file_type file_type, llvm::StringRef); + typedef std::function + EnumerateDirectoryCallbackType; typedef std::function diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm index 6e924fdc684cf..a94fd3b57f9d6 100644 --- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm +++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm @@ -15,11 +15,14 @@ #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 @@ -569,10 +572,52 @@ 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 +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> 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 HostInfoMacOSX::GetSDKRoot(SDKOptions options) { static llvm::StringMap g_sdk_path; static std::mutex g_sdk_path_mutex; @@ -581,6 +626,21 @@ 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); }); From 96ae6dbf77692fb063eae65a15730997b3f65303 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 28 Feb 2025 12:55:03 +0000 Subject: [PATCH 2/8] fixup! if path at DW_AT_LLVM_sysroot exists, use it --- lldb/include/lldb/Host/FileSystem.h | 5 +- lldb/include/lldb/Symbol/SymbolFile.h | 6 +- lldb/include/lldb/Symbol/SymbolFileOnDemand.h | 2 +- lldb/include/lldb/Target/Platform.h | 24 +------- .../Host/macosx/objcxx/HostInfoMacOSX.mm | 60 ------------------- .../Clang/ClangExpressionParser.cpp | 8 +-- .../Platform/MacOSX/PlatformDarwin.cpp | 56 ++++------------- .../Plugins/Platform/MacOSX/PlatformDarwin.h | 6 +- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 32 ++++++---- .../SymbolFile/DWARF/SymbolFileDWARF.h | 3 +- .../DWARF/SymbolFileDWARFDebugMap.cpp | 3 +- .../DWARF/SymbolFileDWARFDebugMap.h | 3 +- lldb/source/Symbol/SymbolFileOnDemand.cpp | 7 ++- .../SymbolFile/DWARF/XcodeSDKModuleTests.cpp | 2 +- 14 files changed, 54 insertions(+), 163 deletions(-) diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h index 4128d7b012041..640f3846e448c 100644 --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -183,9 +183,8 @@ class FileSystem { eEnumerateDirectoryResultQuit }; - typedef std::function - EnumerateDirectoryCallbackType; + typedef EnumerateDirectoryResult (*EnumerateDirectoryCallbackType)( + void *baton, llvm::sys::fs::file_type file_type, llvm::StringRef); typedef std::function diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index 837b922ae77f7..d417899e97084 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -32,6 +32,7 @@ #include #include #include +#include #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 + 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 diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h index 7a366bfabec86..77e7b50539043 100644 --- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h +++ b/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 ParseXcodeSDK(lldb_private::CompileUnit &comp_unit) override; void InitializeObject() override; diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h index a702abb540fd9..5f85caeb790ba 100644 --- a/lldb/include/lldb/Target/Platform.h +++ b/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> - 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 GetSDKPathFromDebugInfo(CompileUnit &unit) { + /// \returns A parsed XcodeSDK object if successful, an Error otherwise. + virtual llvm::Expected> + GetSDKPathFromDebugInfo(CompileUnit &unit) { return llvm::createStringError( llvm::formatv("{0} not implemented for '{1}' platform.", LLVM_PRETTY_FUNCTION, GetName())); diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm index a94fd3b57f9d6..6e924fdc684cf 100644 --- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm +++ b/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 -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> 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 HostInfoMacOSX::GetSDKRoot(SDKOptions options) { static llvm::StringMap 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); }); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index f1573bae2651b..e980c914afd1a 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/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(); diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 51e9a6d81b839..ef10481503efe 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1386,57 +1386,18 @@ llvm::Triple::OSType PlatformDarwin::GetHostOSType() { #endif // __APPLE__ } -llvm::Expected> -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 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("")); - return path_or_err->str(); + return ResolveSDKPathFromDebugInfo(*cu_sp); } -llvm::Expected +llvm::Expected> 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) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h index e2d4cb6726d58..0a4e6c9938c17 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h @@ -124,13 +124,11 @@ class PlatformDarwin : public PlatformPOSIX { /// located in. static FileSpec GetCurrentCommandLineToolsDirectory(); - llvm::Expected> - GetSDKPathFromDebugInfo(Module &module) override; - llvm::Expected ResolveSDKPathFromDebugInfo(Module &module) override; - llvm::Expected GetSDKPathFromDebugInfo(CompileUnit &unit) override; + llvm::Expected> + GetSDKPathFromDebugInfo(CompileUnit &unit) override; llvm::Expected ResolveSDKPathFromDebugInfo(CompileUnit &unit) override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index a96757afabddf..714586a4e2468 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/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 +SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) { std::lock_guard 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) { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 7309f7a86b659..8f7f84aff355d 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/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 + ParseXcodeSDK(CompileUnit &comp_unit) override; size_t ParseFunctions(CompileUnit &comp_unit) override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 0ecf47a3c7869..eb2db68dd9435 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/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 +SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) { std::lock_guard guard(GetModuleMutex()); SymbolFileDWARF *oso_dwarf = GetSymbolFile(comp_unit); if (oso_dwarf) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index df41d6a2a4e42..9f1fe9abe002b 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/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 + ParseXcodeSDK(CompileUnit &comp_unit) override; llvm::SmallSet ParseAllLanguages(CompileUnit &comp_unit) override; size_t ParseFunctions(CompileUnit &comp_unit) override; diff --git a/lldb/source/Symbol/SymbolFileOnDemand.cpp b/lldb/source/Symbol/SymbolFileOnDemand.cpp index 94979b2fb1c22..94c6e01d83d1d 100644 --- a/lldb/source/Symbol/SymbolFileOnDemand.cpp +++ b/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 +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); } diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp index fc008ca7011e4..bda69815ad388 100644 --- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp +++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp @@ -118,7 +118,7 @@ TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) { ASSERT_TRUE(static_cast(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); } From 4d7c510e79ac33ed3bd72b073f7876b51735b338 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 3 Mar 2025 12:31:47 +0000 Subject: [PATCH 3/8] Revert "fixup! if path at DW_AT_LLVM_sysroot exists, use it" This reverts commit 96ae6dbf77692fb063eae65a15730997b3f65303. --- lldb/include/lldb/Host/FileSystem.h | 5 +- lldb/include/lldb/Symbol/SymbolFile.h | 6 +- lldb/include/lldb/Symbol/SymbolFileOnDemand.h | 2 +- lldb/include/lldb/Target/Platform.h | 24 +++++++- .../Host/macosx/objcxx/HostInfoMacOSX.mm | 60 +++++++++++++++++++ .../Clang/ClangExpressionParser.cpp | 8 ++- .../Platform/MacOSX/PlatformDarwin.cpp | 56 +++++++++++++---- .../Plugins/Platform/MacOSX/PlatformDarwin.h | 6 +- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 32 ++++------ .../SymbolFile/DWARF/SymbolFileDWARF.h | 3 +- .../DWARF/SymbolFileDWARFDebugMap.cpp | 3 +- .../DWARF/SymbolFileDWARFDebugMap.h | 3 +- lldb/source/Symbol/SymbolFileOnDemand.cpp | 7 +-- .../SymbolFile/DWARF/XcodeSDKModuleTests.cpp | 2 +- 14 files changed, 163 insertions(+), 54 deletions(-) diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h index 640f3846e448c..4128d7b012041 100644 --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -183,8 +183,9 @@ class FileSystem { eEnumerateDirectoryResultQuit }; - typedef EnumerateDirectoryResult (*EnumerateDirectoryCallbackType)( - void *baton, llvm::sys::fs::file_type file_type, llvm::StringRef); + typedef std::function + EnumerateDirectoryCallbackType; typedef std::function diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index d417899e97084..837b922ae77f7 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -32,7 +32,6 @@ #include #include #include -#include #if defined(LLDB_CONFIGURATION_DEBUG) #define ASSERT_MODULE_LOCK(expr) (expr->AssertModuleLock()) @@ -149,10 +148,7 @@ class SymbolFile : public PluginInterface { virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0; /// Return the Xcode SDK comp_unit was compiled against. - virtual std::pair - ParseXcodeSDK(CompileUnit &comp_unit) { - return {}; - } + virtual XcodeSDK 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 diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h index 77e7b50539043..7a366bfabec86 100644 --- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h +++ b/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; - std::pair + lldb_private::XcodeSDK ParseXcodeSDK(lldb_private::CompileUnit &comp_unit) override; void InitializeObject() override; diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h index 5f85caeb790ba..a702abb540fd9 100644 --- a/lldb/include/lldb/Target/Platform.h +++ b/lldb/include/lldb/Target/Platform.h @@ -438,6 +438,25 @@ 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> + 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`). @@ -458,9 +477,8 @@ class Platform : public PluginInterface { /// /// \param[in] unit The CU /// - /// \returns A parsed XcodeSDK object if successful, an Error otherwise. - virtual llvm::Expected> - GetSDKPathFromDebugInfo(CompileUnit &unit) { + /// \returns A parsed XcodeSDK object if successful, an Error otherwise. + virtual llvm::Expected GetSDKPathFromDebugInfo(CompileUnit &unit) { return llvm::createStringError( llvm::formatv("{0} not implemented for '{1}' platform.", LLVM_PRETTY_FUNCTION, GetName())); diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm index 6e924fdc684cf..a94fd3b57f9d6 100644 --- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm +++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm @@ -15,11 +15,14 @@ #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 @@ -569,10 +572,52 @@ 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 +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> 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 HostInfoMacOSX::GetSDKRoot(SDKOptions options) { static llvm::StringMap g_sdk_path; static std::mutex g_sdk_path_mutex; @@ -581,6 +626,21 @@ 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); }); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index e980c914afd1a..f1573bae2651b 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -336,7 +336,13 @@ sdkSupportsBuiltinModules(lldb_private::Target &target) { if (!platform_sp) return llvm::createStringError("No Platform plugin found on target."); - auto sdk_path_or_err = platform_sp->ResolveSDKPathFromDebugInfo(*module_sp); + 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(); diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index ef10481503efe..51e9a6d81b839 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1386,18 +1386,57 @@ llvm::Triple::OSType PlatformDarwin::GetHostOSType() { #endif // __APPLE__ } +llvm::Expected> +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 PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) { - auto cu_sp = module.GetCompileUnitAtIndex(0); - if (!cu_sp) + auto sdk_or_err = GetSDKPathFromDebugInfo(module); + if (!sdk_or_err) return llvm::createStringError( - "Couldn't retrieve compile-unit for module '%s'.", - module.GetObjectName().AsCString("")); + 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()))); - return ResolveSDKPathFromDebugInfo(*cu_sp); + return path_or_err->str(); } -llvm::Expected> +llvm::Expected PlatformDarwin::GetSDKPathFromDebugInfo(CompileUnit &unit) { ModuleSP module_sp = unit.CalculateSymbolContextModule(); if (!module_sp) @@ -1420,10 +1459,7 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(CompileUnit &unit) { llvm::formatv("Failed to parse SDK path from debug-info: {0}", llvm::toString(sdk_or_err.takeError()))); - auto [sdk, sysroot] = std::move(*sdk_or_err); - - if (FileSystem::Instance().Exists(sysroot)) - return sysroot; + auto sdk = std::move(*sdk_or_err); auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk}); if (!path_or_err) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h index 0a4e6c9938c17..e2d4cb6726d58 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h @@ -124,11 +124,13 @@ class PlatformDarwin : public PlatformPOSIX { /// located in. static FileSpec GetCurrentCommandLineToolsDirectory(); + llvm::Expected> + GetSDKPathFromDebugInfo(Module &module) override; + llvm::Expected ResolveSDKPathFromDebugInfo(Module &module) override; - llvm::Expected> - GetSDKPathFromDebugInfo(CompileUnit &unit) override; + llvm::Expected GetSDKPathFromDebugInfo(CompileUnit &unit) override; llvm::Expected ResolveSDKPathFromDebugInfo(CompileUnit &unit) override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 714586a4e2468..a96757afabddf 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -982,8 +982,7 @@ lldb::LanguageType SymbolFileDWARF::ParseLanguage(CompileUnit &comp_unit) { return eLanguageTypeUnknown; } -std::pair -SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) { +XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) { std::lock_guard guard(GetModuleMutex()); DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit); if (!dwarf_cu) @@ -994,26 +993,21 @@ SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) { const char *sdk = cu_die.GetAttributeValueAsString(DW_AT_APPLE_sdk, nullptr); if (!sdk) return {}; - std::string sysroot = + const char *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); - // 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); - } + ModuleSP local_module_sp = m_objfile_sp->GetModule(); + if (local_module_sp && local_module_sp != module_sp) + local_module_sp->RegisterXcodeSDK(sdk, sysroot); - return std::pair{XcodeSDK{sdk}, std::move(sysroot)}; + return {sdk}; } size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 8f7f84aff355d..7309f7a86b659 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -110,8 +110,7 @@ class SymbolFileDWARF : public SymbolFileCommon { lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) override; - std::pair - ParseXcodeSDK(CompileUnit &comp_unit) override; + XcodeSDK ParseXcodeSDK(CompileUnit &comp_unit) override; size_t ParseFunctions(CompileUnit &comp_unit) override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index eb2db68dd9435..0ecf47a3c7869 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -673,8 +673,7 @@ SymbolFileDWARFDebugMap::ParseLanguage(CompileUnit &comp_unit) { return eLanguageTypeUnknown; } -std::pair -SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) { +XcodeSDK SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) { std::lock_guard guard(GetModuleMutex()); SymbolFileDWARF *oso_dwarf = GetSymbolFile(comp_unit); if (oso_dwarf) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index 9f1fe9abe002b..df41d6a2a4e42 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -64,8 +64,7 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon { // Compile Unit function calls lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) override; - std::pair - ParseXcodeSDK(CompileUnit &comp_unit) override; + XcodeSDK ParseXcodeSDK(CompileUnit &comp_unit) override; llvm::SmallSet ParseAllLanguages(CompileUnit &comp_unit) override; size_t ParseFunctions(CompileUnit &comp_unit) override; diff --git a/lldb/source/Symbol/SymbolFileOnDemand.cpp b/lldb/source/Symbol/SymbolFileOnDemand.cpp index 94c6e01d83d1d..94979b2fb1c22 100644 --- a/lldb/source/Symbol/SymbolFileOnDemand.cpp +++ b/lldb/source/Symbol/SymbolFileOnDemand.cpp @@ -58,18 +58,17 @@ lldb::LanguageType SymbolFileOnDemand::ParseLanguage(CompileUnit &comp_unit) { return m_sym_file_impl->ParseLanguage(comp_unit); } -std::pair -SymbolFileOnDemand::ParseXcodeSDK(CompileUnit &comp_unit) { +XcodeSDK 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) { - auto [sdk, sysroot] = m_sym_file_impl->ParseXcodeSDK(comp_unit); + XcodeSDK sdk = 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); } diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp index bda69815ad388..fc008ca7011e4 100644 --- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp +++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp @@ -118,7 +118,7 @@ TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) { ASSERT_TRUE(static_cast(comp_unit.get())); ModuleSP module = t.GetModule(); ASSERT_EQ(module->GetSourceMappingList().GetSize(), 0u); - auto [sdk, sysroot] = sym_file.ParseXcodeSDK(*comp_unit); + XcodeSDK sdk = sym_file.ParseXcodeSDK(*comp_unit); ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX); ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u); } From 2b5628eb2d9515d9beeb767f106e8a39b980e309 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 3 Mar 2025 12:31:48 +0000 Subject: [PATCH 4/8] Revert "[lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths" This reverts commit 2665e747d5866480f2f704d5a72fe016d258f200. --- lldb/include/lldb/Host/FileSystem.h | 5 +- .../Host/macosx/objcxx/HostInfoMacOSX.mm | 60 ------------------- 2 files changed, 2 insertions(+), 63 deletions(-) diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h index 4128d7b012041..640f3846e448c 100644 --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -183,9 +183,8 @@ class FileSystem { eEnumerateDirectoryResultQuit }; - typedef std::function - EnumerateDirectoryCallbackType; + typedef EnumerateDirectoryResult (*EnumerateDirectoryCallbackType)( + void *baton, llvm::sys::fs::file_type file_type, llvm::StringRef); typedef std::function diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm index a94fd3b57f9d6..6e924fdc684cf 100644 --- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm +++ b/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 -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> 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 HostInfoMacOSX::GetSDKRoot(SDKOptions options) { static llvm::StringMap 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); }); From 907b2f4682528ca06856f1a6690607f2ee19c94c Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 28 Feb 2025 17:54:36 +0000 Subject: [PATCH 5/8] fixup! store sysroot inside XcodeSDK --- .../include/lldb/Host/macosx/HostInfoMacOSX.h | 3 ++ lldb/include/lldb/Utility/XcodeSDK.h | 5 ++++ .../Platform/MacOSX/PlatformDarwin.cpp | 12 ++++++++ .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 29 +++++++++++-------- lldb/source/Utility/XcodeSDK.cpp | 19 +++++++++--- .../SymbolFile/DWARF/XcodeSDKModuleTests.cpp | 19 ++++++++++-- lldb/unittests/Utility/XcodeSDKTest.cpp | 12 ++++++++ 7 files changed, 81 insertions(+), 18 deletions(-) diff --git a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h index 8eb2ede382c22..9034c80fdefa4 100644 --- a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h +++ b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h @@ -32,6 +32,9 @@ class HostInfoMacOSX : public HostInfoPosix { static FileSpec GetXcodeDeveloperDirectory(); /// Query xcrun to find an Xcode SDK directory. + /// + /// Note, this is an expensive operation if the SDK we're querying + /// does not exist in an Xcode installation path on the host. static llvm::Expected GetSDKRoot(SDKOptions options); static llvm::Expected FindSDKTool(XcodeSDK sdk, llvm::StringRef tool); diff --git a/lldb/include/lldb/Utility/XcodeSDK.h b/lldb/include/lldb/Utility/XcodeSDK.h index 2720d0d8a44a1..fa893c0ca8d7e 100644 --- a/lldb/include/lldb/Utility/XcodeSDK.h +++ b/lldb/include/lldb/Utility/XcodeSDK.h @@ -23,6 +23,8 @@ namespace lldb_private { /// An abstraction for Xcode-style SDKs that works like \ref ArchSpec. class XcodeSDK { std::string m_name; + std::optional + m_sysroot_path; // TODO: should this be std::optional? public: /// Different types of Xcode SDKs. @@ -62,6 +64,8 @@ class XcodeSDK { /// directory component of a path one would pass to clang's -isysroot /// parameter. For example, "MacOSX.10.14.sdk". XcodeSDK(std::string &&name) : m_name(std::move(name)) {} + XcodeSDK(std::string name, std::string sysroot) + : m_name(std::move(name)), m_sysroot_path(std::move(sysroot)) {} static XcodeSDK GetAnyMacOS() { return XcodeSDK("MacOSX.sdk"); } /// The merge function follows a strict order to maintain monotonicity: @@ -79,6 +83,7 @@ class XcodeSDK { llvm::VersionTuple GetVersion() const; Type GetType() const; llvm::StringRef GetString() const; + std::optional GetSysroot() const; /// Whether this Xcode SDK supports Swift. bool SupportsSwift() const; diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 51e9a6d81b839..d74c8651e4abc 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -79,6 +79,14 @@ static Status ExceptionMaskValidator(const char *string, void *unused) { return {}; } +static bool XcodeSysrootExists(XcodeSDK const &sdk) { + auto maybe_sysroot = sdk.GetSysroot(); + if (!maybe_sysroot) + return false; + + return FileSystem::Instance().Exists(*maybe_sysroot); +} + /// Destructor. /// /// The destructor is virtual since this class is designed to be @@ -1395,6 +1403,7 @@ PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) { llvm::formatv("No symbol file available for module '{0}'", module.GetFileSpec().GetFilename().AsCString(""))); + std::string sysroot; bool found_public_sdk = false; bool found_internal_sdk = false; XcodeSDK merged_sdk; @@ -1425,6 +1434,9 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) { auto [sdk, _] = std::move(*sdk_or_err); + if (XcodeSysrootExists(sdk)) + return sdk.GetSysroot()->str(); + auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk}); if (!path_or_err) return llvm::createStringError( diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index a96757afabddf..9b5eb68652f58 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -993,21 +993,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 {sdk, std::move(sysroot)}; } size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) { diff --git a/lldb/source/Utility/XcodeSDK.cpp b/lldb/source/Utility/XcodeSDK.cpp index b7d51f7e0827a..9e22720053e91 100644 --- a/lldb/source/Utility/XcodeSDK.cpp +++ b/lldb/source/Utility/XcodeSDK.cpp @@ -142,6 +142,10 @@ XcodeSDK::Type XcodeSDK::GetType() const { llvm::StringRef XcodeSDK::GetString() const { return m_name; } +std::optional XcodeSDK::GetSysroot() const { + return m_sysroot_path; +} + bool XcodeSDK::Info::operator<(const Info &other) const { return std::tie(type, version, internal) < std::tie(other.type, other.version, other.internal); @@ -153,6 +157,10 @@ bool XcodeSDK::Info::operator==(const Info &other) const { } void XcodeSDK::Merge(const XcodeSDK &other) { + auto add_internal_sdk_suffix = [](std::string const &sdk) { + return sdk.substr(0, sdk.size() - 3) + "Internal.sdk"; + }; + // The "bigger" SDK always wins. auto l = Parse(); auto r = other.Parse(); @@ -160,10 +168,13 @@ void XcodeSDK::Merge(const XcodeSDK &other) { *this = other; else { // The Internal flag always wins. - if (llvm::StringRef(m_name).ends_with(".sdk")) - if (!l.internal && r.internal) - m_name = - m_name.substr(0, m_name.size() - 3) + std::string("Internal.sdk"); + if (!l.internal && r.internal) { + if (llvm::StringRef(m_name).ends_with(".sdk")) + m_name = add_internal_sdk_suffix(m_name); + + if (m_sysroot_path && llvm::StringRef(*m_sysroot_path).ends_with(".sdk")) + m_sysroot_path.emplace(add_internal_sdk_suffix(*m_sysroot_path)); + } } } diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp index fc008ca7011e4..c1e471653a279 100644 --- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp +++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp @@ -310,13 +310,28 @@ SDKPathParsingTestData sdkPathParsingTestCases[] = { .expect_internal_sdk = true, .expect_sdk_path_pattern = "Internal.sdk"}, - /// Two CUs with an internal SDK each + /// Two CUs with a public (non-CommandLineTools) SDK each + {.input_sdk_paths = {"/Path/To/SDKs/iPhoneOS14.1.sdk", + "/Path/To/SDKs/MacOSX11.3.sdk"}, + .expect_mismatch = false, + .expect_internal_sdk = false, + .expect_sdk_path_pattern = "iPhoneOS14.1.sdk"}, + + /// One CU with CommandLineTools and the other a public SDK {.input_sdk_paths = {"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.1.sdk", - "/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk"}, + "/Path/To/SDKs/MacOSX11.3.sdk"}, .expect_mismatch = false, .expect_internal_sdk = false, .expect_sdk_path_pattern = "iPhoneOS14.1.sdk"}, + + /// One CU with CommandLineTools and the other an internal SDK + {.input_sdk_paths = + {"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.1.sdk", + "/Path/To/SDKs/MacOSX11.3.Internal.sdk"}, + .expect_mismatch = false, + .expect_internal_sdk = false, + .expect_sdk_path_pattern = "iPhoneOS14.1.Internal.sdk"}, }; INSTANTIATE_TEST_SUITE_P(SDKPathParsingTests, SDKPathParsingMultiparamTests, diff --git a/lldb/unittests/Utility/XcodeSDKTest.cpp b/lldb/unittests/Utility/XcodeSDKTest.cpp index 8bf7ee1be1dba..95307ade4d43a 100644 --- a/lldb/unittests/Utility/XcodeSDKTest.cpp +++ b/lldb/unittests/Utility/XcodeSDKTest.cpp @@ -34,12 +34,16 @@ TEST(XcodeSDKTest, ParseTest) { EXPECT_EQ(XcodeSDK("MacOSX10.9.sdk").GetVersion(), llvm::VersionTuple(10, 9)); EXPECT_EQ(XcodeSDK("MacOSX10.15.4.sdk").GetVersion(), llvm::VersionTuple(10, 15)); EXPECT_EQ(XcodeSDK("MacOSX.sdk").IsAppleInternalSDK(), false); + EXPECT_EQ(XcodeSDK("MacOSX.sdk", "/Path/To/MacOSX.sdk").GetSysroot(), + "/Path/To/MacOSX.sdk"); EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetType(), XcodeSDK::MacOSX); EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetVersion(), llvm::VersionTuple(10, 15)); EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").IsAppleInternalSDK(), true); + EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetSysroot(), std::nullopt); EXPECT_EQ(XcodeSDK().GetType(), XcodeSDK::unknown); EXPECT_EQ(XcodeSDK().GetVersion(), llvm::VersionTuple()); + EXPECT_EQ(XcodeSDK().GetSysroot(), std::nullopt); } TEST(XcodeSDKTest, MergeTest) { @@ -60,6 +64,14 @@ TEST(XcodeSDKTest, MergeTest) { XcodeSDK empty; empty.Merge(XcodeSDK("MacOSX10.14.Internal.sdk")); EXPECT_EQ(empty.GetString(), llvm::StringRef("MacOSX10.14.Internal.sdk")); + EXPECT_EQ(empty.GetSysroot(), std::nullopt); + empty.Merge(XcodeSDK("MacOSX9.5.Internal.sdk", "/Path/To/9.5.sdk")); + EXPECT_EQ(empty.GetSysroot(), std::nullopt); + empty.Merge(XcodeSDK("MacOSX12.5.sdk", "/Path/To/12.5.sdk")); + EXPECT_EQ(empty.GetSysroot(), "/Path/To/12.5.sdk"); + empty.Merge( + XcodeSDK("MacOSX11.5.Internal.sdk", "/Path/To/12.5.Internal.sdk")); + EXPECT_EQ(empty.GetSysroot(), "/Path/To/12.5.Internal.sdk"); } #ifndef _WIN32 From c172ffe4df0abbb8061423465c02813b205692f5 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 3 Mar 2025 13:13:24 +0000 Subject: [PATCH 6/8] fixup! make sysroot member a FileSpec --- lldb/include/lldb/Utility/XcodeSDK.h | 10 ++++---- .../Platform/MacOSX/PlatformDarwin.cpp | 12 ++------- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +- lldb/source/Utility/XcodeSDK.cpp | 12 ++++----- lldb/unittests/Utility/XcodeSDKTest.cpp | 25 ++++++++++--------- 5 files changed, 26 insertions(+), 35 deletions(-) diff --git a/lldb/include/lldb/Utility/XcodeSDK.h b/lldb/include/lldb/Utility/XcodeSDK.h index fa893c0ca8d7e..3a6cbb9c98f6b 100644 --- a/lldb/include/lldb/Utility/XcodeSDK.h +++ b/lldb/include/lldb/Utility/XcodeSDK.h @@ -9,6 +9,7 @@ #ifndef LLDB_UTILITY_SDK_H #define LLDB_UTILITY_SDK_H +#include "lldb/Utility/FileSpec.h" #include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/VersionTuple.h" @@ -23,8 +24,7 @@ namespace lldb_private { /// An abstraction for Xcode-style SDKs that works like \ref ArchSpec. class XcodeSDK { std::string m_name; - std::optional - m_sysroot_path; // TODO: should this be std::optional? + FileSpec m_sysroot; public: /// Different types of Xcode SDKs. @@ -64,8 +64,8 @@ class XcodeSDK { /// directory component of a path one would pass to clang's -isysroot /// parameter. For example, "MacOSX.10.14.sdk". XcodeSDK(std::string &&name) : m_name(std::move(name)) {} - XcodeSDK(std::string name, std::string sysroot) - : m_name(std::move(name)), m_sysroot_path(std::move(sysroot)) {} + XcodeSDK(std::string name, FileSpec sysroot) + : m_name(std::move(name)), m_sysroot(std::move(sysroot)) {} static XcodeSDK GetAnyMacOS() { return XcodeSDK("MacOSX.sdk"); } /// The merge function follows a strict order to maintain monotonicity: @@ -83,7 +83,7 @@ class XcodeSDK { llvm::VersionTuple GetVersion() const; Type GetType() const; llvm::StringRef GetString() const; - std::optional GetSysroot() const; + const FileSpec &GetSysroot() const; /// Whether this Xcode SDK supports Swift. bool SupportsSwift() const; diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index d74c8651e4abc..0bdd1b37d5bd0 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -79,14 +79,6 @@ static Status ExceptionMaskValidator(const char *string, void *unused) { return {}; } -static bool XcodeSysrootExists(XcodeSDK const &sdk) { - auto maybe_sysroot = sdk.GetSysroot(); - if (!maybe_sysroot) - return false; - - return FileSystem::Instance().Exists(*maybe_sysroot); -} - /// Destructor. /// /// The destructor is virtual since this class is designed to be @@ -1434,8 +1426,8 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) { auto [sdk, _] = std::move(*sdk_or_err); - if (XcodeSysrootExists(sdk)) - return sdk.GetSysroot()->str(); + if (FileSystem::Instance().Exists(sdk.GetSysroot())) + return sdk.GetSysroot().GetPath(); auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk}); if (!path_or_err) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 9b5eb68652f58..eecc8269006c2 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1012,7 +1012,7 @@ XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) { local_module_sp->RegisterXcodeSDK(sdk, sysroot); } - return {sdk, std::move(sysroot)}; + return {sdk, FileSpec{std::move(sysroot)}}; } size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) { diff --git a/lldb/source/Utility/XcodeSDK.cpp b/lldb/source/Utility/XcodeSDK.cpp index 9e22720053e91..02cf7866e22fb 100644 --- a/lldb/source/Utility/XcodeSDK.cpp +++ b/lldb/source/Utility/XcodeSDK.cpp @@ -142,9 +142,7 @@ XcodeSDK::Type XcodeSDK::GetType() const { llvm::StringRef XcodeSDK::GetString() const { return m_name; } -std::optional XcodeSDK::GetSysroot() const { - return m_sysroot_path; -} +const FileSpec &XcodeSDK::GetSysroot() const { return m_sysroot; } bool XcodeSDK::Info::operator<(const Info &other) const { return std::tie(type, version, internal) < @@ -157,8 +155,8 @@ bool XcodeSDK::Info::operator==(const Info &other) const { } void XcodeSDK::Merge(const XcodeSDK &other) { - auto add_internal_sdk_suffix = [](std::string const &sdk) { - return sdk.substr(0, sdk.size() - 3) + "Internal.sdk"; + auto add_internal_sdk_suffix = [](llvm::StringRef sdk) { + return (sdk.substr(0, sdk.size() - 3) + "Internal.sdk").str(); }; // The "bigger" SDK always wins. @@ -172,8 +170,8 @@ void XcodeSDK::Merge(const XcodeSDK &other) { if (llvm::StringRef(m_name).ends_with(".sdk")) m_name = add_internal_sdk_suffix(m_name); - if (m_sysroot_path && llvm::StringRef(*m_sysroot_path).ends_with(".sdk")) - m_sysroot_path.emplace(add_internal_sdk_suffix(*m_sysroot_path)); + if (m_sysroot.GetFileNameExtension() == ".sdk") + m_sysroot.SetFilename(add_internal_sdk_suffix(m_sysroot.GetFilename())); } } } diff --git a/lldb/unittests/Utility/XcodeSDKTest.cpp b/lldb/unittests/Utility/XcodeSDKTest.cpp index 95307ade4d43a..bca7f1291c425 100644 --- a/lldb/unittests/Utility/XcodeSDKTest.cpp +++ b/lldb/unittests/Utility/XcodeSDKTest.cpp @@ -34,16 +34,17 @@ TEST(XcodeSDKTest, ParseTest) { EXPECT_EQ(XcodeSDK("MacOSX10.9.sdk").GetVersion(), llvm::VersionTuple(10, 9)); EXPECT_EQ(XcodeSDK("MacOSX10.15.4.sdk").GetVersion(), llvm::VersionTuple(10, 15)); EXPECT_EQ(XcodeSDK("MacOSX.sdk").IsAppleInternalSDK(), false); - EXPECT_EQ(XcodeSDK("MacOSX.sdk", "/Path/To/MacOSX.sdk").GetSysroot(), - "/Path/To/MacOSX.sdk"); + EXPECT_EQ( + XcodeSDK("MacOSX.sdk", FileSpec{"/Path/To/MacOSX.sdk"}).GetSysroot(), + FileSpec("/Path/To/MacOSX.sdk")); EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetType(), XcodeSDK::MacOSX); EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetVersion(), llvm::VersionTuple(10, 15)); EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").IsAppleInternalSDK(), true); - EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetSysroot(), std::nullopt); + EXPECT_FALSE(XcodeSDK("MacOSX10.15.Internal.sdk").GetSysroot()); EXPECT_EQ(XcodeSDK().GetType(), XcodeSDK::unknown); EXPECT_EQ(XcodeSDK().GetVersion(), llvm::VersionTuple()); - EXPECT_EQ(XcodeSDK().GetSysroot(), std::nullopt); + EXPECT_FALSE(XcodeSDK().GetSysroot()); } TEST(XcodeSDKTest, MergeTest) { @@ -64,14 +65,14 @@ TEST(XcodeSDKTest, MergeTest) { XcodeSDK empty; empty.Merge(XcodeSDK("MacOSX10.14.Internal.sdk")); EXPECT_EQ(empty.GetString(), llvm::StringRef("MacOSX10.14.Internal.sdk")); - EXPECT_EQ(empty.GetSysroot(), std::nullopt); - empty.Merge(XcodeSDK("MacOSX9.5.Internal.sdk", "/Path/To/9.5.sdk")); - EXPECT_EQ(empty.GetSysroot(), std::nullopt); - empty.Merge(XcodeSDK("MacOSX12.5.sdk", "/Path/To/12.5.sdk")); - EXPECT_EQ(empty.GetSysroot(), "/Path/To/12.5.sdk"); - empty.Merge( - XcodeSDK("MacOSX11.5.Internal.sdk", "/Path/To/12.5.Internal.sdk")); - EXPECT_EQ(empty.GetSysroot(), "/Path/To/12.5.Internal.sdk"); + EXPECT_FALSE(empty.GetSysroot()); + empty.Merge(XcodeSDK("MacOSX9.5.Internal.sdk", FileSpec{"/Path/To/9.5.sdk"})); + EXPECT_FALSE(empty.GetSysroot()); + empty.Merge(XcodeSDK("MacOSX12.5.sdk", FileSpec{"/Path/To/12.5.sdk"})); + EXPECT_EQ(empty.GetSysroot(), FileSpec{"/Path/To/12.5.sdk"}); + empty.Merge(XcodeSDK("MacOSX11.5.Internal.sdk", + FileSpec{"/Path/To/12.5.Internal.sdk"})); + EXPECT_EQ(empty.GetSysroot(), FileSpec{"/Path/To/12.5.Internal.sdk"}); } #ifndef _WIN32 From 18bc94ac7b92801a214cf957ce0abb8f7c2ca76d Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 3 Mar 2025 13:29:28 +0000 Subject: [PATCH 7/8] fixup! remove redundant local variable --- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 0bdd1b37d5bd0..b777914407c6c 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1395,7 +1395,6 @@ PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) { llvm::formatv("No symbol file available for module '{0}'", module.GetFileSpec().GetFilename().AsCString(""))); - std::string sysroot; bool found_public_sdk = false; bool found_internal_sdk = false; XcodeSDK merged_sdk; From b23e6310b5c1fa65f97391915ed39b8a0e6c54dd Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 3 Mar 2025 13:32:58 +0000 Subject: [PATCH 8/8] fixup! fix test expectation --- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp index c1e471653a279..fe6755d80c3aa 100644 --- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp +++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp @@ -330,7 +330,7 @@ SDKPathParsingTestData sdkPathParsingTestCases[] = { {"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.1.sdk", "/Path/To/SDKs/MacOSX11.3.Internal.sdk"}, .expect_mismatch = false, - .expect_internal_sdk = false, + .expect_internal_sdk = true, .expect_sdk_path_pattern = "iPhoneOS14.1.Internal.sdk"}, };