Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: Aditya Vidyadhar Kamath <akamath996@gmail.com>,
	ulrich.weigand@de.ibm.com,  tom@tromey.com
Cc: gdb-patches@sourceware.org, Aditya.Kamath1@ibm.com,
	sangamesh.swamy@in.ibm.com
Subject: Re: [PATCH v4 2/4] Remove stale legacy code from rs6000-aix-nat.c file.
Date: Mon, 5 Oct 2026 11:02:23 -0400	[thread overview]
Message-ID: <ccb802e1-2aa8-4cea-a6c1-1171f226e538@polymtl.ca> (raw)
In-Reply-To: <20261005121609.31787-2-akamath996@gmail.com>

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

  reply	other threads:[~2026-10-05 15:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-05 12:16 Aditya Vidyadhar Kamath
2026-10-05 15:02 ` Simon Marchi [this message]
2026-10-08  8:57   ` Aditya Kamath

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ccb802e1-2aa8-4cea-a6c1-1171f226e538@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=Aditya.Kamath1@ibm.com \
    --cc=akamath996@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=sangamesh.swamy@in.ibm.com \
    --cc=tom@tromey.com \
    --cc=ulrich.weigand@de.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox