Re: [PATCH] Clear *VAL in regcache_raw_read_unsigned

[Yao Qi <qiyaoltc at gmail dot com>]

Pedro Alves <palves@redhat.com> writes:

Hi Pedro,

> The issue you noticed exposed that regcache_raw_read_unsigned function
> is broken for memcpy'ing a 32-bit value into a 64-bit variable, and I think
> your patch papered over the problem for little endian, only.

regcache_raw_read_unsigned has two orthogonal issues, one is VAL isn't
cleared, and the other one is the endianness issue.  My "Clear *VAL"
patch fixes the former, and I didn't realize the latter then.

>
> enum register_status
> regcache_raw_read_unsigned (struct regcache *regcache, int regnum,
> 			    ULONGEST *val)
> {
>   int size;
>
>   gdb_assert (regcache != NULL);
>   gdb_assert (regnum >= 0 && regnum < regcache->tdesc->num_registers);
>
>   size = register_size (regcache->tdesc, regnum);
>
>   if (size > (int) sizeof (ULONGEST))
>     error (_("That operation is not available on integers of more than"
>             "%d bytes."),
>           (int) sizeof (ULONGEST));
>
>   collect_register (regcache, regnum, val);
>
>   return REG_VALID;
> }
>
> VAL is 64-bit.  collect_register () writes directly into VAL, but it
> only writes 32-bits.  On little endian, that will leave the upper halve
> of VAL as garbage.  So your patch clears that.  But on big endian,
> that collect_register() call writes into the upper halve of VAL, and
> the result is that VAL now has the wrong value.

On big endian, we need to clear VAL too, otherwise the bottom half of
VAL is garbage.

>
> E.g., if the 32-bit register's value is supposed to be 0x11223344,
> after the collect register call, *VAL ends up with 0x1122334400000000,
> which happens to work for little endian, but not for big endian.
>
> You should be able to trigger this on an ARM system with big endian code.

I don't have a big endian system on my hand, but my patch below, which is
copied from ppc_collect_ptrace_register, should fix the endianess problem.

-- 
Yao (éå)
From 330b996c191b119a6b0414c2d96c4e20f18588bf Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Thu, 11 Feb 2016 09:53:35 +0000
Subject: [PATCH] Fix regcache_raw_read_unsigned in big endian

This patch fixes function regcache_raw_read_unsigned in big endian.

gdb/gdbserver:

2016-02-11  Yao Qi  <yao.qi@linaro.org>

	* regcache.c: Include endian.h.
	(regcache_raw_read_unsigned): Handle big endian.

diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 2af8e24..d69ed5b 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -21,6 +21,8 @@
 #include "gdbthread.h"
 #include "tdesc.h"
 #include "rsp-low.h"
+#include <endian.h>
+
 #ifndef IN_PROCESS_AGENT
 
 struct regcache *
@@ -441,7 +443,27 @@ regcache_raw_read_unsigned (struct regcache *regcache, int regnum,
           (int) sizeof (ULONGEST));
 
   *val = 0;
-  collect_register (regcache, regnum, val);
+
+  if (__BYTE_ORDER == __LITTLE_ENDIAN)
+    {
+      /* Little-endian values always sit at the left end of the buffer.  */
+      collect_register (regcache, regnum, val);
+    }
+  else if (__BYTE_ORDER == __BIG_ENDIAN)
+    {
+      /* Big-endian values sit at the right end of the buffer.  In case of
+	 registers whose sizes are smaller than sizeof (long), we must use a
+	 padding to access them correctly.  */
+      int size = register_size (regcache->tdesc, regnum);
+
+      if (size < sizeof (ULONGEST))
+	{
+	  collect_register (regcache, regnum,
+			    (char *) val + sizeof (ULONGEST) - size);
+	}
+      else
+	collect_register (regcache, regnum, val);
+    }
 
   return REG_VALID;
 }