[Lldb-commits] [lldb] [lldb] Address post-commit feedback on DynamicLoader binary loading (NFC) (PR #214635)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/214635

Follow-up to b2ba87ae2551, addressing Jim's post-commit feedback:

- Documentation clarifications to the DynamicLoader batch-loading API.
- Prefixing symbol-search errors with the binary's description.

>From a4fffa8d869901d05d4709341809913fa652949d Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 6 Aug 2026 21:51:02 -0700
Subject: [PATCH] [lldb] Address post-commit feedback on DynamicLoader binary
 loading (NFC)

Follow-up to b2ba87ae2551, addressing Jim's post-commit feedback:

- Documentation clarifications to the DynamicLoader batch-loading API.
- Prefixing symbol-search errors with the binary's description.
---
 lldb/include/lldb/Target/DynamicLoader.h | 28 ++++++++++++++----------
 lldb/source/Core/DynamicLoader.cpp       |  4 +++-
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h
index 62307a9f9c2a2..1a85066b0b25c 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -216,12 +216,11 @@ class DynamicLoader : public PluginInterface {
                                              lldb::addr_t base_addr,
                                              bool base_addr_is_offset);
 
-  /// A binary to find and load into a Target, and the state that finding it
-  /// produces.
+  /// A binary to find and load into a Target.
   ///
-  /// Loading a binary is split into LocateBinaries and LoadBinaryInTarget so
-  /// that a caller with many binaries can search for all of them before adding
-  /// any of them to the Target.
+  /// The leading fields are inputs, filled in by the caller.  The trailing
+  /// fields are outputs: searching for the binary records its results in them,
+  /// and loading the binary into the Target consumes them.
   struct BinarySpec {
     /// Name of the binary, if available.  If no matching binary can be found on
     /// the debug host, a module may be created out of live memory and given
@@ -286,8 +285,11 @@ class DynamicLoader : public PluginInterface {
   /// Given an address, try to read the binary out of memory, get the UUID, find
   /// the file if possible and load it unslid, or add the memory module.
   ///
-  /// May force an expensive search on the computer to find the binary by UUID.
-  /// To load more than one binary, use LocateBinaries and LoadBinaryInTarget.
+  /// May force an expensive search on the host system to find the binary by
+  /// UUID.  To load more than one binary, use LocateBinaries and
+  /// LoadBinaryInTarget instead: the search is the expensive part, and those
+  /// let all of the searching happen before any of the binaries are added to
+  /// the Target.
   ///
   /// \param[in] process
   ///     The process to add this binary to.
@@ -304,7 +306,8 @@ class DynamicLoader : public PluginInterface {
 
   /// Search for a batch of binaries, without mutating the Target.
   ///
-  /// The entries are independent: a binary that cannot be found leaves its
+  /// The entries are searched for independently, and stay in the order they
+  /// were given in.  A binary that cannot be found leaves its
   /// BinarySpec::module_sp empty and has no effect on the others.  Its
   /// BinarySpec::memory_module_sp is set only when the binary's header had to
   /// be read out of memory to get the UUID.  Nothing is registered with the
@@ -324,10 +327,11 @@ class DynamicLoader : public PluginInterface {
   /// load address.
   ///
   /// This mutates the Target and may read the process' memory, so it has to be
-  /// called for one binary at a time.  Call it in the caller's own order rather
-  /// than in the order the searches finished: the order decides the Target's
-  /// module order, which binary gets to set the Target's architecture, and the
-  /// order in which messages reach the user.
+  /// called for one binary at a time.  Call it over the batch in the order the
+  /// batch was built in: that order decides the Target's module order, which
+  /// binary gets to set the Target's architecture, and the order in which
+  /// messages reach the user.  LocateBinaries preserves it, whatever order the
+  /// searches themselves happen to run or finish in.
   ///
   /// Whether a failure is worth telling the user about is left to the caller,
   /// which knows whether it went looking for a binary that has to be there.  A
diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
index 96da588a0d644..1f85b655ba116 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -354,9 +354,11 @@ DynamicLoader::LoadBinaryInTarget(Process *process, BinarySpec &bin_spec) {
   }
 
   // A binary was found, but a symbol server may still have had something to say
-  // about its symbols.
+  // about its symbols.  Name the binary: a symbol locator's error is not
+  // required to identify what it was asked to look for.
   if (search_error)
     *target.GetDebugger().GetAsyncErrorStream()
+        << GetBinaryDescription(bin_spec) << ": "
         << llvm::toString(std::move(search_error)) << "\n";
 
   // Ensure the Target has an architecture set in case