https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/146062
>From 6ce75177264d47ffedbf0e6ee44831eac1d3ea55 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/3] [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 aa2417acc66af5612abb33bf39edc9d2b41478a7 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/3] 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 9673fe93a86f5d6118b07354e96072e318dac750 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/3] 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, _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits