jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

I handle two types of binary specification in qProcessInfo -- the 
`main-binary-{uuid,address,slide}` and `binary-addresses` -- so the remote stub 
can tell lldb to discover/load binaries when we connect.

I did this loading in `ProcessGDBRemote::DoConnectRemote()` which is very early 
in process setup, e.g. we don't have threads yet.  This caused problems for one 
group that has a python script in the dSYM and need a more fully set up process.

This patch moves this binary loading from `DoConnectRemote()` to 
`DidLaunchOrAttach()` where we're nearly done setting up the Process.  There's 
a method, `MaybeLoadExecutableModule` which reads the load address of the main 
binary from the remote stub (maybge on linux or something) and sets the load 
address in the Target; I added my new `LoadStubBinaries()` method next to that 
one in `DidLaunchOrAttach()`. I originally thought to simply add the code to 
`MaybeLoadExecutableModule` but then the method was doing two completely 
unrelated things and it would only serve to confuse people in the future I 
think.

I don't know of a particularly good reviewer on this one, but I'll add Jim 
without thinking of a better choice offhand.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141972

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -374,6 +374,7 @@
   bool UpdateThreadIDList();
 
   void DidLaunchOrAttach(ArchSpec &process_arch);
+  void LoadStubBinaries();
   void MaybeLoadExecutableModule();
 
   Status ConnectToDebugserver(llvm::StringRef host_port);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -555,58 +555,6 @@
         }
       }
 
-      // The remote stub may know about the "main binary" in
-      // the context of a firmware debug session, and can
-      // give us a UUID and an address/slide of where the
-      // binary is loaded in memory.
-      UUID standalone_uuid;
-      addr_t standalone_value;
-      bool standalone_value_is_offset;
-      if (m_gdb_comm.GetProcessStandaloneBinary(
-              standalone_uuid, standalone_value, standalone_value_is_offset)) {
-        ModuleSP module_sp;
-
-        if (standalone_uuid.IsValid()) {
-          const bool force_symbol_search = true;
-          const bool notify = true;
-          DynamicLoader::LoadBinaryWithUUIDAndAddress(
-              this, llvm::StringRef(), standalone_uuid, standalone_value,
-              standalone_value_is_offset, force_symbol_search, notify);
-        }
-      }
-
-      // The remote stub may know about a list of binaries to
-      // force load into the process -- a firmware type situation
-      // where multiple binaries are present in virtual memory,
-      // and we are only given the addresses of the binaries.
-      // Not intended for use with userland debugging when we
-      // a DynamicLoader plugin that knows how to find the loaded
-      // binaries and will track updates as binaries are added.
-
-      std::vector<addr_t> bin_addrs = m_gdb_comm.GetProcessStandaloneBinaries();
-      if (bin_addrs.size()) {
-        UUID uuid;
-        const bool value_is_slide = false;
-        for (addr_t addr : bin_addrs) {
-          const bool notify = true;
-          // First see if this is a special platform
-          // binary that may determine the DynamicLoader and
-          // Platform to be used in this Process/Target in the
-          // process of loading it.
-          if (GetTarget()
-                  .GetDebugger()
-                  .GetPlatformList()
-                  .LoadPlatformBinaryAndSetup(this, addr, notify))
-            continue;
-
-          const bool force_symbol_search = true;
-          // Second manually load this binary into the Target.
-          DynamicLoader::LoadBinaryWithUUIDAndAddress(
-              this, llvm::StringRef(), uuid, addr, value_is_slide,
-              force_symbol_search, notify);
-        }
-      }
-
       const StateType state = SetThreadStopInfo(response);
       if (state != eStateInvalid) {
         SetPrivateState(state);
@@ -1007,6 +955,7 @@
     }
   }
 
+  LoadStubBinaries();
   MaybeLoadExecutableModule();
 
   // Find out which StructuredDataPlugins are supported by the debug monitor.
@@ -1028,6 +977,60 @@
   }
 }
 
+void ProcessGDBRemote::LoadStubBinaries() {
+  // The remote stub may know about the "main binary" in
+  // the context of a firmware debug session, and can
+  // give us a UUID and an address/slide of where the
+  // binary is loaded in memory.
+  UUID standalone_uuid;
+  addr_t standalone_value;
+  bool standalone_value_is_offset;
+  if (m_gdb_comm.GetProcessStandaloneBinary(standalone_uuid, standalone_value,
+                                            standalone_value_is_offset)) {
+    ModuleSP module_sp;
+
+    if (standalone_uuid.IsValid()) {
+      const bool force_symbol_search = true;
+      const bool notify = true;
+      DynamicLoader::LoadBinaryWithUUIDAndAddress(
+          this, llvm::StringRef(), standalone_uuid, standalone_value,
+          standalone_value_is_offset, force_symbol_search, notify);
+    }
+  }
+
+  // The remote stub may know about a list of binaries to
+  // force load into the process -- a firmware type situation
+  // where multiple binaries are present in virtual memory,
+  // and we are only given the addresses of the binaries.
+  // Not intended for use with userland debugging when we
+  // a DynamicLoader plugin that knows how to find the loaded
+  // binaries and will track updates as binaries are added.
+
+  std::vector<addr_t> bin_addrs = m_gdb_comm.GetProcessStandaloneBinaries();
+  if (bin_addrs.size()) {
+    UUID uuid;
+    const bool value_is_slide = false;
+    for (addr_t addr : bin_addrs) {
+      const bool notify = true;
+      // First see if this is a special platform
+      // binary that may determine the DynamicLoader and
+      // Platform to be used in this Process/Target in the
+      // process of loading it.
+      if (GetTarget()
+              .GetDebugger()
+              .GetPlatformList()
+              .LoadPlatformBinaryAndSetup(this, addr, notify))
+        continue;
+
+      const bool force_symbol_search = true;
+      // Second manually load this binary into the Target.
+      DynamicLoader::LoadBinaryWithUUIDAndAddress(this, llvm::StringRef(), uuid,
+                                                  addr, value_is_slide,
+                                                  force_symbol_search, notify);
+    }
+  }
+}
+
 void ProcessGDBRemote::MaybeLoadExecutableModule() {
   ModuleSP module_sp = GetTarget().GetExecutableModule();
   if (!module_sp)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to