Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.

Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.

[Raunaq 12]

David Edelsohn <dje.gcc@gmail.com> wrote on 08/09/2013 09:23:42 PM:

> From: David Edelsohn <dje.gcc@gmail.com>
> To: Raunaq 12/India/IBM@IBMIN, Mark Kettenis <mark.kettenis@xs4all.nl>,
Ulrich Weigand
> <Ulrich.Weigand@de.ibm.com>
> Cc: gdb-patches@sourceware.org
> Date: 08/09/2013 09:22 PM
> Subject: Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.
>
> > When GDB is compiled in 64 BIT mode ptrace and ptracex calls are not
defined on AIX.
> > So, instead we check if ptrace64 is defined, if it is then we call that
instead of ptrace/x.
>
> Have you tested the patch in both 32 bit mode and 64 bit mode for both
host GDB and target
> debugee?

Yes, ran the tests in testsuite/gdb.base and any new failures did not arise
when

>I tried the earlier version of this patch from last year and experienced
problems
> debugging in 32 bit mode (I no longer could debug GCC).

The earlier version of this patch was using ptrace64 only if BFD64 was
defined,
i.e. when GDB was built in 64 BIT mode.
Did you build 32 bit gdb?
If so, according to the previous version of this patch,
no ptrace related calls were changed as ptrace64 was only
called if BFD64 was defined. Could you please resend the error you got
while debugging
32 bit GCC to my mail id? I can look into it to see where the problem is
coming from.

> ptrace, ptracex and ptrace64 are defined in syscalls.exp.
>
> ptrace64                syscall3264
> # 32
> ptrace                  syscall32
> ptracex                 syscall32

> As you wrote, ptrace and ptracex are 32 bit only API; ptrace64 is
available as 32 bit and 64 bit
> API. ptrace64 is suppose to support 32 bit debugee targets, but I
experienced problems.
>
> ptrace64 will be discovered as supported by configure when building 32
bit GDB.

> Is your intention to always use ptrace64 when it is available or only to
use it for GDB hosted on
> 64 bit mode?

My initial intention was to use ptrace64 only if ptrace64 was available and
GDB was built in 64 BIT. But after a few discussions over this, I decided
to use ptrace64 when ever the API is defined.

So, this present patch would use
ptrace64 even for a 32 bit build as you rightly noticed.
This did not change the test results for a 32 bit build of GDB.

Thanks,
Raunaq M. Bathija

Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.

[David Edelsohn]

On Mon, Aug 12, 2013 at 8:00 AM, Raunaq 12 <raunaq12@in.ibm.com> wrote:

> The earlier version of this patch was using ptrace64 only if BFD64 was
> defined,
> i.e. when GDB was built in 64 BIT mode.
> Did you build 32 bit gdb?
> If so, according to the previous version of this patch,
> no ptrace related calls were changed as ptrace64 was only
> called if BFD64 was defined. Could you please resend the error you got
> while debugging
> 32 bit GCC to my mail id? I can look into it to see where the problem is
> coming from.

I have not tried to build GDB with the latest test of patches.  When I
used the earlier patches to build GDB in 32 bit mode but with
--enable-64-bit-bfd (because I want to debug 64 bit applications), and
tried to use the debugger with GCC, I encountered the problem that no
function call argument values were available.

Thanks, David

Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.

[Raunaq 12]

[ sending again without the line wrapping that makes it so hard to read]

David Edelsohn <dje.gcc@gmail.com> wrote on 08/09/2013 09:23:42 PM:

> From: David Edelsohn <dje.gcc@gmail.com>
> To: Raunaq 12/India/IBM@IBMIN, Mark Kettenis <mark.kettenis@xs4all.nl>, Ulrich Weigand
> <Ulrich.Weigand@de.ibm.com>
> Cc: gdb-patches@sourceware.org
> Date: 08/09/2013 09:22 PM
> Subject: Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.
>
> > When GDB is compiled in 64 BIT mode ptrace and ptracex calls are not defined on AIX.
> > So, instead we check if ptrace64 is defined, if it is then we call that instead of ptrace/x.
>
> Have you tested the patch in both 32 bit mode and 64 bit mode for both host GDB and target
> debugee?

Yes, ran the tests in testsuite/gdb.base and any new failures did not arise when

>I tried the earlier version of this patch from last year and experienced problems
> debugging in 32 bit mode (I no longer could debug GCC).

The earlier version of this patch was using ptrace64 only if BFD64 was defined,
i.e. when GDB was built in 64 BIT mode.
Did you build 32 bit gdb?
If so, according to the previous version of this patch,
no ptrace related calls were changed as ptrace64 was only
called if BFD64 was defined. Could you please resend the error you got while debugging
32 bit GCC to my mail id? I can look into it to see where the problem is coming from.

> ptrace, ptracex and ptrace64 are defined in syscalls.exp.
>
> ptrace64                syscall3264
> # 32
> ptrace                  syscall32
> ptracex                 syscall32

> As you wrote, ptrace and ptracex are 32 bit only API; ptrace64 is available as 32 bit and 64 bit
> API. ptrace64 is suppose to support 32 bit debugee targets, but I experienced problems.
>
> ptrace64 will be discovered as supported by configure when building 32 bit GDB.

> Is your intention to always use ptrace64 when it is available or only to use it for GDB hosted on
> 64 bit mode?

My initial intention was to use ptrace64 only if ptrace64 was available and
GDB was built in 64 BIT. But after a few discussions over this, I decided
to use ptrace64 when ever the API is defined.

So, this present patch would use
ptrace64 even for a 32 bit build as you rightly noticed.
This did not change the test results for a 32 bit build of GDB.

Thanks,
Raunaq M. Bathija

Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.

[David Edelsohn]

On Mon, Aug 12, 2013 at 8:10 AM, Raunaq 12 <raunaq12@in.ibm.com> wrote:

> My initial intention was to use ptrace64 only if ptrace64 was available and
> GDB was built in 64 BIT. But after a few discussions over this, I decided
> to use ptrace64 when ever the API is defined.
>
> So, this present patch would use
> ptrace64 even for a 32 bit build as you rightly noticed.
> This did not change the test results for a 32 bit build of GDB.

I built gdb-7.6 with the ptrace64 patch (without --enable-64-bit-bfd)
and it seems to work correctly on cc1plus. So at least the current
patch does not harm debugging.

Thanks, David

Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.

[Joel Brobecker]

> I built gdb-7.6 with the ptrace64 patch (without --enable-64-bit-bfd)
> and it seems to work correctly on cc1plus. So at least the current
> patch does not harm debugging.

Actually, I just found that it breaks thread support on 32bit AIX,
at least.  A simpler way to show the problem is:

    (gdb) x /x &__n_pthreads
    0xf06a8258 <__n_pthreads>:      Cannot access memory at address 0xf06a8258

Prior to the patch, we have:

    (gdb) x /x &__n_pthreads
    0xf06a8258 <__n_pthreads>:      0x00000003

The error returned by ptrace64 is EIO (5), which means it thinks
the address is wrong (or else the request ID is invalid, but I doubt
that). I'm a bit rushed, as usual, but I still have a little extra
time today to dig a little further.

-- 
Joel

[RFA/ppc-aix] fix thread support breakage (was: "Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.")

[Joel Brobecker]

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

Hello again,

> Actually, I just found that it breaks thread support on 32bit AIX,
> at least.  A simpler way to show the problem is:
> 
>     (gdb) x /x &__n_pthreads
>     0xf06a8258 <__n_pthreads>:      Cannot access memory at address 0xf06a8258
> 
> Prior to the patch, we have:
> 
>     (gdb) x /x &__n_pthreads
>     0xf06a8258 <__n_pthreads>:      0x00000003

Attached is the patch I checked in locally at AdaCore:

gdb/ChangeLog:

        * rs6000-nat.c (rs6000_ptrace32): Cast "addr" to "uintptr_t"
        instead of "long long" in call to ptrace64.

Tested on ppc-aix in full 32bit mode (both GDB and inferior are
32bit). I think the other scenario would be 64bit GDB debugging
a 32bit inferior, but I don't have a 64bit compiler to try it.

I'd like to commit this patch, unless there are comments/suggestions.

Thank you,
-- 
Joel

[-- Attachment #2: 0001-thread-support-broken-on-ppc-aix.patch --]
[-- Type: text/x-diff, Size: 2060 bytes --]

From 91f7d8c9ae626954479217739aaa476e79e5daeb Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 22 Aug 2013 20:33:27 -0400
Subject: [PATCH] thread support broken on ppc-aix.

Thread support got broken when adding 64bit support on ppc-aix.
Upon digging further, I found that the following patch...

   | * gdb_ptrace.h: Use ptrace64 instead of ptrace if HAVE_PTRACE64
   | is defined.
   | * rs6000-nat.c: Check for __ld_info64_ if compiling 64 BIT gdb.
   | (rs6000_ptrace32): Call ptrace64 instead of ptrace if present.
   | (rs6000_ptrace64): Call ptace64 instead of ptracex if present.
   | * configure.ac: Check for ptrace64.
   | * configure, config.in: Regenerate.

... is responsible for this regression:

    (gdb) x /x &__n_pthreads
    0xf06a8258 <__n_pthreads>: Cannot access memory at address 0xf06a8258

Prior to the patch, we have:

    (gdb) x /x &__n_pthreads
    0xf06a8258 <__n_pthreads>: 0x00000003

The problem occurs inside rs6000_ptrace32, while calling ptrace64.
The address is given to rs6000_ptrace32 as an "int *", while
ptrace64 takes a "long long". The cast causes the address to be
sign-extended, which results in GDB trying to read the wrong address.

This patch fixes the issue by casting the address to a "uintptr_t"
instead, and letting the compiler do the implicit conversion to
"long long" in the function call.

gdb/ChangeLog:

        * rs6000-nat.c (rs6000_ptrace32): Cast "addr" to "uintptr_t"
        instead of "long long" in call to ptrace64.

---
 gdb/rs6000-nat.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/rs6000-nat.c b/gdb/rs6000-nat.c
index 0953356..b214cd9 100644
--- a/gdb/rs6000-nat.c
+++ b/gdb/rs6000-nat.c
@@ -132,7 +132,7 @@ static int
 rs6000_ptrace32 (int req, int id, int *addr, int data, int *buf)
 {
   #ifdef HAVE_PTRACE64
-  int ret = ptrace64 (req, id, (long long) addr, data, buf);
+  int ret = ptrace64 (req, id, (uintptr_t) addr, data, buf);
   #else
   int ret = ptrace (req, id, (int *)addr, data, buf);
   #endif
-- 
1.7.0.4

Re: [RFA/ppc-aix] fix thread support breakage (was: "Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.")

[David Edelsohn]

On Thu, Aug 22, 2013 at 8:48 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> Hello again,
>
>> Actually, I just found that it breaks thread support on 32bit AIX,
>> at least.  A simpler way to show the problem is:
>>
>>     (gdb) x /x &__n_pthreads
>>     0xf06a8258 <__n_pthreads>:      Cannot access memory at address 0xf06a8258
>>
>> Prior to the patch, we have:
>>
>>     (gdb) x /x &__n_pthreads
>>     0xf06a8258 <__n_pthreads>:      0x00000003
>
> Attached is the patch I checked in locally at AdaCore:
>
> gdb/ChangeLog:
>
>         * rs6000-nat.c (rs6000_ptrace32): Cast "addr" to "uintptr_t"
>         instead of "long long" in call to ptrace64.
>
> Tested on ppc-aix in full 32bit mode (both GDB and inferior are
> 32bit). I think the other scenario would be 64bit GDB debugging
> a 32bit inferior, but I don't have a 64bit compiler to try it.
>
> I'd like to commit this patch, unless there are comments/suggestions.

LGTM.

Is there a testcase that can be added to the GDB testsuite to catch
this type of regression?

Thanks, David

Re: [RFA/ppc-aix] fix thread support breakage (was: "Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.")

[Joel Brobecker]

> > gdb/ChangeLog:
> >
> >         * rs6000-nat.c (rs6000_ptrace32): Cast "addr" to "uintptr_t"
> >         instead of "long long" in call to ptrace64.
> >
> > Tested on ppc-aix in full 32bit mode (both GDB and inferior are
> > 32bit). I think the other scenario would be 64bit GDB debugging
> > a 32bit inferior, but I don't have a 64bit compiler to try it.
> >
> > I'd like to commit this patch, unless there are comments/suggestions.
> 
> LGTM.

Thanks :)

> Is there a testcase that can be added to the GDB testsuite to catch
> this type of regression?

All testcases involving thread support should reveal this type of
failure. I cannot remember exactly how this patch was tested, but
ISTR that it couldn't be tested through the official testsuite,
for some reasons that were deemed reasonable...

-- 
Joel

Re: [RFA/ppc-aix] fix thread support breakage (was: "Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.")

[David Edelsohn]

On Fri, Aug 23, 2013 at 9:34 AM, Joel Brobecker <brobecker@adacore.com> wrote:

>> Is there a testcase that can be added to the GDB testsuite to catch
>> this type of regression?
>
> All testcases involving thread support should reveal this type of
> failure. I cannot remember exactly how this patch was tested, but
> ISTR that it couldn't be tested through the official testsuite,
> for some reasons that were deemed reasonable...

Uli and I both asked Raunaq if the patch had been tested with the
testsuite. However, the testsuite may not have been run with the full
matrix: GDB32 -> Testsuite32, GDB32 ->Testsuite64, GDB64 ->
Testsuite32, GDB64 -> Testsuite64.

- David

Re: [RFA/ppc-aix] fix thread support breakage (was: "Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.")

[Raunaq 12]

David Edelsohn <dje.gcc@gmail.com> wrote on 08/23/2013 08:44:23 PM:

> From: David Edelsohn <dje.gcc@gmail.com>
> To: Joel Brobecker <brobecker@adacore.com>
> Cc: Raunaq 12/India/IBM@IBMIN, GDB Patches <gdb-patches@sourceware.org>,
Mark Kettenis
> <mark.kettenis@xs4all.nl>, Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
> Date: 08/23/2013 08:43 PM
> Subject: Re: [RFA/ppc-aix] fix thread support breakage (was: "Re: [PATCH
4/5] powerpc64-aix
> ptrace64 when defined.")
>
> On Fri, Aug 23, 2013 at 9:34 AM, Joel Brobecker <brobecker@adacore.com>
wrote:
>
> >> Is there a testcase that can be added to the GDB testsuite to catch
> >> this type of regression?
> >
> > All testcases involving thread support should reveal this type of
> > failure. I cannot remember exactly how this patch was tested, but
> > ISTR that it couldn't be tested through the official testsuite,
> > for some reasons that were deemed reasonable...
>
> Uli and I both asked Raunaq if the patch had been tested with the
> testsuite. However, the testsuite may not have been run with the full
> matrix: GDB32 -> Testsuite32, GDB32 ->Testsuite64, GDB64 ->
> Testsuite32, GDB64 -> Testsuite64.

Was out of office for a while so couldn't reply earlier. Sorry for that.

David, my intention was to run test cases for all the combinations that you
have mentioned above. Since we use XLC (AIX native compiler) to compile the
test cases, the entire test bucket does not run very smoothly. In fact it
is a
very cumbersome process. So the test failures would have been over looked
by me.

In future, I  will stick to using GCC to compile the test cases to ensure a
smoother
execution of the test bucket.

Joel, thanks for fixing the regression:) Applied the patch on my AIX
machine and it does
not cause any issues in 32/64 BIT.

Regards,
Raunaq M. Bathija

Re: [RFA/ppc-aix] fix thread support breakage

[Tom Tromey]

>>>>> ">" == Raunaq 12 <raunaq12@in.ibm.com> writes:

>> David, my intention was to run test cases for all the combinations
>> that you have mentioned above. Since we use XLC (AIX native compiler)
>> to compile the test cases, the entire test bucket does not run very
>> smoothly.

I guess this supplies a future to-do list :-)

>> In fact it is a very cumbersome process. So the test failures would
>> have been over looked by me.

The standard thing to do is run the test suite twice, once before your
patch and once after, and compare the results.

Tom

Checked in: [RFA/ppc-aix] fix thread support breakage (was: "Re: [PATCH 4/5] powerpc64-aix ptrace64 when defined.")

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * rs6000-nat.c (rs6000_ptrace32): Cast "addr" to "uintptr_t"
>         instead of "long long" in call to ptrace64.
> 
> Tested on ppc-aix in full 32bit mode (both GDB and inferior are
> 32bit). I think the other scenario would be 64bit GDB debugging
> a 32bit inferior, but I don't have a 64bit compiler to try it.
> 
> I'd like to commit this patch, unless there are comments/suggestions.

Now checked in.

-- 
Joel