llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/214635.diff 2 Files Affected: - (modified) lldb/include/lldb/Target/DynamicLoader.h (+16-12) - (modified) lldb/source/Core/DynamicLoader.cpp (+3-1) ``````````diff 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 `````````` </details> https://github.com/llvm/llvm-project/pull/214635 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
