diff mbox

[3/4] gdb.trace: Use g packet order in tfile_fetch_registers.

Message ID 1454773157-31569-4-git-send-email-koriakin@0x04.net
State New, archived
Headers

Commit Message

Marcin Kościelnicki Feb. 6, 2016, 3:39 p.m. UTC 
  tfile_fetch_registers currently wrongly fetches registers using
gdb order instead of g packet order.  On x86_64 with AVX, this causes
problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
g packet has orig_rax first.

gdb/ChangeLog:

	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
	instead of gdb order.
---
 gdb/ChangeLog         |  5 +++++
 gdb/tracefile-tfile.c | 11 ++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

Comments

Pedro Alves Feb. 10, 2016, 1:20 p.m. UTC | #1 
On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:
> tfile_fetch_registers currently wrongly fetches registers using
> gdb order instead of g packet order.  On x86_64 with AVX, this causes
> problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
> g packet has orig_rax first.
> 
> gdb/ChangeLog:
> 
> 	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
> 	instead of gdb order.

OK.

The docs already explicitly say that we use g packet order, though the
bit about GDB register order seems odd:

 @table @code
 @item R @var{bytes}
 Register block.  The number and ordering of bytes matches that of a
                                 ^^^^^^^^          ^^^^^^^

 @code{g} packet in the remote protocol.  Note that these are the
 ^^^^^^^^^^^^^^^
 actual bytes, in target order and @value{GDBN} register order, not a
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^

                                            ????

 hexadecimal encoding.


I can't make sense of that.  I think we should
s/and @value{GDBN} register order,// .

WDYT?

Thanks,
Pedro Alves
Marcin Kościelnicki Feb. 10, 2016, 1:21 p.m. UTC | #2 
On 10/02/16 14:20, Pedro Alves wrote:
> On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:
>> tfile_fetch_registers currently wrongly fetches registers using
>> gdb order instead of g packet order.  On x86_64 with AVX, this causes
>> problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
>> g packet has orig_rax first.
>>
>> gdb/ChangeLog:
>>
>> 	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
>> 	instead of gdb order.
>
> OK.
>
> The docs already explicitly say that we use g packet order, though the
> bit about GDB register order seems odd:
>
>   @table @code
>   @item R @var{bytes}
>   Register block.  The number and ordering of bytes matches that of a
>                                   ^^^^^^^^          ^^^^^^^
>
>   @code{g} packet in the remote protocol.  Note that these are the
>   ^^^^^^^^^^^^^^^
>   actual bytes, in target order and @value{GDBN} register order, not a
>                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>                                              ????
>
>   hexadecimal encoding.
>
>
> I can't make sense of that.  I think we should
> s/and @value{GDBN} register order,// .
>
> WDYT?
>
> Thanks,
> Pedro Alves
>

Yeah, I think so.  Should I add it to the patch?
Pedro Alves Feb. 10, 2016, 1:54 p.m. UTC | #3 
On 02/10/2016 01:21 PM, Marcin Kościelnicki wrote:

>> I can't make sense of that.  I think we should
>> s/and @value{GDBN} register order,// .
>>
>> WDYT?
>>

> Yeah, I think so.  Should I add it to the patch?

Yes please.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f5cd0c7..5afdb60 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@ 
 2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
+	instead of gdb order.
+
+2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* tracefile-tfile.c (trace_tdesc): New static variable.
 	(trace_tdesc_alloc): New static variable.
 	(trace_tdesc_len): New static variable.
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 71ac437..23d78c3 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -30,6 +30,7 @@ 
 #include "filenames.h"
 #include "xml-tdesc.h"
 #include "target-descriptions.h"
+#include "remote.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -857,7 +858,7 @@  tfile_fetch_registers (struct target_ops *ops,
 		       struct regcache *regcache, int regno)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  int offset, regn, regsize;
+  int offset, regn, regsize, dummy;
 
   /* An uninitialized reg size says we're not going to be
      successful at getting register blocks.  */
@@ -870,11 +871,12 @@  tfile_fetch_registers (struct target_ops *ops,
 
       tfile_read (regs, trace_regblock_size);
 
-      /* Assume the block is laid out in GDB register number order,
-	 each register with the size that it has in GDB.  */
-      offset = 0;
       for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++)
 	{
+	  if (!remote_register_number_and_offset (get_regcache_arch (regcache),
+						  regn, &dummy, &offset))
+	    continue;
+
 	  regsize = register_size (gdbarch, regn);
 	  /* Make sure we stay within block bounds.  */
 	  if (offset + regsize >= trace_regblock_size)
@@ -891,7 +893,6 @@  tfile_fetch_registers (struct target_ops *ops,
 		  regcache_raw_supply (regcache, regn, regs + offset);
 		}
 	    }
-	  offset += regsize;
 	}
     }
   else