Re: [RFC PATCH] hppa-hpux-tdep.c: Fix host dependency.

[Pedro Alves <palves@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

[Adding Dave]

Hi Dave,

On 03/08/2013 10:56 PM, Joel Brobecker wrote:
>> I have no way to test this though, other than making sure it compiles.
>> I'm inclined to just go ahead and put it in, if nobody's up for
>> testing it.
>>
>> Comments?
>>
>> 2013-03-08  Pedro Alves  <palves@redhat.com>
>>
>> 	* hppa-hpux-tdep.c (hppa_hpux_push_dummy_code): Define INSN macro,
>> 	use it to rewrite the trampoline buffers with type gdb_byte[], and
>> 	undefine the macro.  Remove char* cast.
> 
> FWIW, it looks good to me too. IIRC, John David Anglin is still
> able to test pa-hpux patches (Tom should be able to confirm that,
> since he had some SOM patches tested recently). I wish I could too,
> but we turned our machines off and put them in storage...
> 

Would you be able to help giving this patch a quick try
on HP-UX?

Thanks,
-- 
Pedro Alves


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-hppa-hpux-tdep.c-Fix-host-dependency.patch --]
[-- Type: text/x-patch; name="0001-hppa-hpux-tdep.c-Fix-host-dependency.patch", Size: 5670 bytes --]

From 885ed2f17eabe2deb1dd52706a5345d752b6bdee Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 8 Mar 2013 15:01:02 +0000
Subject: [PATCH] hppa-hpux-tdep.c: Fix host dependency.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

$ make WERROR_CFLAGS="-Wpointer-sign -Werror" hppa-hpux-tdep.o -k 2>&1 1>/dev/null
../../src/gdb/hppa-hpux-tdep.c: In function ‘hppa_hpux_push_dummy_code’:
../../src/gdb/hppa-hpux-tdep.c:1225:7: error: pointer targets in passing argument 2 of ‘write_memory’ differ in signedness [-Werror=pointer-sign]
In file included from ../../src/gdb/hppa-hpux-tdep.c:22:0:
../../src/gdb/gdbcore.h:85:13: note: expected ‘const gdb_byte *’ but argument is of type ‘char *’
../../src/gdb/hppa-hpux-tdep.c:1251:7: error: pointer targets in passing argument 2 of ‘write_memory’ differ in signedness [-Werror=pointer-sign]
In file included from ../../src/gdb/hppa-hpux-tdep.c:22:0:
../../src/gdb/gdbcore.h:85:13: note: expected ‘const gdb_byte *’ but argument is of type ‘char *’
../../src/gdb/hppa-hpux-tdep.c: In function ‘hppa_hpux_supply_save_state’:
../../src/gdb/hppa-hpux-tdep.c:1354:9: error: pointer targets in passing argument 1 of ‘extract_unsigned_integer’ differ in signedness [-Werror=pointer-sign]
In file included from ../../src/gdb/hppa-hpux-tdep.c:20:0:
../../src/gdb/defs.h:675:22: note: expected ‘const gdb_byte *’ but argument is of type ‘const char *’

Casting to gdb_byte would fix it, however, writing an
unsigned int array like this

      static unsigned int hppa64_tramp[] = {
        0xeac0f000, /* bve,l (r22),%r2 */
        0x0fdf12d1, /* std r31,-8(,sp) */
        0x0fd110c2, /* ldd -8(,sp),rp */
        0xe840d002, /* bve,n (rp) */
        0x08000240  /* nop */
        ...

directly to target memory assumes the host endianness is the same as
the target's.  hppa is big endian, so I believe this patch should be
correct -- it defines the array as a gdb_byte array.  It uses a macro
to make the insn bytes a little more readable.  I thought of using
write_memory_unsigned_integer once for each element of the unsigned
int array, but this way keeps issuing a single target memory write /
roundtrip for the whole trampoline.

I have no way to test this though, other than making sure it compiles.
I'm inclined to just go ahead and put it in, if nobody's up for
testing it.

Comments?

2013-03-08  Pedro Alves  <palves@redhat.com>

	* hppa-hpux-tdep.c (hppa_hpux_push_dummy_code): Define INSN macro,
	use it to rewrite the trampoline buffers with type gdb_byte[], and
	undefine the macro.  Remove char* cast.
---
 gdb/hppa-hpux-tdep.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/gdb/hppa-hpux-tdep.c b/gdb/hppa-hpux-tdep.c
index aacb0a3..9948e9b 100644
--- a/gdb/hppa-hpux-tdep.c
+++ b/gdb/hppa-hpux-tdep.c
@@ -1201,17 +1201,18 @@ hppa_hpux_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
 
   if (IS_32BIT_TARGET (gdbarch))
     {
-      static unsigned int hppa32_tramp[] = {
-        0x0fdf1291, /* stw r31,-8(,sp) */
-        0x02c010a1, /* ldsid (,r22),r1 */
-        0x00011820, /* mtsp r1,sr0 */
-        0xe6c00000, /* be,l 0(sr0,r22),%sr0,%r31 */
-        0x081f0242, /* copy r31,rp */
-        0x0fd11082, /* ldw -8(,sp),rp */
-        0x004010a1, /* ldsid (,rp),r1 */
-        0x00011820, /* mtsp r1,sr0 */
-        0xe0400000, /* be 0(sr0,rp) */
-        0x08000240  /* nop */
+#define INSN(I1, I2, I3, I4) 0x ## I1, 0x ## I2, 0x ## I3, 0x ## I4
+     static const gdb_byte hppa32_tramp[] = {
+	INSN(0f,df,12,91), /* stw r31,-8(,sp) */
+	INSN(02,c0,10,a1), /* ldsid (,r22),r1 */
+	INSN(00,01,18,20), /* mtsp r1,sr0 */
+	INSN(e6,c0,00,00), /* be,l 0(sr0,r22),%sr0,%r31 */
+	INSN(08,1f,02,42), /* copy r31,rp */
+	INSN(0f,d1,10,82), /* ldw -8(,sp),rp */
+	INSN(00,40,10,a1), /* ldsid (,rp),r1 */
+	INSN(00,01,18,20), /* mtsp r1,sr0 */
+	INSN(e0,40,00,00), /* be 0(sr0,rp) */
+	INSN(08,00,02,40)  /* nop */
       };
 
       /* for hppa32, we must call the function through a stub so that on
@@ -1222,7 +1223,7 @@ hppa_hpux_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
 	       "(no import stub).\n"));
       regcache_cooked_write_unsigned (regcache, 22, stubaddr);
 
-      write_memory (sp, (char *)&hppa32_tramp, sizeof (hppa32_tramp));
+      write_memory (sp, hppa32_tramp, sizeof (hppa32_tramp));
 
       *bp_addr = hppa_hpux_find_dummy_bpaddr (pc);
       regcache_cooked_write_unsigned (regcache, 31, *bp_addr);
@@ -1237,18 +1238,19 @@ hppa_hpux_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
     }
   else
     {
-      static unsigned int hppa64_tramp[] = {
-        0xeac0f000, /* bve,l (r22),%r2 */
-        0x0fdf12d1, /* std r31,-8(,sp) */
-        0x0fd110c2, /* ldd -8(,sp),rp */
-        0xe840d002, /* bve,n (rp) */
-        0x08000240  /* nop */
+      static const gdb_byte hppa64_tramp[] = {
+	INSN(ea,c0,f0,00), /* bve,l (r22),%r2 */
+	INSN(0f,df,12,d1), /* std r31,-8(,sp) */
+	INSN(0f,d1,10,c2), /* ldd -8(,sp),rp */
+	INSN(e8,40,d0,02), /* bve,n (rp) */
+	INSN(08,00,02,40)  /* nop */
       };
+#undef INSN
 
       /* for hppa64, we don't need to call through a stub; all functions
          return via a bve.  */
       regcache_cooked_write_unsigned (regcache, 22, funcaddr);
-      write_memory (sp, (char *)&hppa64_tramp, sizeof (hppa64_tramp));
+      write_memory (sp, hppa64_tramp, sizeof (hppa64_tramp));
 
       *bp_addr = pc - 4;
       regcache_cooked_write_unsigned (regcache, 31, *bp_addr);
-- 
1.7.11.7