Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH v4 2/4] Remove stale legacy code from rs6000-aix-nat.c file.
@ 2026-10-05 12:16 Aditya Vidyadhar Kamath
  2026-10-05 15:02 ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Aditya Vidyadhar Kamath @ 2026-10-05 12:16 UTC (permalink / raw)
  To: ulrich.weigand, simon.marchi, tom
  Cc: gdb-patches, Aditya.Kamath1, sangamesh.swamy, Aditya Vidyadhar Kamath

From: Aditya Vidyadhar Kamath <aditya.kamath1@ibm.com>

GDB on AIX is built in 64 bit mode only.

As per
 https://www.ibm.com/docs/en/aix/7.2.0?topic=p-ptrace-ptracex-ptrace64-subroutine

though ptrace64 will also support 32-bit debugees if debugee is a 32 bit process,
the options that refer to GPRs or SPRs fail with errno = EIO.

This is something I have left in the comments of the code as well, so we do not miss
it while testing.

When I built GDB as a 64 bit AIX application [Using ptrace () for 32 bit inferior debugee]
 to just test, linking fails with:

CXXLD  gdb
  ld: 0711-317 ERROR: Undefined symbol: .ptrace
  ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
  collect2: error: ld returned 8 exit status
  gmake[2]: *** [Makefile:2367: gdb] Error 1

This clearly shows from AIX 7.2, the standard ptrace system call symbol is not
exported in 64-bit mode; 64 bit mode must use ptrace64 exclusively which is what
this patch does.

As document mentioned,
ptrace: 32- or 64-bit GDB, 32-bit inferior
ptracex: 32-bit GDB, 64-bit inferior
ptrace64: 64-bit GDB, 64-bit inferior

This is right,

Legacy rs6000_ptrace32 and rs6000_ptrace64 wrappers conditionally called
ptrace/ptracex/ptrace64 and relied on ARCH3264 macro. This macro is now not needed.
We will only use ptrace64 going forward.

This patch modernises rs6000-aix-nat.c for AIX 7.2+/7.3+

Here are my thoughts during implementation.
1: Remove rs6000_ptrace32 () and rs6000_ptrace64 () completey. We will only use
   ptrace64 for all operations. No ptracex/ptrace any more since GDB will not be
   built in 32 bit mode.
2: Wrote get_thread_id () helper to obtain kernel thread ID via getthrds64 ()
3: For a 32 bit debugee where process-level PT_READ_GPR, PT_WRITE_GPR and
   PT_READ_FPR/PT_WRITE_FPR fail with EIO when called from 64 bit GDB via ptrace64 ()
   let us use thread-level PTT_READ_GPRS/PTT_WRITE_GPRS and PTT_READ_SPRS/PTT_WRITE_SPRS.
   Likewise, PTT_READ_FPRS/PTT_WRITE_FPRS.

I also ran tests for for both 64 bit and 32 bit mode. First result is 64 bit mode. Second summary is 32 bit. Ran the command in gdb/testsuite directory.
Command used: gmake check RUNTESTFLAGS='--directory=gdb.base CC_FOR_TARGET="/opt/freeware/bin/gcc" CXX_FOR_TARGET="/opt/freeware/bin/g++" CXXFLAGS_FOR_TARGET="-O0 -w -g -gdwarf -maix64" CFLAGS_FOR_TARGET="-O0 -w -g -gdwarf -maix64"'

Before this patch:
=== gdb Summary ===
# of unexpected core files      1
# of expected passes            41792
# of unexpected failures        2346
# of unexpected successes       1
# of expected failures          17
# of known failures             30
# of unresolved testcases       30
# of untested testcases         104
# of unsupported tests          179
# of paths in test names        7
# of duplicate test names       2

--------------------------------------------------
                === gdb Summary ===

# of unexpected core files      1
# of expected passes            41687
# of unexpected failures        2302
# of unexpected successes       1
# of expected failures          15
# of known failures             30
# of unresolved testcases       11
# of untested testcases         107
# of unsupported tests          179
# of paths in test names        7
# of duplicate test names       2

After this patch:
=== gdb Summary ===

# of unexpected core files      1
# of expected passes            41617
# of unexpected failures        2297
# of unexpected successes       1
# of expected failures          15
# of known failures             30
# of unresolved testcases       10
# of untested testcases         108
# of unsupported tests          179
# of paths in test names        7
# of duplicate test names       2

------------------------------------
                === gdb Summary ===

# of unexpected core files      1
# of expected passes            41796
# of unexpected failures        2346
# of unexpected successes       1
# of expected failures          17
# of known failures             30
# of unresolved testcases       31
# of untested testcases         105
# of unsupported tests          179
# of paths in test names        7
# of duplicate test names       2

--------------------------------------------------

---
 gdb/rs6000-aix-nat.c | 306 +++++++++++++++++++++----------------------
 1 file changed, 148 insertions(+), 158 deletions(-)

diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 276df378824..6113bf988bc 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -60,24 +60,9 @@
 /* Header files for alti-vec reg.  */
 #include <sys/context.h>
 
-/* On AIX4.3+, sys/ldr.h provides different versions of struct ld_info for
-   debugging 32-bit and 64-bit processes.  Define a typedef and macros for
-   accessing fields in the appropriate structures.  */
-
-/* In 32-bit compilation mode (which is the only mode from which ptrace()
-   works on 4.3), __ld_info32 is #defined as equivalent to ld_info.  */
-
-#if defined (__ld_info32) || defined (__ld_info64)
-# define ARCH3264
-#endif
-
 /* Return whether the current architecture is 64-bit.  */
 
-#ifndef ARCH3264
-# define ARCH64() 0
-#else
-# define ARCH64() (register_size (current_inferior ()->arch (), 0) == 8)
-#endif
+#define ARCH64() (register_size (current_inferior ()->arch (), 0) == 8)
 
 class rs6000_nat_target final : public inf_ptrace_target
 {
@@ -246,42 +231,20 @@ regmap (struct gdbarch *gdbarch, int regno, int *isfloat)
     return -1;
 }
 
-/* Call ptrace(REQ, ID, ADDR, DATA, BUF).  */
+/* Return the thread ID of the first kernel thread in PID, or 0 on
+   failure.  Used to call PTT_* requests which require a tid rather
+   than a pid.  */
 
-static int
-rs6000_ptrace32 (int req, int id, int *addr, int data, int *buf)
+static tid64_t
+get_thread_id (pid_t pid)
 {
-#ifdef HAVE_PTRACE64
-  int ret = ptrace64 (req, id, (uintptr_t) addr, data, buf);
-#else
-  int ret = ptrace (req, id, (int *)addr, data, buf);
-#endif
-#if 0
-  printf ("rs6000_ptrace32 (%d, %d, 0x%x, %08x, 0x%x) = 0x%x\n",
-	  req, id, (unsigned int)addr, data, (unsigned int)buf, ret);
-#endif
-  return ret;
-}
-
-/* Call ptracex(REQ, ID, ADDR, DATA, BUF).  */
+  struct thrdentry64 thrdentry;
+  tid64_t thrd_i = 0;
 
-static int
-rs6000_ptrace64 (int req, int id, long long addr, int data, void *buf)
-{
-#ifdef ARCH3264
-#  ifdef HAVE_PTRACE64
-  int ret = ptrace64 (req, id, addr, data, (PTRACE_TYPE_ARG5) buf);
-#  else
-  int ret = ptracex (req, id, addr, data, (PTRACE_TYPE_ARG5) buf);
-#  endif
-#else
-  int ret = 0;
-#endif
-#if 0
-  printf ("rs6000_ptrace64 (%d, %d, %s, %08x, 0x%x) = 0x%x\n",
-	  req, id, hex_string (addr), data, (unsigned int)buf, ret);
-#endif
-  return ret;
+  if (getthrds64 (pid, &thrdentry, sizeof (struct thrdentry64),
+		  &thrd_i, 1) == 1)
+    return thrdentry.ti_tid;
+  return 0;
 }
 
 /* Store the vsx registers.  */
@@ -292,32 +255,21 @@ store_vsx_register_aix (struct regcache *regcache, int regno)
   int ret;
   struct gdbarch *gdbarch = regcache->arch ();
   ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
-  struct thrdentry64 thrdentry;
   __vsx_context_t vsx;
   pid_t pid = inferior_ptid.pid ();
-  tid64_t thrd_i = 0;
-
-  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
-		 &thrd_i, 1) == 1)
-    thrd_i = thrdentry.ti_tid;
+  tid64_t thrd_i = get_thread_id (pid);
 
   memset(&vsx, 0, sizeof(__vsx_context_t));
   if (__power_vsx() && thrd_i > 0)
     {
-      if (ARCH64 ())
-	ret = rs6000_ptrace64 (PTT_READ_VSX, thrd_i, (long long) &vsx, 0, 0);
-      else
-	ret = rs6000_ptrace32 (PTT_READ_VSX, thrd_i, (int *)&vsx, 0, 0);
+      ret = ptrace64 (PTT_READ_VSX, thrd_i, (long long) &vsx, 0, 0);
       if (ret < 0)
 	return;
 
       regcache->raw_collect (regno, &(vsx.__vsr_dw1[0])+
 			     regno - tdep->ppc_vsr0_upper_regnum);
 
-      if (ARCH64 ())
-	ret = rs6000_ptrace64 (PTT_WRITE_VSX, thrd_i, (long long) &vsx, 0, 0);
-      else
-	ret = rs6000_ptrace32 (PTT_WRITE_VSX, thrd_i, (int *) &vsx, 0, 0);
+      ret = ptrace64 (PTT_WRITE_VSX, thrd_i, (long long) &vsx, 0, 0);
 
       if (ret < 0)
 	perror_with_name (_("Unable to write VSX registers after reading it"));
@@ -332,32 +284,21 @@ store_altivec_register_aix (struct regcache *regcache, int regno)
   int ret;
   struct gdbarch *gdbarch = regcache->arch ();
   ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
-  struct thrdentry64 thrdentry;
   __vmx_context_t vmx;
   pid_t pid = inferior_ptid.pid ();
-  tid64_t  thrd_i = 0;
-
-  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
-		 &thrd_i, 1) == 1)
-    thrd_i = thrdentry.ti_tid;
+  tid64_t thrd_i = get_thread_id (pid);
 
   memset(&vmx, 0, sizeof(__vmx_context_t));
   if (__power_vmx() && thrd_i > 0)
     {
-      if (ARCH64 ())
-	ret = rs6000_ptrace64 (PTT_READ_VEC, thrd_i, (long long) &vmx, 0, 0);
-      else
-	ret = rs6000_ptrace32 (PTT_READ_VEC, thrd_i, (int *) &vmx, 0, 0);
+      ret = ptrace64 (PTT_READ_VEC, thrd_i, (long long) &vmx, 0, 0);
       if (ret < 0)
 	return;
 
       regcache->raw_collect (regno, &(vmx.__vr[0]) + regno
 			     - tdep->ppc_vr0_regnum);
 
-      if (ARCH64 ())
-	ret = rs6000_ptrace64 (PTT_WRITE_VEC, thrd_i, (long long) &vmx, 0, 0);
-      else
-	ret = rs6000_ptrace32 (PTT_WRITE_VEC, thrd_i, (int *) &vmx, 0, 0);
+      ret = ptrace64 (PTT_WRITE_VEC, thrd_i, (long long) &vmx, 0, 0);
       if (ret < 0)
 	perror_with_name (_("Unable to store AltiVec register after reading it"));
     }
@@ -385,22 +326,14 @@ supply_vrregset_aix (struct regcache *regcache, __vmx_context_t *vmx)
 static void
 fetch_altivec_registers_aix (struct regcache *regcache)
 {
-  struct thrdentry64 thrdentry;
   __vmx_context_t vmx;
   pid_t pid = current_inferior ()->pid;
-  tid64_t  thrd_i = 0;
-
-  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
-		 &thrd_i, 1) == 1)
-    thrd_i = thrdentry.ti_tid;
+  tid64_t thrd_i = get_thread_id (pid);
 
   memset(&vmx, 0, sizeof(__vmx_context_t));
   if (__power_vmx() && thrd_i > 0)
     {
-      if (ARCH64 ())
-	rs6000_ptrace64 (PTT_READ_VEC, thrd_i, (long long) &vmx, 0, 0);
-      else
-	rs6000_ptrace32 (PTT_READ_VEC, thrd_i, (int *) &vmx, 0, 0);
+      ptrace64 (PTT_READ_VEC, thrd_i, (long long) &vmx, 0, 0);
       supply_vrregset_aix (regcache, &vmx);
     }
 }
@@ -423,22 +356,14 @@ supply_vsxregset_aix (struct regcache *regcache, __vsx_context_t *vsx)
 static void
 fetch_vsx_registers_aix (struct regcache *regcache)
 {
-  struct thrdentry64 thrdentry;
   __vsx_context_t vsx;
   pid_t pid = current_inferior ()->pid;
-  tid64_t  thrd_i = 0;
-
-  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
-		 &thrd_i, 1) == 1)
-    thrd_i = thrdentry.ti_tid;
+  tid64_t thrd_i = get_thread_id (pid);
 
   memset(&vsx, 0, sizeof(__vsx_context_t));
   if (__power_vsx() && thrd_i > 0)
     {
-      if (ARCH64 ())
-	rs6000_ptrace64 (PTT_READ_VSX, thrd_i, (long long) &vsx, 0, 0);
-      else
-	rs6000_ptrace32 (PTT_READ_VSX, thrd_i, (int *) &vsx, 0, 0);
+      ptrace64 (PTT_READ_VSX, thrd_i, (long long) &vsx, 0, 0);
       supply_vsxregset_aix (regcache, &vsx);
     }
 }
@@ -451,10 +376,7 @@ void rs6000_nat_target::post_startup_inferior (ptid_t ptid)
      with the process ID which can fork () and
      the data parameter [fourth parameter] must be 1.  */
 
-  if (!ARCH64 ())
-    rs6000_ptrace32 (PT_MULTI, ptid.pid(), 0, 1, 0);
-  else
-    rs6000_ptrace64 (PT_MULTI, ptid.pid(), 0, 1, 0);
+  ptrace64 (PT_MULTI, ptid.pid(), 0, 1, 0);
 }
 
 void
@@ -477,10 +399,7 @@ rs6000_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
 
   if (detach_fork && !follow_child)
   {
-    if (ARCH64 ())
-      rs6000_ptrace64 (PT_DETACH, child_ptid.pid (), 0, 0, 0);
-    else
-      rs6000_ptrace32 (PT_DETACH, child_ptid.pid (), 0, 0, 0);
+    ptrace64 (PT_DETACH, child_ptid.pid (), 0, 0, 0);
   }
 }
 
@@ -528,7 +447,23 @@ fetch_register (struct regcache *regcache, int regno)
 
   /* Floating-point registers.  */
   if (isfloat)
-    rs6000_ptrace32 (PT_READ_FPR, pid, addr, nr, 0);
+    {
+      if (ARCH64 ())
+	ptrace64 (PT_READ_FPR, pid, (long long) addr, nr, 0);
+      else
+	{
+	  /* PT_READ_FPR via ptrace64 fails with EIO for a 32-bit inferior.
+	     Use PTT_READ_FPRS (thread-level) instead, which works for both
+	     32-bit and 64-bit inferiors from a 64-bit debugger.  */
+	  double fprs[ppc_num_fprs];
+	  tid64_t tid = get_thread_id (pid);
+	  if (tid > 0)
+	    {
+	      ptrace64 (PTT_READ_FPRS, tid, (long long) fprs, 0, 0);
+	      memcpy (addr, &fprs[nr - FPR0], 8);
+	    }
+	}
+    }
 
   /* Bogus register number.  */
   else if (nr < 0)
@@ -543,31 +478,59 @@ fetch_register (struct regcache *regcache, int regno)
   /* Fixed-point registers.  */
   else
     {
-      if (!ARCH64 ())
-	*addr = rs6000_ptrace32 (PT_READ_GPR, pid, (int *) nr, 0, 0);
-      else
+      if (ARCH64 ())
 	{
-	  /* PT_READ_GPR requires the buffer parameter to point to long long,
-	     even if the register is really only 32 bits.  */
+	  /* PT_READ_GPR via ptrace64 works for 64-bit inferiors.
+	     The buffer parameter must point to a long long area.  */
 	  long long buf;
-	  rs6000_ptrace64 (PT_READ_GPR, pid, nr, 0, &buf);
+	  ptrace64 (PT_READ_GPR, pid, nr, 0, (PTRACE_TYPE_ARG5) &buf);
 	  if (register_size (gdbarch, regno) == 8)
 	    memcpy (addr, &buf, 8);
 	  else
 	    *addr = buf;
 	}
+      else
+	{
+	  /* PT_READ_GPR via ptrace64 fails with EIO for a 32-bit inferior.
+	     Use PTT_READ_GPRS / PTT_READ_SPRS (thread-level) instead.  */
+	  tid64_t tid = get_thread_id (pid);
+	  if (tid > 0)
+	    {
+	      if (nr >= 0 && nr < ppc_num_gprs)
+		{
+		  /* GPR: PTT_READ_GPRS writes all 32 GPRs as unsigned int[32].
+		     Index directly by register number.  */
+		  unsigned int gprs[ppc_num_gprs];
+		  ptrace64 (PTT_READ_GPRS, tid, (long long) gprs, 0, 0);
+		  *addr = gprs[nr];
+		}
+	      else
+		{
+		  /* SPR: PTT_READ_SPRS writes a ptsprs struct.  Map the
+		     register number back to the appropriate field.  */
+		  struct ptsprs sprs;
+		  ptrace64 (PTT_READ_SPRS, tid, (long long) &sprs, 0, 0);
+		  switch (nr)
+		    {
+		    case IAR:   *addr = sprs.pt_iar;   break;
+		    case MSR:   *addr = sprs.pt_msr;   break;
+		    case CR:    *addr = sprs.pt_cr;    break;
+		    case LR:    *addr = sprs.pt_lr;    break;
+		    case CTR:   *addr = sprs.pt_ctr;   break;
+		    case XER:   *addr = sprs.pt_xer;   break;
+		    case MQ:    *addr = sprs.pt_mq;    break;
+		    case FPSCR: *addr = sprs.pt_fpscr; break;
+		    default: return;
+		    }
+		}
+	    }
+	}
     }
 
   if (!errno)
     regcache->raw_supply (regno, (char *) addr);
   else
-    {
-#if 0
-      /* FIXME: this happens 3 times at the start of each 64-bit program.  */
-      perror (_("ptrace read"));
-#endif
-      errno = 0;
-    }
+    errno = 0;
 }
 
 /* Store register REGNO back into the inferior.  */
@@ -602,7 +565,24 @@ store_register (struct regcache *regcache, int regno)
 
   /* Floating-point registers.  */
   if (isfloat)
-    rs6000_ptrace32 (PT_WRITE_FPR, pid, addr, nr, 0);
+    {
+      if (ARCH64 ())
+	ptrace64 (PT_WRITE_FPR, pid, (long long) addr, nr, 0);
+      else
+	{
+	  /* PT_WRITE_FPR via ptrace64 fails with EIO for a 32-bit inferior.
+	     Use PTT_WRITE_FPRS (thread-level) instead: read all FPRs, update
+	     the one register, then write them all back.  */
+	  double fprs[ppc_num_fprs];
+	  tid64_t tid = get_thread_id (pid);
+	  if (tid > 0)
+	    {
+	      ptrace64 (PTT_READ_FPRS, tid, (long long) fprs, 0, 0);
+	      memcpy (&fprs[nr - FPR0], addr, 8);
+	      ptrace64 (PTT_WRITE_FPRS, tid, (long long) fprs, 0, 0);
+	    }
+	}
+    }
 
   /* Bogus register number.  */
   else if (nr < 0)
@@ -616,21 +596,52 @@ store_register (struct regcache *regcache, int regno)
   /* Fixed-point registers.  */
   else
     {
-      /* The PT_WRITE_GPR operation is rather odd.  For 32-bit inferiors,
-	 the register's value is passed by value, but for 64-bit inferiors,
-	 the address of a buffer containing the value is passed.  */
-      if (!ARCH64 ())
-	rs6000_ptrace32 (PT_WRITE_GPR, pid, (int *) nr, *addr, 0);
-      else
+      if (ARCH64 ())
 	{
-	  /* PT_WRITE_GPR requires the buffer parameter to point to an 8-byte
-	     area, even if the register is really only 32 bits.  */
+	  /* PT_WRITE_GPR via ptrace64 works for 64-bit inferiors.
+	     The buffer parameter must point to an 8-byte area.  */
 	  long long buf;
 	  if (register_size (gdbarch, regno) == 8)
 	    memcpy (&buf, addr, 8);
 	  else
 	    buf = *addr;
-	  rs6000_ptrace64 (PT_WRITE_GPR, pid, nr, 0, &buf);
+	  ptrace64 (PT_WRITE_GPR, pid, nr, 0, (PTRACE_TYPE_ARG5) &buf);
+	}
+      else
+	{
+	  /* PT_WRITE_GPR via ptrace64 fails with EIO for a 32-bit inferior.
+	     Use PTT_WRITE_GPRS / PTT_WRITE_SPRS (thread-level) instead.  */
+	  tid64_t tid = get_thread_id (pid);
+	  if (tid > 0)
+	    {
+	      if (nr >= 0 && nr < ppc_num_gprs)
+		{
+		  /* GPR: read all 32, update one, write all back.  */
+		  unsigned int gprs[ppc_num_gprs];
+		  ptrace64 (PTT_READ_GPRS, tid, (long long) gprs, 0, 0);
+		  gprs[nr] = *addr;
+		  ptrace64 (PTT_WRITE_GPRS, tid, (long long) gprs, 0, 0);
+		}
+	      else
+		{
+		  /* SPR: read ptsprs, update one field, write back.  */
+		  struct ptsprs sprs;
+		  ptrace64 (PTT_READ_SPRS, tid, (long long) &sprs, 0, 0);
+		  switch (nr)
+		    {
+		    case IAR:   sprs.pt_iar   = *addr; break;
+		    case MSR:   sprs.pt_msr   = *addr; break;
+		    case CR:    sprs.pt_cr    = *addr; break;
+		    case LR:    sprs.pt_lr    = *addr; break;
+		    case CTR:   sprs.pt_ctr   = *addr; break;
+		    case XER:   sprs.pt_xer   = *addr; break;
+		    case MQ:    sprs.pt_mq    = *addr; break;
+		    case FPSCR: sprs.pt_fpscr = *addr; break;
+		    default: return;
+		    }
+		  ptrace64 (PTT_WRITE_SPRS, tid, (long long) &sprs, 0, 0);
+		}
+	    }
 	}
     }
 
@@ -760,7 +771,6 @@ rs6000_nat_target::xfer_partial (enum target_object object,
 				 ULONGEST *xfered_len)
 {
   pid_t pid = inferior_ptid.pid ();
-  int arch64 = ARCH64 ();
 
   switch (object)
     {
@@ -801,14 +811,8 @@ rs6000_nat_target::xfer_partial (enum target_object object,
 		    < rounded_offset + sizeof (PTRACE_TYPE_RET)))
 	      {
 		/* Need part of initial word -- fetch it.  */
-		if (arch64)
-		  buffer.word = rs6000_ptrace64 (PT_READ_I, pid,
-						 rounded_offset, 0, NULL);
-		else
-		  buffer.word = rs6000_ptrace32 (PT_READ_I, pid,
-						 (int *) (uintptr_t)
-						 rounded_offset,
-						 0, NULL);
+		buffer.word = ptrace64 (PT_READ_I, pid,
+					rounded_offset, 0, NULL);
 	      }
 
 	    /* Copy data to be written over corresponding part of
@@ -817,13 +821,8 @@ rs6000_nat_target::xfer_partial (enum target_object object,
 		    writebuf, partial_len);
 
 	    errno = 0;
-	    if (arch64)
-	      rs6000_ptrace64 (PT_WRITE_D, pid,
-			       rounded_offset, buffer.word, NULL);
-	    else
-	      rs6000_ptrace32 (PT_WRITE_D, pid,
-			       (int *) (uintptr_t) rounded_offset,
-			       buffer.word, NULL);
+	    ptrace64 (PT_WRITE_D, pid,
+		      rounded_offset, buffer.word, NULL);
 	    if (errno)
 	      return TARGET_XFER_EOF;
 	  }
@@ -831,13 +830,8 @@ rs6000_nat_target::xfer_partial (enum target_object object,
 	if (readbuf)
 	  {
 	    errno = 0;
-	    if (arch64)
-	      buffer.word = rs6000_ptrace64 (PT_READ_I, pid,
-					     rounded_offset, 0, NULL);
-	    else
-	      buffer.word = rs6000_ptrace32 (PT_READ_I, pid,
-					     (int *)(uintptr_t)rounded_offset,
-					     0, NULL);
+	    buffer.word = ptrace64 (PT_READ_I, pid,
+				    rounded_offset, 0, NULL);
 	    if (errno)
 	      return TARGET_XFER_EOF;
 
@@ -1014,12 +1008,8 @@ rs6000_ptrace_ldinfo (ptid_t ptid)
 
   while (1)
     {
-      if (ARCH64 ())
-	rc = rs6000_ptrace64 (PT_LDINFO, pid, (unsigned long) ldi.data (),
-			      ldi.size (), NULL);
-      else
-	rc = rs6000_ptrace32 (PT_LDINFO, pid, (int *) ldi.data (),
-			      ldi.size (), NULL);
+      rc = ptrace64 (PT_LDINFO, pid, (unsigned long) ldi.data (),
+		     ldi.size (), NULL);
 
       if (rc != -1)
 	break; /* Success, we got the entire ld_info data.  */
-- 
2.51.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4 2/4] Remove stale legacy code from rs6000-aix-nat.c file.
  2026-10-05 12:16 [PATCH v4 2/4] Remove stale legacy code from rs6000-aix-nat.c file Aditya Vidyadhar Kamath
@ 2026-10-05 15:02 ` Simon Marchi
  2026-10-08  8:57   ` Aditya Kamath
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2026-10-05 15:02 UTC (permalink / raw)
  To: Aditya Vidyadhar Kamath, ulrich.weigand, tom
  Cc: gdb-patches, Aditya.Kamath1, sangamesh.swamy

On 10/5/26 8:16 AM, Aditya Vidyadhar Kamath wrote:
> From: Aditya Vidyadhar Kamath <aditya.kamath1@ibm.com>
> 
> GDB on AIX is built in 64 bit mode only.
> 
> As per
>  https://www.ibm.com/docs/en/aix/7.2.0?topic=p-ptrace-ptracex-ptrace64-subroutine
> 
> though ptrace64 will also support 32-bit debugees if debugee is a 32 bit process,
> the options that refer to GPRs or SPRs fail with errno = EIO.

The existing code does this:

      if (!ARCH64 ())
	*addr = rs6000_ptrace32 (PT_READ_GPR, pid, (int *) nr, 0, 0);

So a 64-bit GDB debugging a 32-bit inferior, before this patch, would go
into rs6000_ptrace32, which has:

  #ifdef HAVE_PTRACE64
    int ret = ptrace64 (req, id, (uintptr_t) addr, data, buf);
  #else
    int ret = ptrace (req, id, (int *)addr, data, buf);
  #endif

So today it would get PT_READ_GPR using ptrace64, right?  Does it
already fail today?

It would be clear that up first.  We don't want this patch to mix
removing legacy code and bug fixing.

Note that while it uses ptrace64 in both cases, it uses it differently.
When reading a GPR from a 64-bit process, the output is through an
8-byte buffer passed as an out parameter (the 5th parameter).  When
reading a GPR from a 32-bit process, the output is the return value.

For the record, I asked Claude to test ptrace64 against both a 64-bit
and a 32-bit debuggee, and it appears to work in both cases.  But this
is on AIX 7.3 (7300-01-02-2320), so maybe the behavior is different on
7.2.


> This is something I have left in the comments of the code as well, so we do not miss
> it while testing.
> 
> When I built GDB as a 64 bit AIX application [Using ptrace () for 32 bit inferior debugee]
>  to just test, linking fails with:
> 
> CXXLD  gdb
>   ld: 0711-317 ERROR: Undefined symbol: .ptrace
>   ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
>   collect2: error: ld returned 8 exit status
>   gmake[2]: *** [Makefile:2367: gdb] Error 1

It's strange that this happens, since the doc clearly states that you
can use ptrace in 64-bit applications.

> 
> This clearly shows from AIX 7.2, the standard ptrace system call symbol is not
> exported in 64-bit mode; 64 bit mode must use ptrace64 exclusively which is what
> this patch does.
> 
> As document mentioned,
> ptrace: 32- or 64-bit GDB, 32-bit inferior
> ptracex: 32-bit GDB, 64-bit inferior
> ptrace64: 64-bit GDB, 64-bit inferior
> 
> This is right,

This sentence is incomplete.

> 
> Legacy rs6000_ptrace32 and rs6000_ptrace64 wrappers conditionally called
> ptrace/ptracex/ptrace64 and relied on ARCH3264 macro. This macro is now not needed.
> We will only use ptrace64 going forward.
> 
> This patch modernises rs6000-aix-nat.c for AIX 7.2+/7.3+
> 
> Here are my thoughts during implementation.
> 1: Remove rs6000_ptrace32 () and rs6000_ptrace64 () completey. We will only use
>    ptrace64 for all operations. No ptracex/ptrace any more since GDB will not be
>    built in 32 bit mode.
> 2: Wrote get_thread_id () helper to obtain kernel thread ID via getthrds64 ()
> 3: For a 32 bit debugee where process-level PT_READ_GPR, PT_WRITE_GPR and
>    PT_READ_FPR/PT_WRITE_FPR fail with EIO when called from 64 bit GDB via ptrace64 ()
>    let us use thread-level PTT_READ_GPRS/PTT_WRITE_GPRS and PTT_READ_SPRS/PTT_WRITE_SPRS.
>    Likewise, PTT_READ_FPRS/PTT_WRITE_FPRS.



> 
> I also ran tests for for both 64 bit and 32 bit mode. First result is 64 bit mode. Second summary is 32 bit. Ran the command in gdb/testsuite directory.
> Command used: gmake check RUNTESTFLAGS='--directory=gdb.base CC_FOR_TARGET="/opt/freeware/bin/gcc" CXX_FOR_TARGET="/opt/freeware/bin/g++" CXXFLAGS_FOR_TARGET="-O0 -w -g -gdwarf -maix64" CFLAGS_FOR_TARGET="-O0 -w -g -gdwarf -maix64"'
> 
> Before this patch:
> === gdb Summary ===
> # of unexpected core files      1
> # of expected passes            41792
> # of unexpected failures        2346
> # of unexpected successes       1
> # of expected failures          17
> # of known failures             30
> # of unresolved testcases       30
> # of untested testcases         104
> # of unsupported tests          179
> # of paths in test names        7
> # of duplicate test names       2
> 
> --------------------------------------------------
>                 === gdb Summary ===
> 
> # of unexpected core files      1
> # of expected passes            41687
> # of unexpected failures        2302
> # of unexpected successes       1
> # of expected failures          15
> # of known failures             30
> # of unresolved testcases       11
> # of untested testcases         107
> # of unsupported tests          179
> # of paths in test names        7
> # of duplicate test names       2
> 
> After this patch:
> === gdb Summary ===
> 
> # of unexpected core files      1
> # of expected passes            41617
> # of unexpected failures        2297
> # of unexpected successes       1
> # of expected failures          15
> # of known failures             30
> # of unresolved testcases       10
> # of untested testcases         108
> # of unsupported tests          179
> # of paths in test names        7
> # of duplicate test names       2
> 
> ------------------------------------
>                 === gdb Summary ===
> 
> # of unexpected core files      1
> # of expected passes            41796
> # of unexpected failures        2346
> # of unexpected successes       1
> # of expected failures          17
> # of known failures             30
> # of unresolved testcases       31
> # of untested testcases         105
> # of unsupported tests          179
> # of paths in test names        7
> # of duplicate test names       2

Not sure what to think of those test results, it seems like some numbers
got worse.  Are you sure the 64 and 32 bit summaries are not swapped in
one of those?

Simon

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH v4 2/4] Remove stale legacy code from rs6000-aix-nat.c file.
  2026-10-05 15:02 ` Simon Marchi
@ 2026-10-08  8:57   ` Aditya Kamath
  0 siblings, 0 replies; 3+ messages in thread
From: Aditya Kamath @ 2026-10-08  8:57 UTC (permalink / raw)
  To: Simon Marchi, Aditya Vidyadhar Kamath, Ulrich Weigand, tom
  Cc: gdb-patches, SANGAMESH MALLAYYA

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

Hi Ulrich and Simon,

Here is my answer to our questions for the discussion. Let me know your thoughts.

>The existing code does this:
>      if (!ARCH64 ())
>        *addr = rs6000_ptrace32 (PT_READ_GPR, pid, (int *) nr, 0, 0);
>So a 64-bit GDB debugging a 32-bit inferior, before this patch, would go
>into rs6000_ptrace32, which has:
>#ifdef HAVE_PTRACE64
>   int ret = ptrace64 (req, id, (uintptr_t) addr, data, buf);
>  #else
>    int ret = ptrace (req, id, (int *)addr, data, buf);
>  #endif
>So today it would get PT_READ_GPR using ptrace64, right?  Does it
>already fail today?

>It would be clear that up first.  We don't want this patch to mix
>removing legacy code and bug fixing.

>Note that while it uses ptrace64 in both cases, it uses it differently.
>When reading a GPR from a 64-bit process, the output is through an
>8-byte buffer passed as an out parameter (the 5th parameter).  When
>reading a GPR from a 32-bit process, the output is the return value.

>For the record, I asked Claude to test ptrace64 against both a 64-bit
>and a 32-bit debuggee, and it appears to work in both cases.  But this
>is on AIX 7.3 (7300-01-02-2320), so maybe the behavior is different on
>7.2.

I checked this on both AIX 7.2 and AIX 7.3. PT_READ_GPR, PT_READ_FPR, PT_WRITE_GPR and PT_WRITE_FPR all work correctly when a 32-bit inferior is attached to a 64-bit GDB using ptrace64. I did not observe any EIO failures.


I also discussed the documentation with the AIX kernel maintainers. They clarified that the documented EIO behaviour only applies to certain memory-access and breakpoint-related ptrace operations when using a genuine 64-bit address (addrhi != 0). It does not apply to the GPR/FPR operations above. They also mentioned that the documentation will be updated accordingly.


Based on these findings, we can continue using ptrace64 for both 32-bit and 64-bit inferiors. The assumptions behind my current four patches are therefore no longer valid, and I will rework the series accordingly. While looking through the code I also noticed a fair amount of redundant logic in both aix-thread.c and rs6000-aix-nat.c. Would you prefer that I first complete the cleanup work and then submit any refactoring separately, or would it be reasonable to combine some of those simplifications as part of this effort?

>> When I built GDB as a 64 bit AIX application [Using ptrace () for 32 bit inferior debugee]
>>  to just test, linking fails with:
>>
>> CXXLD  gdb
>>   ld: 0711-317 ERROR: Undefined symbol: .ptrace
>>   ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
>>   collect2: error: ld returned 8 exit status
>>   gmake[2]: *** [Makefile:2367: gdb] Error 1
>>It's strange that this happens, since the doc clearly states that you
>>can use ptrace in 64-bit applications.


On my systems, both ptrace() and ptracex() are unavailable when linking a 64-bit GDB build.


It is mentioned like this in document


The ptrace and ptracex subroutines are not supported in 64-bit mode.


https://www.ibm.com/docs/en/aix/7.2.0?topic=p-ptrace-ptracex-ptrace64-subroutine

>>                 === gdb Summary ===
>>
>> # of unexpected core files      1
>> # of expected passes            41796
>> # of unexpected failures        2346
>> # of unexpected successes       1
>> # of expected failures          17
>> # of known failures             30
>> # of unresolved testcases       31
>> # of untested testcases         105
>> # of unsupported tests          179
>> # of paths in test names        7
>> # of duplicate test names       2

>Not sure what to think of those test results, it seems like some numbers
>got worse.  Are you sure the 64 and 32 bit summaries are not swapped in
>one of those?

Yes, the numbers are correct.

Have a nice day ahead.

Thanks and regards,
Aditya.

[-- Attachment #2: Type: text/html, Size: 10583 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-08  8:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-05 12:16 [PATCH v4 2/4] Remove stale legacy code from rs6000-aix-nat.c file Aditya Vidyadhar Kamath
2026-10-05 15:02 ` Simon Marchi
2026-10-08  8:57   ` Aditya Kamath

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox