[Lldb-commits] [PATCH] D27289: Return "thread-pcs" in jstopinfo on Linux/Android.

[Pavel Labath via Phabricator via lldb-commits]

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 &reg_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