Re: [Lldb-commits] [PATCH] D19305: Use Process Plugin register indices when communicating with remote

2016-04-25 Thread Francis Ricci via lldb-commits 
fjricci added a comment.

Committed with comment added to ReadRegister declaration to specify that the 
argument is an eRegisterKindProcessPlugin register number


Repository:
  rL LLVM

http://reviews.llvm.org/D19305



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D19305: Use Process Plugin register indices when communicating with remote

2016-04-25 Thread Francis Ricci via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL267466: Use Process Plugin register indices when 
communicating with remote (authored by fjricci).

Changed prior to commit:
  http://reviews.llvm.org/D19305?vs=54303&id=54900#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19305

Files:
  lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -198,10 +198,11 @@
 GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info,
 GDBRemoteCommunicationClient 
&gdb_comm)
 {
-const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
+const uint32_t lldb_reg = reg_info->kinds[eRegisterKindLLDB];
+const uint32_t remote_reg = reg_info->kinds[eRegisterKindProcessPlugin];
 StringExtractorGDBRemote response;
-if (gdb_comm.ReadRegister(m_thread.GetProtocolID(), reg, response))
-return PrivateSetRegisterValue (reg, response);
+if (gdb_comm.ReadRegister(m_thread.GetProtocolID(), remote_reg, response))
+return PrivateSetRegisterValue (lldb_reg, response);
 return false;
 }
 
@@ -316,7 +317,7 @@
 StreamString packet;
 StringExtractorGDBRemote response;
 const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
-packet.Printf ("P%x=", reg);
+packet.Printf ("P%x=", reg_info->kinds[eRegisterKindProcessPlugin]);
 packet.PutBytesAsRawHex8 (m_reg_data.PeekData(reg_info->byte_offset, 
reg_info->byte_size),
   reg_info->byte_size,
   endian::InlHostByteOrder(),
@@ -813,7 +814,7 @@
 if (restore_src)
 {
 StreamString packet;
-packet.Printf ("P%x=", reg);
+packet.Printf ("P%x=", 
reg_info->kinds[eRegisterKindProcessPlugin]);
 packet.PutBytesAsRawHex8 (restore_src,
   reg_byte_size,
   
endian::InlHostByteOrder(),
@@ -836,7 +837,7 @@
 if (write_reg)
 {
 StreamString packet;
-packet.Printf ("P%x=", reg);
+packet.Printf ("P%x=", 
reg_info->kinds[eRegisterKindProcessPlugin]);
 packet.PutBytesAsRawHex8 (restore_src,
   reg_byte_size,
   
endian::InlHostByteOrder(),
@@ -894,7 +895,7 @@
 continue;
 }
 StreamString packet;
-packet.Printf ("P%x=", reg_info->kinds[eRegisterKindLLDB]);
+packet.Printf ("P%x=", 
reg_info->kinds[eRegisterKindProcessPlugin]);
 packet.PutBytesAsRawHex8 (data_sp->GetBytes() + 
reg_info->byte_offset, reg_info->byte_size, endian::InlHostByteOrder(), 
endian::InlHostByteOrder());
 if (thread_suffix_supported)
 packet.Printf (";thread:%4.4" PRIx64 ";", 
m_thread.GetProtocolID());
Index: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -527,7 +527,7 @@
 
 bool
 ReadRegister(lldb::tid_t tid,
- uint32_t reg_num,
+ uint32_t reg_num,   // Must be the eRegisterKindProcessPlugin 
register number, to be sent to the remote
  StringExtractorGDBRemote &response);
 
 bool


Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -198,10 +198,11 @@
 GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info,
 GDBRemoteCommunicationClient &gdb_comm)
 {
-const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
+const uint32_t lldb_reg = reg_info->kinds[eRegisterKindLLDB];
+const uint32_t remote_re

Re: [Lldb-commits] [PATCH] D19305: Use Process Plugin register indices when communicating with remote

2016-04-20 Thread Francis Ricci via lldb-commits 
fjricci added a comment.

I can add a comment to the ReadRegister decl.

Alternatively, if you think it would be preferable, I could change the 
parameter name from reg_num to something more descriptive, like remote_reg_num 
or plugin_reg_num.

Or I could change ReadRegister to take a RegisterInfo instead of a register 
number, and that way it could select the appropriate ProcessPlugin regnum on 
its own, right when it generates the packet. This method actually might be most 
consistent with the rest of the changes in the patch.


http://reviews.llvm.org/D19305



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D19305: Use Process Plugin register indices when communicating with remote

2016-04-20 Thread Greg Clayton via lldb-commits 
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Looks good. As to Jason's comment about a register number class: I wanted 
RegisterContext classes to be able to statically initialize an array or 
RegisterInfo structs without having to call C++ constructors. That is the main 
reason the register numbers are just an array.


http://reviews.llvm.org/D19305



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D19305: Use Process Plugin register indices when communicating with remote

2016-04-19 Thread Jason Molenda via lldb-commits 
jasonmolenda added a comment.

This looks good to me.  Do you think the ReadRegister decl in 
GDBRemoteCommunicationClient.h should have a comment noting that the reg_num is 
a number in the remote register numbering scheme (eRegisterKindProcessPlugin)?  
When lldb passes around register numbers internally, it is usually assuming the 
eRegisterKindLLDB register numbering convention.

(to be honest, I think we should have come up with a RegisterNumber class from 
the start - the presence of raw register numbers, with side knowledge about 
what register encoding should be used, makes it easy for mistakes to creep in.)


http://reviews.llvm.org/D19305



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D19305: Use Process Plugin register indices when communicating with remote

2016-04-19 Thread Francis Ricci via lldb-commits 
fjricci created this revision.
fjricci added reviewers: jasonmolenda, clayborg.
fjricci added subscribers: sas, lldb-commits.

eRegisterKindProcessPlugin is used to store the register
indices used by the remote, and eRegisterKindLLDB is used
to store the internal lldb register indices. However, we're currently
using the lldb indices instead of the process plugin indices
when sending p/P packets. This will break if the remote uses
non-contiguous register indices.

http://reviews.llvm.org/D19305

Files:
  source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

Index: source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -198,10 +198,11 @@
 GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info,
 GDBRemoteCommunicationClient 
&gdb_comm)
 {
-const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
+const uint32_t lldb_reg = reg_info->kinds[eRegisterKindLLDB];
+const uint32_t remote_reg = reg_info->kinds[eRegisterKindProcessPlugin];
 StringExtractorGDBRemote response;
-if (gdb_comm.ReadRegister(m_thread.GetProtocolID(), reg, response))
-return PrivateSetRegisterValue (reg, response);
+if (gdb_comm.ReadRegister(m_thread.GetProtocolID(), remote_reg, response))
+return PrivateSetRegisterValue (lldb_reg, response);
 return false;
 }
 
@@ -316,7 +317,7 @@
 StreamString packet;
 StringExtractorGDBRemote response;
 const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
-packet.Printf ("P%x=", reg);
+packet.Printf ("P%x=", reg_info->kinds[eRegisterKindProcessPlugin]);
 packet.PutBytesAsRawHex8 (m_reg_data.PeekData(reg_info->byte_offset, 
reg_info->byte_size),
   reg_info->byte_size,
   endian::InlHostByteOrder(),
@@ -813,7 +814,7 @@
 if (restore_src)
 {
 StreamString packet;
-packet.Printf ("P%x=", reg);
+packet.Printf ("P%x=", 
reg_info->kinds[eRegisterKindProcessPlugin]);
 packet.PutBytesAsRawHex8 (restore_src,
   reg_byte_size,
   
endian::InlHostByteOrder(),
@@ -836,7 +837,7 @@
 if (write_reg)
 {
 StreamString packet;
-packet.Printf ("P%x=", reg);
+packet.Printf ("P%x=", 
reg_info->kinds[eRegisterKindProcessPlugin]);
 packet.PutBytesAsRawHex8 (restore_src,
   reg_byte_size,
   
endian::InlHostByteOrder(),
@@ -894,7 +895,7 @@
 continue;
 }
 StreamString packet;
-packet.Printf ("P%x=", reg_info->kinds[eRegisterKindLLDB]);
+packet.Printf ("P%x=", 
reg_info->kinds[eRegisterKindProcessPlugin]);
 packet.PutBytesAsRawHex8 (data_sp->GetBytes() + 
reg_info->byte_offset, reg_info->byte_size, endian::InlHostByteOrder(), 
endian::InlHostByteOrder());
 if (thread_suffix_supported)
 packet.Printf (";thread:%4.4" PRIx64 ";", 
m_thread.GetProtocolID());


Index: source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -198,10 +198,11 @@
 GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info,
 GDBRemoteCommunicationClient &gdb_comm)
 {
-const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
+const uint32_t lldb_reg = reg_info->kinds[eRegisterKindLLDB];
+const uint32_t remote_reg = reg_info->kinds[eRegisterKindProcessPlugin];
 StringExtractorGDBRemote response;
-if (gdb_comm.ReadRegister(m_thread.GetProtocolID(), reg, response))
-return PrivateSetRegisterValue (reg, response);
+if (gdb_comm.ReadRegister(m_thread.GetProtocolID(), remote_reg, response))
+return PrivateSetRegisterValue (lldb_reg, response);
 return false;
 }
 
@@ -316,7 +317,7 @@
 StreamString packet;
 StringExtractorGDBRemote response;
 const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
-packet.Printf ("P%x=", reg);
+packet.Printf ("P