Skip to content

[lldb][NFC] Inline ResolveSDKPathFromDebugInfo in one of its call site #146062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+50 −9
Open

[lldb][NFC] Inline ResolveSDKPathFromDebugInfo in one of its call site #146062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ce7517
[lldb][NFC] Inline ResolveSDKPathFromDebugInfo in one of its call site
charles-zablit Jun 27, 2025
aa2417a
extract to helper method
charles-zablit Jun 30, 2025
9673fe9
use llvm::Expected in helper function
charles-zablit Jun 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 47 additions & 9 deletions lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1129,15 +1129,13 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
FileSpec sysroot_spec;

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);
} else {
LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host),
path_or_err.takeError(),
"Failed to resolve SDK path: {0}");
}
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;
}
}

Expand All @@ -1152,6 +1150,46 @@ 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(
llvm::inconvertibleErrorCode(),
llvm::formatv("Failed to get module from target"));
Comment on lines +1159 to +1161
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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"));
Comment on lines +1165 to +1167
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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) {
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: I would invert this condition and return error if !path_or_err.

Then return FileSpec(*path_or_err) as the last return from this function. So we're consistent with all the other early returns in this function

return FileSpec(*path_or_err);

return llvm::createStringError(
llvm::inconvertibleErrorCode(),
llvm::formatv("Failed to resolve SDK path: {0}",
llvm::toString(path_or_err.takeError())));
Comment on lines +1187 to +1190
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
llvm::formatv("Failed to resolve SDK path: {0}",
llvm::toString(path_or_err.takeError())));
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;
Expand Down
3 changes: 3 additions & 0 deletions lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ class PlatformDarwin : public PlatformPOSIX {
std::vector<std::string> &options,
XcodeSDK::Type sdk_type);

static llvm::Expected<FileSpec>
ResolveSDKPathFromDebugInfo(lldb_private::Target *target);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this to be in the header. We can just make it local to the .cpp file


Status FindBundleBinaryInExecSearchPaths(
const ModuleSpec &module_spec, Process *process,
lldb::ModuleSP &module_sp, const FileSpecList *module_search_paths_ptr,
Expand Down
Loading