[Lldb-commits] [lldb] [lldb] Fix progress reporting for SymbolLocatorDebugSymbols (PR #79624)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/79624

>From 7979269a57fc553a7b010a36b9c75bf570adf674 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 26 Jan 2024 09:34:11 -0800
Subject: [PATCH 1/2] [lldb] Fix progress reporting for
 SymbolLocatorDebugSymbols

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
---
 .../SymbolLocatorDebugSymbols.cpp             | 39 ++++++++++---------
 1 file changed, 20 insertions(+), 19 deletions(-)

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;

>From 69015699499e23de26d61bd0faa16fcbc75bba49 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 26 Jan 2024 12:57:24 -0800
Subject: [PATCH 2/2] Avoid C-String Conversions

---
 .../SymbolLocatorDebugSymbols.cpp             | 24 +++++++++----------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index 24e563d6ee0f353..f7df4650941a800 100644
--- a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++ b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -1049,28 +1049,26 @@ 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() : "";
 
-  if (uuid_str.empty() && file_path_str.empty())
+  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.
-  const char *lookup_arg =
-      !uuid_str.empty() ? uuid_str.c_str() : file_path_str.c_str();
-  const char *copy_executable_arg = copy_executable ? "--copyExecutable " : "";
-
   StreamString command;
-  command.Printf("%s --ignoreNegativeCache %s%s", dsymForUUID_exe_path.c_str(),
-                 copy_executable_arg, lookup_arg);
+  command << dsymForUUID_exe_path << " --ignoreNegativeCache ";
+  if (copy_executable)
+    command << "--copyExecutable ";
+  command << 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());
+  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);