[Lldb-commits] [PATCH] D25021: [LLDB][MIPS] Fix qProcessInfo to return correct pointer size based on ELF ABI

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We need to send each flag individually. See inlined comments.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1887
@@ -1885,1 +1886,3 @@
             ++num_keys_decoded;
+        } else if (name.equals("eflags")) {
+          if (!value.getAsInteger(16,proc_arch_eflags))
----------------
labath wrote:
> Why you chose this name? eflags is the name of a i386 register. How about we use a different field name? `archflags` maybe ?
We should avoid trying pass ArchSpec specific flags over the wire. Seems like we should be abstracting each flag we want to send as a new key. How is some code that isn't based on LLDB going to fill in these flags? Please add a key/value pair for each flag you want to send. You can add a function to ArchSpec that gets a StructuredData::Dictionary from the ArchSpec and then uses that to populate the key/value pairs that you send back. Each architecture defines a set of unique flags. Another issue with sending e_flags directly is if someone changes the bit definitions of the flags and we have a new LLDB hook up to an older lldb-server, we will get the flags wrong.

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1218
@@ +1217,3 @@
+    response.Printf("eflags:%" PRIx32 ";", proc_arch.GetFlags());
+    if (proc_triple.isArch64Bit()) {
+       if (proc_arch.IsMIPS()) {
----------------
labath wrote:
> Why do we have the compilcated switch here. Can't we replace that with:
> `response.Printf("ptrsize:%d;", proc_arch.GetAddressByteSize());` for all sizes?
labath: the default ptr size for MIPS64 is 8. We would need to modify ArchSpec.cpp to look at the flags for MIPS64 and change the pointer size to 4 if the N32 is being used. 


Repository:
  rL LLVM

https://reviews.llvm.org/D25021