https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/146062
>From b7700480aa7150237f73aa37b0cf39f592ac512d Mon Sep 17 00:00:00 2001 From: Charles Zablit <c_zab...@apple.com> Date: Fri, 27 Jun 2025 12:49:53 +0100 Subject: [PATCH 1/7] [lldb][NFC] Inline ResolveSDKPathFromDebugInfo in one of its call site --- .../Platform/MacOSX/PlatformDarwin.cpp | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 262a7dc731713..ae46ac63e756a 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1130,13 +1130,33 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType( if (target) { if (ModuleSP exe_module_sp = target->GetExecutableModule()) { - auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp); - if (path_or_err) { - sysroot_spec = FileSpec(*path_or_err); + SymbolFile *sym_file = exe_module_sp->GetSymbolFile(); + if (!sym_file) + return; + + XcodeSDK merged_sdk; + for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) { + if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) { + auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp); + merged_sdk.Merge(cu_sdk); + } + } + + // TODO: The result of this loop is almost equivalent to deriving the SDK + // from the target triple, which would be a lot cheaper. + + if (FileSystem::Instance().Exists(merged_sdk.GetSysroot())) { + sysroot_spec = merged_sdk.GetSysroot(); } else { - LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host), - path_or_err.takeError(), - "Failed to resolve SDK path: {0}"); + auto path_or_err = + HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk}); + if (path_or_err) { + sysroot_spec = FileSpec(*path_or_err); + } else { + LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host), + path_or_err.takeError(), + "Failed to resolve SDK path: {0}"); + } } } } >From ae9afb7b786087716fab975dfce2d3b8158f653d Mon Sep 17 00:00:00 2001 From: Charles Zablit <c_zab...@apple.com> Date: Mon, 30 Jun 2025 12:22:21 +0100 Subject: [PATCH 2/7] extract to helper method --- .../Platform/MacOSX/PlatformDarwin.cpp | 70 +++++++++++-------- .../Plugins/Platform/MacOSX/PlatformDarwin.h | 3 + 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index ae46ac63e756a..4411c88c97ab2 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1128,37 +1128,8 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType( FileSpec sysroot_spec; - if (target) { - if (ModuleSP exe_module_sp = target->GetExecutableModule()) { - SymbolFile *sym_file = exe_module_sp->GetSymbolFile(); - if (!sym_file) - return; - - XcodeSDK merged_sdk; - for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) { - if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) { - auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp); - merged_sdk.Merge(cu_sdk); - } - } - - // TODO: The result of this loop is almost equivalent to deriving the SDK - // from the target triple, which would be a lot cheaper. - - if (FileSystem::Instance().Exists(merged_sdk.GetSysroot())) { - sysroot_spec = merged_sdk.GetSysroot(); - } else { - auto path_or_err = - HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk}); - if (path_or_err) { - sysroot_spec = FileSpec(*path_or_err); - } else { - LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host), - path_or_err.takeError(), - "Failed to resolve SDK path: {0}"); - } - } - } + if (target && ResolveSDKPathFromDebugInfo(target, sysroot_spec)) { + return; } if (!FileSystem::Instance().IsDirectory(sysroot_spec.GetPath())) { @@ -1172,6 +1143,43 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType( } } +bool lldb_private::PlatformDarwin::ResolveSDKPathFromDebugInfo( + lldb_private::Target *target, lldb_private::FileSpec &sysroot_spec) { + + ModuleSP exe_module_sp = target->GetExecutableModule(); + if (!exe_module_sp) + return false; + + SymbolFile *sym_file = exe_module_sp->GetSymbolFile(); + if (!sym_file) + return true; + + XcodeSDK merged_sdk; + for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) { + if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) { + auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp); + merged_sdk.Merge(cu_sdk); + } + } + + // TODO: The result of this loop is almost equivalent to deriving the SDK + // from the target triple, which would be a lot cheaper. + FileSpec sdk_path = merged_sdk.GetSysroot(); + if (FileSystem::Instance().Exists(sdk_path)) { + sysroot_spec = sdk_path; + } else { + auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk}); + if (path_or_err) { + sysroot_spec = FileSpec(*path_or_err); + } else { + LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host), + path_or_err.takeError(), + "Failed to resolve SDK path: {0}"); + } + } + return false; +} + ConstString PlatformDarwin::GetFullNameForDylib(ConstString basename) { if (basename.IsEmpty()) return basename; diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h index f8a62ceb958fe..2c543a21fc1a9 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h @@ -187,6 +187,9 @@ class PlatformDarwin : public PlatformPOSIX { std::vector<std::string> &options, XcodeSDK::Type sdk_type); + static bool ResolveSDKPathFromDebugInfo(lldb_private::Target *target, + lldb_private::FileSpec &sysroot_spec); + Status FindBundleBinaryInExecSearchPaths( const ModuleSpec &module_spec, Process *process, lldb::ModuleSP &module_sp, const FileSpecList *module_search_paths_ptr, >From 153c15856a88df5a2c598a2f9f498ebf7498ae8b Mon Sep 17 00:00:00 2001 From: Charles Zablit <c_zab...@apple.com> Date: Mon, 30 Jun 2025 15:23:48 +0100 Subject: [PATCH 3/7] use llvm::Expected in helper function --- .../Platform/MacOSX/PlatformDarwin.cpp | 44 ++++++++++++------- .../Plugins/Platform/MacOSX/PlatformDarwin.h | 4 +- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 4411c88c97ab2..8ec0d30fbcd20 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1128,8 +1128,15 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType( FileSpec sysroot_spec; - if (target && ResolveSDKPathFromDebugInfo(target, sysroot_spec)) { - return; + if (target) { + auto sysroot_spec_or_err = ResolveSDKPathFromDebugInfo(target); + if (!sysroot_spec_or_err) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host), + sysroot_spec_or_err.takeError(), + "Failed to resolve sysroot: {0}"); + } else { + sysroot_spec = *sysroot_spec_or_err; + } } if (!FileSystem::Instance().IsDirectory(sysroot_spec.GetPath())) { @@ -1143,16 +1150,21 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType( } } -bool lldb_private::PlatformDarwin::ResolveSDKPathFromDebugInfo( - lldb_private::Target *target, lldb_private::FileSpec &sysroot_spec) { +llvm::Expected<lldb_private::FileSpec> +lldb_private::PlatformDarwin::ResolveSDKPathFromDebugInfo( + lldb_private::Target *target) { ModuleSP exe_module_sp = target->GetExecutableModule(); if (!exe_module_sp) - return false; + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + llvm::formatv("Failed to get module from target")); SymbolFile *sym_file = exe_module_sp->GetSymbolFile(); if (!sym_file) - return true; + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + llvm::formatv("Failed to get symbol file from module")); XcodeSDK merged_sdk; for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) { @@ -1166,18 +1178,16 @@ bool lldb_private::PlatformDarwin::ResolveSDKPathFromDebugInfo( // from the target triple, which would be a lot cheaper. FileSpec sdk_path = merged_sdk.GetSysroot(); if (FileSystem::Instance().Exists(sdk_path)) { - sysroot_spec = sdk_path; - } else { - auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk}); - if (path_or_err) { - sysroot_spec = FileSpec(*path_or_err); - } else { - LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host), - path_or_err.takeError(), - "Failed to resolve SDK path: {0}"); - } + return sdk_path; } - return false; + auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk}); + if (path_or_err) + return FileSpec(*path_or_err); + + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + llvm::formatv("Failed to resolve SDK path: {0}", + llvm::toString(path_or_err.takeError()))); } ConstString PlatformDarwin::GetFullNameForDylib(ConstString basename) { diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h index 2c543a21fc1a9..e697cf2ef7cf6 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h @@ -187,8 +187,8 @@ class PlatformDarwin : public PlatformPOSIX { std::vector<std::string> &options, XcodeSDK::Type sdk_type); - static bool ResolveSDKPathFromDebugInfo(lldb_private::Target *target, - lldb_private::FileSpec &sysroot_spec); + static llvm::Expected<FileSpec> + ResolveSDKPathFromDebugInfo(lldb_private::Target *target); Status FindBundleBinaryInExecSearchPaths( const ModuleSpec &module_spec, Process *process, >From c5dafab2ba6c315fc4d8fa6c037586f358f547f7 Mon Sep 17 00:00:00 2001 From: Charles Zablit <zablitchar...@gmail.com> Date: Mon, 30 Jun 2025 17:28:18 +0100 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Michael Buch <michaelbuc...@gmail.com> --- .../source/Plugins/Platform/MacOSX/PlatformDarwin.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 8ec0d30fbcd20..ea4ab5ede9269 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1156,15 +1156,11 @@ lldb_private::PlatformDarwin::ResolveSDKPathFromDebugInfo( ModuleSP exe_module_sp = target->GetExecutableModule(); if (!exe_module_sp) - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - llvm::formatv("Failed to get module from target")); + return llvm::createStringError("Failed to get module from target"); SymbolFile *sym_file = exe_module_sp->GetSymbolFile(); if (!sym_file) - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - llvm::formatv("Failed to get symbol file from module")); + return llvm::createStringError("Failed to get symbol file from module"); XcodeSDK merged_sdk; for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) { @@ -1185,9 +1181,8 @@ lldb_private::PlatformDarwin::ResolveSDKPathFromDebugInfo( return FileSpec(*path_or_err); return llvm::createStringError( - llvm::inconvertibleErrorCode(), llvm::formatv("Failed to resolve SDK path: {0}", - llvm::toString(path_or_err.takeError()))); + llvm::toString(path_or_err.takeError())); } ConstString PlatformDarwin::GetFullNameForDylib(ConstString basename) { >From ad7ff2b748812e71e08c788da0fe6a4f441a6a6b Mon Sep 17 00:00:00 2001 From: Charles Zablit <c_zab...@apple.com> Date: Mon, 30 Jun 2025 17:31:38 +0100 Subject: [PATCH 5/7] make function local to the file --- .../Platform/MacOSX/PlatformDarwin.cpp | 69 +++++++++---------- .../Plugins/Platform/MacOSX/PlatformDarwin.h | 3 - 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index ea4ab5ede9269..e156c7e53a4ed 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1030,6 +1030,40 @@ PlatformDarwin::ExtractAppSpecificInfo(Process &process) { return dict_sp; } +static llvm::Expected<lldb_private::FileSpec> +ResolveSDKPathFromDebugInfo(lldb_private::Target *target) { + + ModuleSP exe_module_sp = target->GetExecutableModule(); + if (!exe_module_sp) + return llvm::createStringError("Failed to get module from target"); + + SymbolFile *sym_file = exe_module_sp->GetSymbolFile(); + if (!sym_file) + return llvm::createStringError("Failed to get symbol file from module"); + + XcodeSDK merged_sdk; + for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) { + if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) { + auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp); + merged_sdk.Merge(cu_sdk); + } + } + + // TODO: The result of this loop is almost equivalent to deriving the SDK + // from the target triple, which would be a lot cheaper. + FileSpec sdk_path = merged_sdk.GetSysroot(); + if (FileSystem::Instance().Exists(sdk_path)) { + return sdk_path; + } + auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk}); + if (!path_or_err) + return llvm::createStringError( + llvm::formatv("Failed to resolve SDK path: {0}", + llvm::toString(path_or_err.takeError()))); + + return FileSpec(*path_or_err); +} + void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType( Target *target, std::vector<std::string> &options, XcodeSDK::Type sdk_type) { const std::vector<std::string> apple_arguments = { @@ -1150,41 +1184,6 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType( } } -llvm::Expected<lldb_private::FileSpec> -lldb_private::PlatformDarwin::ResolveSDKPathFromDebugInfo( - lldb_private::Target *target) { - - ModuleSP exe_module_sp = target->GetExecutableModule(); - if (!exe_module_sp) - return llvm::createStringError("Failed to get module from target"); - - SymbolFile *sym_file = exe_module_sp->GetSymbolFile(); - if (!sym_file) - return llvm::createStringError("Failed to get symbol file from module"); - - XcodeSDK merged_sdk; - for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) { - if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) { - auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp); - merged_sdk.Merge(cu_sdk); - } - } - - // TODO: The result of this loop is almost equivalent to deriving the SDK - // from the target triple, which would be a lot cheaper. - FileSpec sdk_path = merged_sdk.GetSysroot(); - if (FileSystem::Instance().Exists(sdk_path)) { - return sdk_path; - } - auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk}); - if (path_or_err) - return FileSpec(*path_or_err); - - return llvm::createStringError( - llvm::formatv("Failed to resolve SDK path: {0}", - llvm::toString(path_or_err.takeError())); -} - ConstString PlatformDarwin::GetFullNameForDylib(ConstString basename) { if (basename.IsEmpty()) return basename; diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h index e697cf2ef7cf6..f8a62ceb958fe 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h @@ -187,9 +187,6 @@ class PlatformDarwin : public PlatformPOSIX { std::vector<std::string> &options, XcodeSDK::Type sdk_type); - static llvm::Expected<FileSpec> - ResolveSDKPathFromDebugInfo(lldb_private::Target *target); - Status FindBundleBinaryInExecSearchPaths( const ModuleSpec &module_spec, Process *process, lldb::ModuleSP &module_sp, const FileSpecList *module_search_paths_ptr, >From a174359dfe9c36c541d97c42c39f1261e927847e Mon Sep 17 00:00:00 2001 From: Charles Zablit <c_zab...@apple.com> Date: Mon, 30 Jun 2025 18:06:16 +0100 Subject: [PATCH 6/7] trigger CI >From 08713014832461038ba63580e131f713a328fa68 Mon Sep 17 00:00:00 2001 From: Charles Zablit <c_zab...@apple.com> Date: Tue, 1 Jul 2025 14:40:26 +0100 Subject: [PATCH 7/7] fix build issue --- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index e156c7e53a4ed..1db7bc78013d7 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1163,7 +1163,7 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType( FileSpec sysroot_spec; if (target) { - auto sysroot_spec_or_err = ResolveSDKPathFromDebugInfo(target); + auto sysroot_spec_or_err = ::ResolveSDKPathFromDebugInfo(target); if (!sysroot_spec_or_err) { LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host), sysroot_spec_or_err.takeError(), _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits