[lldb] Fix progress reporting for SymbolLocatorDebugSymbols

[JDevlieghere]

This fixes two issues related to the DebugSymbols symbol locator:

1. Only the default symbol locator plugin reports progress. On Darwin, which uses the DebugSymbols framework we need to report the same progress form the corresponding SymbolLocator plugin.

2. Forceful dSYM lookups, for example when using add-dsym, use a different code path that currently does not report progress, which is confusing. Here the progress event can be more specific and specify its downloading a symbol file rather than just locating it as we'll always shell out to dsymForUUID or its equivalent.

rdar://121629777

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

This fixes two issues related to the DebugSymbols symbol locator:

1. Only the default symbol locator plugin reports progress. On Darwin, which uses the DebugSymbols framework we need to report the same progress form the corresponding SymbolLocator plugin.

2. Forceful dSYM lookups, for example when using add-dsym, use a different code path that currently does not report progress, which is confusing. Here the progress event can be more specific and specify its downloading a symbol file rather than just locating it as we'll always shell out to dsymForUUID or its equivalent.

rdar://121629777

---

Full diff: https://github.com/llvm/llvm-project/pull/79624.diff

1 Files Affected:

• (modified) lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp (+20-19)

diff --git a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index 9f32d252b22f5bd..24e563d6ee0f353 100644
--- a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++ b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -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",
+      module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>"));
+
   FileSpec symbol_fspec;
   ModuleSpec dsym_module_spec;
   // First try and find the dSYM in the same directory as the executable or in
@@ -1050,28 +1054,25 @@ bool SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   const std::string file_path_str =
       file_spec_ptr ? file_spec_ptr->GetPath() : "";
 
-  Log *log = GetLog(LLDBLog::Host);
+  if (uuid_str.empty() && file_path_str.empty())
+    return false;
 
   // Create the dsymForUUID command.
-  StreamString command;
+  const char *lookup_arg =
+      !uuid_str.empty() ? uuid_str.c_str() : file_path_str.c_str();
   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;
-  }
+
+  StreamString command;
+  command.Printf("%s --ignoreNegativeCache %s%s", dsymForUUID_exe_path.c_str(),
+                 copy_executable_arg, lookup_arg);
+
+  // Log and report progress.
+  Log *log = GetLog(LLDBLog::Host);
+  LLDB_LOGF(log, "Calling %s with %s to find dSYM: %s",
+            dsymForUUID_exe_path.c_str(), lookup_arg,
+            command.GetString().data());
+
+  Progress progress("Downloading symbol file", lookup_arg);
 
   // Invoke dsymForUUID.
   int exit_status = -1;

[chelcassanova]

LGTM, thanks for adding new reports!

[clayborg]

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

[JDevlieghere]

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.

[adrian-prantl]

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

[JDevlieghere]

#79649

[adrian-prantl]

more unnecessary comments about unnecessary c string conversions :-)

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