[Lldb-commits] [PATCH] D62221: [lldb-server][LLGS] Support 'g' packets

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp:307-316
+#ifdef LLDB_REMOTE_SHOULD_READ_ALL_REGISTERS_AT_ONCE
+      // Always fetch the registers at once when the flag has been defined.
+      bool read_all_registers_at_once = true;
+#else
       // read_all_registers_at_once will be true if 'p' packet is not
       // supported.
       bool read_all_registers_at_once =
----------------
clayborg wrote:
> I would make this a gdb-remote setting like "plugin.process.gdb-remote.packet-timeout" but named "plugin.process.gdb-remote.use-g-packet". Then we don't need a #define. It would be fine to use this all the time for most targets I believe. So it might be worth trying this out. We will need to watch stepping times.
One way to force the use of g/G is to not support p/P in the Remote Serial Protocol stub.  I've seen some stubs that behave this way.  g is the older packet, and IMO is a more fragile one because it requires complete agreement about the layout of the register context. for lldb + lldbserver or lldb + debugserver, this isn't such a concern, but with interop with random other Remote Serial Protocol implementations, p/P is much simpler.

I understand the perf concerns of using p/P for multiple registers - as Greg says, ideally the RSP stub will include the base register context for the stopped thread in the stop packet aka T packet aka ? packet.  For debugging with multiple threads, we use the jThreadsInfo packet to send the GPR register contents and likely stack memory needed for a simple stack unwind to address these issues.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62221/new/

https://reviews.llvm.org/D62221