[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

[Jonas Devlieghere via lldb-commits]

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

>From b50eb7442ab8f37659596efad19f8b30b0000145 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 14 Mar 2024 17:43:02 -0700
Subject: [PATCH 1/2] [lldb] Show module name in progress update for
 downloading symbols

Currently, we always show the argument passed to dsymForUUID in the
corresponding progress update. Most of the time this is a UUID, but it
can also be an absolute path. The former is pretty uninformative and the
latter needlessly noisy.

This changes the progress update to print the UUID and the module name,
if both are available. Otherwise, we print the UUID or the module name
depending on which one is available.

We now also unconditionally pass the module file spec and architecture
to DownloadObjectAndSymbolFile, while previously this was conditional on
the file existing on-disk. This should be harmless:

  - We already check that the file exists in DownloadObjectAndSymbolFile.
  - It doesn't make sense to check the filesystem for the architecutre.

rdar://124643548
---
 lldb/source/Commands/CommandObjectTarget.cpp    | 17 +++++------------
 .../DebugSymbols/SymbolLocatorDebugSymbols.cpp  | 14 ++++++++++++--
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index b2346c2402a81d..ae6c6d5479a198 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3377,7 +3377,7 @@ class CommandObjectTargetModulesList : public CommandObjectParsed {
       case 'r': {
         size_t ref_count = 0;
         char in_shared_cache = 'Y';
-        
+
         ModuleSP module_sp(module->shared_from_this());
         if (!ModuleList::ModuleIsInCache(module))
           in_shared_cache = 'N';
@@ -4508,11 +4508,8 @@ class CommandObjectTargetSymbolsAdd : public CommandObjectParsed {
 
     ModuleSpec module_spec;
     module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-    if (FileSystem::Instance().Exists(frame_module_sp->GetPlatformFileSpec())) {
-      module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-      module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-    }
+    module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
+    module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
 
     if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
       result.AppendError("unable to find debug symbols for the current frame");
@@ -4557,12 +4554,8 @@ class CommandObjectTargetSymbolsAdd : public CommandObjectParsed {
 
       ModuleSpec module_spec;
       module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-      if (FileSystem::Instance().Exists(
-              frame_module_sp->GetPlatformFileSpec())) {
-        module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-        module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-      }
+      module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
+      module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
 
       bool current_frame_flush = false;
       if (DownloadObjectAndSymbolFile(module_spec, result, current_frame_flush))
diff --git a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index f7df4650941a80..acdf962a447266 100644
--- a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++ b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -1066,11 +1066,21 @@ bool SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+    lookup_desc =
+        llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+                      uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+    lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+    lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-           lookup_arg, command.GetString());
+           lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_arg);
+  Progress progress("Downloading symbol file", lookup_desc);
 
   // Invoke dsymForUUID.
   int exit_status = -1;

>From b23b1eedbf39f4d963927cbbb99a69ecb308f449 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 15 Mar 2024 12:14:53 -0700
Subject: [PATCH 2/2] Change wording

---
 .../SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index acdf962a447266..f894897c43f8b4 100644
--- a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++ b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -1080,7 +1080,7 @@ bool SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
            lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_desc);
+  Progress progress("Downloading symbol file for", lookup_desc);
 
   // Invoke dsymForUUID.
   int exit_status = -1;