Re: [PATCH v3] Remove stale pre-AIX-7.2 compatibility guards

[Simon Marchi <simon.marchi@polymtl.ca>]

On 9/22/26 12:52 AM, Aditya Vidyadhar Kamath wrote:
> From: Aditya Kamath <Aditya.Kamath1@ibm.com>
> 
> GDB now requires AIX 7.2 as the minimum supported version and will
> support AIX 7.2 TL5, AIX 7.3 and upcoming AIX releases.  Remove
> dead compatibility code that existed only for older releases.
> 
> Remove the HAVE_DECL_GETTHRDS configure check since aix-thread.c was the last
> user of that macro.  Merge ptrace64aix and ptrace32 into a single
> ptrace_aix function since they became identical after this
> cleanup, and collapse all call sites that branched on arch64 just to
> pick between the two.  Also clean up rs6000-aix-nat.c by removing the
> ARCH3264 and HAVE_PTRACE64 guards along with the ptracex fallback in
> rs6000_ptrace32 and rs6000_ptrace64, which are now simple wrappers
> around ptrace64.
> 
> Also as per https://www.ibm.com/docs/en/aix/7.2.0?topic=p-ptrace-ptracex-ptrace64-subroutine
> ptrace64 will also support 32-bit debugees.
> 
> In store_regs_user_thread we use ppc_vsr0_upper_regnum when checking validity
> and collecting the VSX upper-doubleword registers.
> 
> In store_regs_user_thread guard ctx.fpscr with ppc_fpscr_regnum instead of ppc_xer_regnum, and
> add the ppc_fpscr_regnum >= 0 check to match the 64-bit path.
> Use ppc_num_gprs instead of ppc_num_fprs in the GPR regno range check.

Sorry, I missed that last bit when reading the first time.  Are those
changed related to removing stale pre-AIX 7.2 code?  Or are they fixes
on their own?  If it's the latter, that should be a separate patch.

I also forgot, but I asked Claude to review this patch, it raised some
good points.  There is this one that sounds important, but I have no way
of checking if it's true:

  - The 32-bit SPR fix is only half done. If the GPR layout problem is
    being fixed in pdc_read_regs, the SPRs have the same problem. memcpy
    (&context->msr,   &sprs32, sizeof (sprs32)) (gdb/aix-thread.c:449)
    copies a struct ptsprs (32-bit fields) over the 64-bit fields of
    pthdb_context_t, and consumers read them field by field
    (supply_sprs32 (regcache, ctx.iar, ctx.msr, ...) at
    gdb/aix-thread.c:1236). pdc_write_regs has the reverse problem: it
    passes &context->msr to PTT_WRITE_SPRS for a 32-bit inferior
    (gdb/aix-thread.c:517). Either fix both in the same (separate)
    commit as the GPRs, or leave it all  for later, but don't do only
    half.

It also pointed out that there is a use of HAVE_PTRACE64 that can be
removed in nat/gdb_ptrace.h.

I wrote some more inline below.

> @@ -470,14 +437,14 @@ pdc_read_regs (pthdb_user_t user_current_pid,
>      {
>        if (data->arch64)
>  	{
> -	  if (!ptrace64aix (PTT_READ_SPRS, tid,
> +	  if (!ptraceaix (PTT_READ_SPRS, tid,
>  			    (unsigned long) &sprs64, 0, NULL))

The indent needs to be adjusted here.

> @@ -1357,14 +1286,14 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
>      {
>        if (data->arch64)
>  	{
> -	  if (!ptrace64aix (PTT_READ_GPRS, tid,
> +	  if (!ptraceaix (PTT_READ_GPRS, tid,
>  			    (unsigned long) gprs64, 0, NULL))

Indent needs to be fixed.

> @@ -1377,9 +1306,9 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
>        int ret = 0;
>        __vmx_context_t vmx;
>        if (data->arch64)
> -	ret = ptrace64aix (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
> +	ret = ptraceaix (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
>        else
> -	ret = ptrace32 (PTT_READ_VEC, tid, (uintptr_t) &vmx, 0, 0);
> +	ret = ptraceaix (PTT_READ_VEC, tid, (uintptr_t) &vmx, 0, 0);

Do we still need two separate ptraceaix calls here?

> @@ -1394,9 +1323,9 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
>        __vsx_context_t vsx;
>        int ret = 0;
>        if (data->arch64)
> -	ret = ptrace64aix (PTT_READ_VSX, tid, (long long) &vsx, 0, 0);
> +	ret = ptraceaix (PTT_READ_VSX, tid, (long long) &vsx, 0, 0);
>        else
> -	ret = ptrace32 (PTT_READ_VSX, tid, (long long) &vsx, 0, 0);
> +	ret = ptraceaix (PTT_READ_VSX, tid, (uintptr_t) &vsx, 0, 0);

And here?

> @@ -1421,7 +1350,7 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
>      {
>        if (data->arch64)
>  	{
> -	  if (!ptrace64aix (PTT_READ_SPRS, tid,
> +	  if (!ptraceaix (PTT_READ_SPRS, tid,
>  			    (unsigned long) &sprs64, 0, NULL))

Indent.

> @@ -1678,9 +1607,9 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
>      {
>        memset(&vsx, 0, sizeof(__vsx_context_t));
>        for (i = 0; i < ppc_num_vshrs; i++)
> -	if (REG_VALID == regcache->get_register_status (tdep->ppc_vsr0_regnum + i))
> +	if (REG_VALID == regcache->get_register_status (tdep->ppc_vsr0_upper_regnum + i))

This line is now too long.

> @@ -1869,16 +1781,16 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
>  	if (__power_vmx())
>  	  {
>  	    if (data->arch64)
> -	      ret = ptrace64aix (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
> +	      ret = ptraceaix (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
>  	    else
> -	      ret = ptrace32 (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
> +	      ret = ptraceaix (PTT_READ_VEC, tid, (uintptr_t) &vmx, 0, 0);

And here?

> @@ -246,42 +231,20 @@ regmap (struct gdbarch *gdbarch, int regno, int *isfloat)
>      return -1;
>  }
>  
> -/* Call ptrace(REQ, ID, ADDR, DATA, BUF).  */
> +/* Call ptrace64(REQ, ID, ADDR, DATA, BUF).  */
>  
>  static int
>  rs6000_ptrace32 (int req, int id, int *addr, int data, int *buf)
>  {
> -#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;
> +  return ptrace64 (req, id, (uintptr_t) addr, data, buf);
>  }

Can the rs6000_ptrace32 and rs6000_ptrace64 wrappers be removed?

Simon