Re: [PATCH] Cast to uintptr_t when calling ptrace32 on aix

[Joel Brobecker <brobecker@adacore.com>]

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

> > > 	* aix-thread.c (pdc_read_regs): Cast parameter to uintptr_t
> > > 	first.
> > > 	(pdc_write_regs): Likewise.
> > > 	(fetch_regs_kernel_thread): Likewise.
> > > 	(store_regs_kernel_thread): Likewise.
> > 
> > Thank you, Yao. I tested your patch with AdaCore's gdb testsuite,
> > and found no regression.
> > 
> > Some of the lines are now exceeding the hard limit (80 characters),
> > and need to be folded. Apart from David's suggestion, which we can
> > look at independently, the patch looks OK to me.
> 
> Actually, re-reading David's comments, I agree with him. Can we try
> using a consistent approach? That would be an easy adaptation of your
> patch, or mine, whichever you'd prefer. I don't really see the real
> need for the second cast, so I'd lean towards adapting yours.

Attached is the patch I just pushed, after testing. FTR, you are
preserved as git author of the commit...

-- 
Joel

[-- Attachment #2: 0001-Cast-to-uintptr_t-when-calling-ptrace32-on-aix.patch --]
[-- Type: text/x-diff, Size: 6795 bytes --]

From 3b631e3720979156e83af0dac8b77f479384c2af Mon Sep 17 00:00:00 2001
From: Yao Qi <yao@codesourcery.com>
Date: Sat, 4 Jan 2014 15:48:21 +0800
Subject: [PATCH] Cast to uintptr_t when calling ptrace32 on aix

When I verify my changes to target.h doesn't break build on aix, I get
the following build error on a clean GDB checkout.

../../binutils-gdb/gdb/aix-thread.c: In function 'pdc_read_regs':
../../binutils-gdb/gdb/aix-thread.c:366:4: error: passing argument 3 of 'ptrace32' makes integer from pointer without a cast [-Werror]
    if (!ptrace32 (PTT_READ_GPRS, tid, gprs32, 0, NULL))
    ^
../../binutils-gdb/gdb/aix-thread.c:263:1: note: expected 'long long int' but argument is of type 'uint32_t *'
 ptrace32 (int req, int id, addr_ptr addr, int data, int *buf)
 ^

../../binutils-gdb/gdb/aix-thread.c:375:42: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
       if (!ptrace32 (PTT_READ_FPRS, tid, (addr_ptr) fprs, 0, NULL))
                                          ^

../../binutils-gdb/gdb/aix-thread.c:392:39: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
    if (!ptrace32 (PTT_READ_SPRS, tid, (addr_ptr) &sprs32, 0, NULL))

GCC uses -maix32 in default, so the 'long long' is 64 bit and address
is 32 bit.  Such warnings should go away if -maix64 is used.

In this patch, I cast the parameter to uintptr_t first, and then cast
to addr_ptr.

gdb:

2014-01-07  Yao Qi  <yao@codesourcery.com>
	    Joel Brobecker  <brobecker@adacore.com>

	* aix-thread.c (pdc_read_regs): Cast parameter to uintptr_t.
	(pdc_write_regs): Likewise.
	(fetch_regs_kernel_thread): Likewise.
	(store_regs_kernel_thread): Likewise.
---
 gdb/aix-thread.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 79adef0..20c7872 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -363,7 +363,7 @@ pdc_read_regs (pthdb_user_t user,
 	}
       else
 	{
-	  if (!ptrace32 (PTT_READ_GPRS, tid, gprs32, 0, NULL))
+	  if (!ptrace32 (PTT_READ_GPRS, tid, (uintptr_t) gprs32, 0, NULL))
 	    memset (gprs32, 0, sizeof (gprs32));
 	  memcpy (context->gpr, gprs32, sizeof(gprs32));
 	}
@@ -372,7 +372,7 @@ pdc_read_regs (pthdb_user_t user,
   /* Floating-point registers.  */
   if (flags & PTHDB_FLAG_FPRS)
     {
-      if (!ptrace32 (PTT_READ_FPRS, tid, (addr_ptr) fprs, 0, NULL))
+      if (!ptrace32 (PTT_READ_FPRS, tid, (uintptr_t) fprs, 0, NULL))
 	memset (fprs, 0, sizeof (fprs));
       memcpy (context->fpr, fprs, sizeof(fprs));
     }
@@ -389,7 +389,7 @@ pdc_read_regs (pthdb_user_t user,
 	}
       else
 	{
-	  if (!ptrace32 (PTT_READ_SPRS, tid, (addr_ptr) &sprs32, 0, NULL))
+	  if (!ptrace32 (PTT_READ_SPRS, tid, (uintptr_t) &sprs32, 0, NULL))
 	    memset (&sprs32, 0, sizeof (sprs32));
       	  memcpy (&context->msr, &sprs32, sizeof(sprs32));
 	}
@@ -424,13 +424,13 @@ pdc_write_regs (pthdb_user_t user,
 	ptrace64aix (PTT_WRITE_GPRS, tid, 
 		     (unsigned long) context->gpr, 0, NULL);
       else
-	ptrace32 (PTT_WRITE_GPRS, tid, (addr_ptr) context->gpr, 0, NULL);
+	ptrace32 (PTT_WRITE_GPRS, tid, (uintptr_t) context->gpr, 0, NULL);
     }
 
  /* Floating-point registers.  */
   if (flags & PTHDB_FLAG_FPRS)
     {
-      ptrace32 (PTT_WRITE_FPRS, tid, (addr_ptr) context->fpr, 0, NULL);
+      ptrace32 (PTT_WRITE_FPRS, tid, (uintptr_t) context->fpr, 0, NULL);
     }
 
   /* Special-purpose registers.  */
@@ -443,7 +443,7 @@ pdc_write_regs (pthdb_user_t user,
 	}
       else
 	{
-	  ptrace32 (PTT_WRITE_SPRS, tid, (addr_ptr) &context->msr, 0, NULL);
+	  ptrace32 (PTT_WRITE_SPRS, tid, (uintptr_t) &context->msr, 0, NULL);
 	}
     }
   return 0;
@@ -1250,7 +1250,7 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
 	}
       else
 	{
-	  if (!ptrace32 (PTT_READ_GPRS, tid, (addr_ptr) gprs32, 0, NULL))
+	  if (!ptrace32 (PTT_READ_GPRS, tid, (uintptr_t) gprs32, 0, NULL))
 	    memset (gprs32, 0, sizeof (gprs32));
 	  for (i = 0; i < ppc_num_gprs; i++)
 	    supply_reg32 (regcache, tdep->ppc_gp0_regnum + i, gprs32[i]);
@@ -1264,7 +1264,7 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
           || (regno >= tdep->ppc_fp0_regnum
               && regno < tdep->ppc_fp0_regnum + ppc_num_fprs)))
     {
-      if (!ptrace32 (PTT_READ_FPRS, tid, (addr_ptr) fprs, 0, NULL))
+      if (!ptrace32 (PTT_READ_FPRS, tid, (uintptr_t) fprs, 0, NULL))
 	memset (fprs, 0, sizeof (fprs));
       supply_fprs (regcache, fprs);
     }
@@ -1286,7 +1286,7 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
 	{
 	  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-	  if (!ptrace32 (PTT_READ_SPRS, tid, (addr_ptr) &sprs32, 0, NULL))
+	  if (!ptrace32 (PTT_READ_SPRS, tid, (uintptr_t) &sprs32, 0, NULL))
 	    memset (&sprs32, 0, sizeof (sprs32));
 	  supply_sprs32 (regcache, sprs32.pt_iar, sprs32.pt_msr, sprs32.pt_cr,
 			 sprs32.pt_lr, sprs32.pt_ctr, sprs32.pt_xer,
@@ -1581,9 +1581,9 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
       else
 	{
 	  /* Pre-fetch: some regs may not be in the cache.  */
-	  ptrace32 (PTT_READ_GPRS, tid, (addr_ptr) gprs32, 0, NULL);
+	  ptrace32 (PTT_READ_GPRS, tid, (uintptr_t) gprs32, 0, NULL);
 	  fill_gprs32 (regcache, gprs32);
-	  ptrace32 (PTT_WRITE_GPRS, tid, (addr_ptr) gprs32, 0, NULL);
+	  ptrace32 (PTT_WRITE_GPRS, tid, (uintptr_t) gprs32, 0, NULL);
 	}
     }
 
@@ -1595,9 +1595,9 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
               && regno < tdep->ppc_fp0_regnum + ppc_num_fprs)))
     {
       /* Pre-fetch: some regs may not be in the cache.  */
-      ptrace32 (PTT_READ_FPRS, tid, (addr_ptr) fprs, 0, NULL);
+      ptrace32 (PTT_READ_FPRS, tid, (uintptr_t) fprs, 0, NULL);
       fill_fprs (regcache, fprs);
-      ptrace32 (PTT_WRITE_FPRS, tid, (addr_ptr) fprs, 0, NULL);
+      ptrace32 (PTT_WRITE_FPRS, tid, (uintptr_t) fprs, 0, NULL);
     }
 
   /* Special-purpose registers.  */
@@ -1629,7 +1629,7 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
 	  gdb_assert (sizeof (sprs32.pt_iar) == 4);
 
 	  /* Pre-fetch: some registers won't be in the cache.  */
-	  ptrace32 (PTT_READ_SPRS, tid, (addr_ptr) &sprs32, 0, NULL);
+	  ptrace32 (PTT_READ_SPRS, tid, (uintptr_t) &sprs32, 0, NULL);
 
 	  fill_sprs32 (regcache, &tmp_iar, &tmp_msr, &tmp_cr, &tmp_lr,
 		       &tmp_ctr, &tmp_xer, &tmp_fpscr);
@@ -1648,7 +1648,7 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
 	      regcache_raw_collect (regcache, tdep->ppc_mq_regnum,
 				    &sprs32.pt_mq);
 
-	  ptrace32 (PTT_WRITE_SPRS, tid, (addr_ptr) &sprs32, 0, NULL);
+	  ptrace32 (PTT_WRITE_SPRS, tid, (uintptr_t) &sprs32, 0, NULL);
 	}
     }
 }
-- 
1.8.3.2