labath added a subscriber: mgorny.
labath added a comment.
I think it would be good to split this patch into two:
- implementing `g` packet in lldb-server
- deciding when lldb sends the `g` packet
For the first part, I don't see any reason why lldb-server should *not* support
the `g` packet.
The second part, as others have pointed out is a bit more tricky, as the `g`
packet may not always be a win. For that, it would be nice to know more about
your use case. When you say "all registers are being fetched every step", do
you mean that lldb does that on its own, or that you (as in, something external
to lldb) for whatever reason want to have all registers read out after each
step?
If it's the first thing, then we can probably do something even better, and
make sure that lldb-server sends the required registers directly in the
stop-reply packet.
If it's the second thing then we can play around with the conditions under
which lldb can use the `g` packet. Eg. it definitely sounds like `register read
--all` would benefit from this packet, but maybe `register read
$specific_register` might not. Or this may not even matter, as for the most
common values of `$specific_register`, we should have the data available from
the stop-reply packets, and others aren't really used that often...
================
Comment at:
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteGPacket.py:14
- def run_test_g_packet(self):
+ def run_test_g_packet(self, read, write):
self.build()
----------------
This test is pretty weak, as all it does is check that "some" data was
received. It would be much better to implement something similar to what
@mgorny did in <https://reviews.llvm.org/D61072> and other patches, only at
lldb-server level. I.e., have an inferior which sets as many registers as
possible to known values, and then have the test assert that. There should
already be some code which parses `qRegisterInfo` responses in the test suite
somewhere, so all you'd need is to fetch the offsets from there, and check that
the values are indeed correct.
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1912-1954
+ NativeRegisterContext ®_ctx = thread->GetRegisterContext();
+
+ // As the register offsets are not necessarily sorted,
+ // use a map to store all registers data.
+ std::map<uint32_t, uint8_t> regs_buffer;
+ for (uint32_t reg_num = 0; reg_num < reg_ctx.GetUserRegisterCount();
+ ++reg_num) {
----------------
There's a `NativeRegisterContext::ReadAllRegisterValues` function. Can you
check if that returns the data in the format that you need here?
Even if it doesn't, there's a good chance we could tweak it so that it does, as
I believe it's now only used for implementing QSave/RestoreRegisterState, and
the format we use for saving that is completely up to us.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62221/new/
https://reviews.llvm.org/D62221
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
