labath added a comment.
Looks like a good starting point. Apart from some style nits, I'd like us to
remove sending of the `registers` field in the `jstopinfo`, as it is not
necessary anymore. Also, we should add an lldb-server style test for this.
@jasonmolenda, we already have jThreadsInfo support in lldb-server, albeit
without the memory snippet parts. A lot less functions use frame pointers on
linux than on osx, so I was not sure whether implementing the simple
FP-following will be useful. However, that is the next thing on my mind in
terms of optimizations here.
================
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:441
+static void WriteRegisterValueInHexFixedWidth(
+ StreamString &response, NativeRegisterContextSP ®_ctx_sp,
----------------
This function is called in only two sites. My preference would be to update
them to use the new signature instead of adding another overload.
================
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:579
if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread_sp, abridged))
thread_obj_sp->SetObject("registers", registers_sp);
----------------
We can now change this code to:
```
if (!abridged) {
if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread_sp))
...
}
```
This was my attempt to send the PCs in the `jstopinfo` field, which used to
work at some point, but now lldb client does not seem to recognize it. When we
start sending the PC in the `thread-pcs` field, the whole `registers` field
will become completely redundant, and we should remove it.
This will not affect the `jThreadsInfo` packet, as there we pass `abridged =
false`.
(Obviously we should also clean up the `GetRegistersAsJSON` function to reflect
the fact it does not need to support PC-only register sending.)
================
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:582
+
thread_obj_sp->SetObject("tid", std::make_shared<JSONNumber>(tid));
----------------
Please remove the random whitespace.
https://reviews.llvm.org/D27289
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
