* Re: [PATCH v4 3/4] Remove obsolete getthrds and HAVE_PTRACE64 configure checks.
2026-10-05 12:38 [PATCH v4 3/4] Remove obsolete getthrds and HAVE_PTRACE64 configure checks Aditya Vidyadhar Kamath
@ 2026-10-09 11:32 ` Andrew Burgess
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Burgess @ 2026-10-09 11:32 UTC (permalink / raw)
To: Aditya Vidyadhar Kamath, ulrich.weigand, simon.marchi, tom
Cc: gdb-patches, Aditya.Kamath1, sangamesh.swamy, Aditya Vidyadhar Kamath
Aditya Vidyadhar Kamath <akamath996@gmail.com> writes:
> From: Aditya Vidyadhar Kamath <aditya.kamath1@ibm.com>
>
> AIX 7.2 is the baseline supported AIX release. On all AIX supported releases,
> getthrds is there <procinfo.h> declared, and ptrace64 is in <sys/ptrace.h>
> declared and defined in libc.a.
>
> This patch removes the obsolete configure checks and cleans up.
>
> We are doing 2 things:
> 1: Remove AC_CHECK_DECLS (getthrds) in configure.ac and config.in.
> 2: Remove obsolete #ifdef HAVE_PTRACE64 in gdb/nat/gdb_ptrace.h
> and use ptrace64 directly under PTRACE_TYPE_ARG5.
I ran this through an AI/LLV review agent which raised the following 3
points:
Issue 1: addr_ptr is undefined after the patch
The patch removes the #define addr_ptr macros (both the long long and int * variants, lines 370–376), but addr_ptr is still used in two places that are NOT modified by the patch:
gdb/aix-thread.c:379: ptrace32 (int req, int id, addr_ptr addr, int data, int *buf)
gdb/aix-thread.c:1120: ptrace32 (PTT_CONTINUE, tid[0], (addr_ptr) 1, ...)
This will cause a build failure on AIX because addr_ptr is no longer defined as either long long or int *.
Issue 2: 5-argument ptrace macro removed, but ptrace32() still calls ptrace() with 5 arguments
The patch removes the 5-argument #define ptrace(request, pid, addr,
data, buf) macro (lines 370–376). After this removal, the only ptrace
macro in scope comes from gdb/nat/gdb_ptrace.h, which defines ptrace
as a 4-argument macro:
#define ptrace(request, pid, addr, data) \
ptrace64 (request, pid, addr, data, 0)
However, ptrace32() at line 383 still calls ptrace (req, id, addr,
data, buf) with 5 arguments. This will cause a preprocessor error
(wrong number of macro arguments) or at least a compilation error.
Issue 3: rs6000-aix-nat.c still uses HAVE_PTRACE64
rs6000-aix-nat.c uses #ifdef HAVE_PTRACE64 at lines 254 and 272. The
patch removes HAVE_PTRACE64 from gdb/config.in but does NOT remove the
AC_CHECK_FUNCS for ptrace64 from gdbsupport/common.m4:74. This means:
If gdbsupport/common.m4 is not also updated, HAVE_PTRACE64 is still defined by the configure system, and the #undef removal from config.in is wrong (it will be regenerated with HAVE_PTRACE64 back).
If the intent is to remove HAVE_PTRACE64 entirely, then rs6000-aix-nat.c must also be updated, and gdbsupport/common.m4 must have ptrace64 removed from the AC_CHECK_FUNCS list.
Either way, the current patch is inconsistent.
For issue #3 I tried on an x86-64 GNU/Linux machine running autoheader
in the binutils-gdb/gdb/ directory, and indeed HAVE_PTRACE64 as added
back, so it does look like there is an inconsistency there.
To validate issues #1 and #2 I'd need to try building on an AIX machine,
which I assume is working for you, so maybe you can extend the commit
message to talk about these issues, e.g. "Remove the addr_ptr define,
GDB now picks up the definition from ....". And similarly for the
ptrace macro: "GDB finds the 5 argument ptrace macro ....".
Though issues #1 and #2 might be obvious for someone familiar with AIX,
I do find the questions raised to be good ones, and even if the AI's
diagnosis of these as bugs is wrong, I think it would be worth
explaining these better in the commit message.
> ---
> gdb/aix-thread.c | 33 ++-------------------------------
> gdb/config.in | 7 -------
> gdb/configure.ac | 5 -----
If you change configure.ac then you need to regenerate the configure
file. This, combined with issue #3 above, makes me thing you are hand
editing config.in, which is the wrong approach. As the comment at the
top of that file says, config.in is auto-generated.
You'll need automake 1.15.1 and autoconf 2.69, then in the gdb/
directory just run `autoreconf`.
You probably should also do this in the gdbserver/ directory once issue
#3 is addressed.
> gdb/nat/gdb_ptrace.h | 13 ++++---------
> 4 files changed, 6 insertions(+), 52 deletions(-)
>
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 65b9c5f7479..95aa18985ed 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -1,4 +1,4 @@
> -/* Low level interface for debugging AIX 4.3+ pthreads.
> +/* Low level interface for debugging AIX 7.2+ pthreads.
>
> Copyright (C) 1999-2026 Free Software Foundation, Inc.
> Written by Nick Duffek <nsd@redhat.com>.
> @@ -19,7 +19,7 @@
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
>
> -/* This module uses the libpthdebug.a library provided by AIX 4.3+ for
> +/* This module uses the libpthdebug.a library provided by AIX 7.2+ for
> debugging pthread applications.
>
> Some name prefix conventions:
> @@ -56,18 +56,9 @@
> #include <sys/pthdebug.h>
> #include <unordered_set>
>
> -#if !HAVE_DECL_GETTHRDS
> -extern int getthrds (pid_t, struct thrdsinfo64 *, int, tid_t *, int);
> -#endif
> -
> /* Whether to emit debugging output. */
> static bool debug_aix_thread;
>
> -/* In AIX 5.1, functions use pthdb_tid_t instead of tid_t. */
> -#ifndef PTHDB_VERSION_3
> -#define pthdb_tid_t tid_t
> -#endif
> -
> /* Success and failure values returned by pthdb callbacks. */
>
> #define PDC_SUCCESS PTHDB_SUCCESS
> @@ -315,15 +306,6 @@ ptrace_check (int req, int id, int ret)
> return 0; /* Not reached. */
> }
>
> -/* Call ptracex (REQ, ID, ADDR, DATA, BUF) or
> - ptrace64 (REQ, ID, ADDR, DATA, BUF) if HAVE_PTRACE64.
> - Return success. */
> -
> -#ifdef HAVE_PTRACE64
> -# define ptracex(request, pid, addr, data, buf) \
> - ptrace64 (request, pid, addr, data, buf)
> -#endif
> -
> static int
> ptrace64aix (int req, int id, long long addr, int data, int *buf)
> {
> @@ -331,17 +313,6 @@ ptrace64aix (int req, int id, long long addr, int data, int *buf)
> return ptrace_check (req, id, ptracex (req, id, addr, data, buf));
> }
>
> -/* Call ptrace (REQ, ID, ADDR, DATA, BUF) or
> - ptrace64 (REQ, ID, ADDR, DATA, BUF) if HAVE_PTRACE64.
> - Return success. */
> -
> -#ifdef HAVE_PTRACE64
> -# define ptrace(request, pid, addr, data, buf) \
> - ptrace64 (request, pid, addr, data, buf)
> -# define addr_ptr long long
> -#else
> -# define addr_ptr int *
> -#endif
>
> static int
> ptrace32 (int req, int id, addr_ptr addr, int data, int *buf)
The change above leaves 2 blank lines between the end of ptrace64aix and
ptrace32, just a single blank line is the normal GDB style.
> diff --git a/gdb/config.in b/gdb/config.in
> index 0e5a6637c2b..dab7a3e7239 100644
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -147,10 +147,6 @@
> /* Define to 1 if you have the declaration of `ffs', and to 0 if you don't. */
> #undef HAVE_DECL_FFS
>
> -/* Define to 1 if you have the declaration of `getthrds', and to 0 if you
> - don't. */
> -#undef HAVE_DECL_GETTHRDS
> -
> /* Define to 1 if you have the declaration of `snprintf', and to 0 if you
> don't. */
> #undef HAVE_DECL_SNPRINTF
> @@ -397,9 +393,6 @@
> /* Define to 1 if you have the `pthread_sigmask' function. */
> #undef HAVE_PTHREAD_SIGMASK
>
> -/* Define to 1 if you have the `ptrace64' function. */
> -#undef HAVE_PTRACE64
> -
> /* Define if sys/ptrace.h defines the PTRACE_GETFPXREGS request. */
> #undef HAVE_PTRACE_GETFPXREGS
>
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 0d88a280b16..684ce8e21fb 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -1915,11 +1915,6 @@ if test "${build}" = "${host}" -a "${host}" = "${target}" ; then
> CONFIG_SRCS="${CONFIG_SRCS} aix-thread.c"
> CONFIG_OBS="${CONFIG_OBS} aix-thread.o"
> LIBS="$LIBS -lpthdebug"
> -
> - # Older versions of AIX do not provide the declaration for
> - # the getthrds function (it appears that it was introduced
> - # with AIX 6.x).
> - AC_CHECK_DECLS(getthrds, [], [], [[#include <procinfo.h>]])
> fi
> ;;
> esac
> diff --git a/gdb/nat/gdb_ptrace.h b/gdb/nat/gdb_ptrace.h
> index e0607164260..0d06cb8eb55 100644
> --- a/gdb/nat/gdb_ptrace.h
> +++ b/gdb/nat/gdb_ptrace.h
> @@ -127,15 +127,10 @@
> zero. */
>
> #ifdef PTRACE_TYPE_ARG5
> -# ifdef HAVE_PTRACE64
> -# define ptrace(request, pid, addr, data) \
> - ptrace64 (request, pid, addr, data, 0)
> -# undef PTRACE_TYPE_ARG3
> -# define PTRACE_TYPE_ARG3 long long
> -# else
> -# define ptrace(request, pid, addr, data) \
> - ptrace (request, pid, addr, data, 0)
> -# endif
> +# define ptrace(request, pid, addr, data) \
> + ptrace64 (request, pid, addr, data, 0)
> +# undef PTRACE_TYPE_ARG3
> +# define PTRACE_TYPE_ARG3 long long
> #else
> /* Wrapper that avoids adding a pointless cast to all callers. */
> # define ptrace(request, pid, addr, data) \
The comment block right about this talks about AIX and HP-UX, but
support for GDB was removed some time ago now. As this is a cleanup
commit, it would be nice to update this comment to remove the HP-UX
references.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 2+ messages in thread