Skip to content

[lldb] Fix progress reporting for SymbolLocatorDebugSymbols #79624

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

Merged
merged 2 commits into from
Jan 26, 2024
Merged

[lldb] Fix progress reporting for SymbolLocatorDebugSymbols #79624

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7979269
[lldb] Fix progress reporting for SymbolLocatorDebugSymbols
JDevlieghere Jan 26, 2024
6901569
Avoid C-String Conversions
JDevlieghere Jan 26, 2024
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
43 changes: 21 additions & 22 deletions lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,10 @@ std::optional<FileSpec> SymbolLocatorDebugSymbols::LocateExecutableSymbolFile(
exec_fspec ? exec_fspec->GetFilename().AsCString("<NULL>") : "<NULL>",
arch ? arch->GetArchitectureName() : "<NULL>", (const void *)uuid);

Progress progress(
"Locating external symbol file",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we looking for an executable here, or the dSYM file? Should be title be "Locating executable file"?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're looking for the symbol file (although dsymForUUID can also get you the symbol rich binary). This is the same message already emitted by SymbolLocatorDefault::LocateExecutableSymbolFile.

module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aside: Looks like we're missing a AsString() method in ConstString so we can avoid the roundtrip through char *

Copy link
Member Author

Choose a reason for hiding this comment

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

#79649


FileSpec symbol_fspec;
ModuleSpec dsym_module_spec;
// First try and find the dSYM in the same directory as the executable or in
Expand Down Expand Up @@ -1045,33 +1049,28 @@ bool SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
if (!dsymForUUID_exe_spec)
return false;

// Create the dsymForUUID command.
const std::string dsymForUUID_exe_path = dsymForUUID_exe_spec.GetPath();
const std::string uuid_str = uuid_ptr ? uuid_ptr->GetAsString() : "";
const std::string file_path_str =
file_spec_ptr ? file_spec_ptr->GetPath() : "";

Log *log = GetLog(LLDBLog::Host);
std::string lookup_arg = uuid_str;
if (lookup_arg.empty())
lookup_arg = file_spec_ptr ? file_spec_ptr->GetPath() : "";
if (lookup_arg.empty())
return false;

// Create the dsymForUUID command.
StreamString command;
Copy link
Collaborator

Choose a reason for hiding this comment

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

more unnecessary comments about unnecessary c string conversions :-)

    llvm::SmallString<64> buf;
    llvm::raw_svector_ostream(buf) << ...;

const char *copy_executable_arg = copy_executable ? "--copyExecutable " : "";
if (!uuid_str.empty()) {
command.Printf("%s --ignoreNegativeCache %s%s",
dsymForUUID_exe_path.c_str(), copy_executable_arg,
uuid_str.c_str());
LLDB_LOGF(log, "Calling %s with UUID %s to find dSYM: %s",
dsymForUUID_exe_path.c_str(), uuid_str.c_str(),
command.GetString().data());
} else if (!file_path_str.empty()) {
command.Printf("%s --ignoreNegativeCache %s%s",
dsymForUUID_exe_path.c_str(), copy_executable_arg,
file_path_str.c_str());
LLDB_LOGF(log, "Calling %s with file %s to find dSYM: %s",
dsymForUUID_exe_path.c_str(), file_path_str.c_str(),
command.GetString().data());
} else {
return false;
}
command << dsymForUUID_exe_path << " --ignoreNegativeCache ";
if (copy_executable)
command << "--copyExecutable ";
command << lookup_arg;

// Log and report progress.
Log *log = GetLog(LLDBLog::Host);
LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
lookup_arg, command.GetString());

Progress progress("Downloading symbol file", lookup_arg);

// Invoke dsymForUUID.
int exit_status = -1;
Expand Down