Re: [PATCH 3/5] powerpc64-aix inf-ptrace patch

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> From: Raunaq 12 <raunaq12@in.ibm.com>
> Date: Mon, 15 Jul 2013 13:00:35 +0530
> 
> Hi Mark/Tom,
> 
> Thanks for the feedback.
> 
> I removed the newly added file and instead used a wrapper function and
> macros.
> Kindly review the patch below and let me know if this should be okay ?

Not quite.

> ChangeLog :-
> 
> * inf-ptrace.c: Add support for ptrace64 in 64 BIT mode
> (inf_ptrace_follow_fork): Likewise
> (inf_ptrace_me): Likewise
> (inf_ptrace_post_startup_inferior): Likewise
> (inf_ptrace_attach): Likewise
> (inf_ptrace_post_attach): Likewise
> (inf_ptrace_detach): Likewise
> (inf_ptrace_kill): Likewise
> (inf_ptrace_resume): Likewise
> (inf_ptrace_wait): Likewise
> (inf_ptrace_xfer_partial): Likewise
> (inf_ptrace_fetch_register): Likewise
> (inf_ptrace_store_register): Likewise
> (inf_ptrace_fetch_register): Likewise
> * rs6000-nat.c: Check for __ld_info64 if compiling 64 BIT gdb.
> (rs6000_ptrace32): Added ptrace64 support for 64 bit mode.
> (rs6000_ptrace64): Likewise.
> * gdb_ptrace.h: Add macro for ptrace64 call.
> ---
> Index: ./gdb/inf-ptrace.c
> ===================================================================
> --- ./gdb/inf-ptrace.c
> +++ ./gdb/inf-ptrace.c
> @@ -36,6 +36,17 @@
>  #include "gdbthread.h"
> 
> ^L
> +#ifdef BFD64
> +#define check_ptrace ptrace64
> +#define addr_ptr long long
> +#define pid_type long long
> +#define addr_uintptr_t long long
> +#else
> +#define check_ptrace ptrace
> +#define addr_ptr PTRACE_TYPE_ARG3
> +#define pid_type int
> +#define addr_uintptr_t uintptr_t
> +#endif

This is the wrong place to put this stuff.  Take a look at
gdb_ptrace.h; it already has code to deal with AIX.  You just need to
adjust that to use ptrace64 if available.  Something like:

#ifdef HAVE_TYPE_ARG5
# ifdef HAVE_PTRACE64
#  define ptrace(request, pid, addr, data) \
        ptrace64 (request, pid, addr, data, 0) 
# else
#  define ptrace(request, pid, addr, data) \
        ptrace (request, pid, addr, data, 0)
# endif
#endif

You can do any casting that's really necessary there.  The (pid_type)
casts are almost certainly not necessary.

The existing (uintptr_t) casts might appear to be a problem if you
want to have a 32-bit gdb that supports debugging 64-bit processes.
However, since xfer_partial(), fetch_registers() and store_registers()
get overridden by rs6000-nat.c that shouldn't be an issue.

So unless I'm missing something, you should not need to make any
changes to inf-ptrace.c itself.

Cheers,

Mark