Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Peter Schauer <peterschauer@gmx.net>
To: uweigand@de.ibm.com (Ulrich Weigand)
Cc: brobecker@adacore.com (Joel Brobecker),
	palves@redhat.com (Pedro Alves),
	  gdb-patches@sourceware.org (GDB Patches)
Subject: Re: eliminate deprecated_insert_raw_breakpoint.  what's left.
Date: Tue, 09 Sep 2014 12:38:00 -0000	[thread overview]
Message-ID: <201409091237.s89CbnwW017512@licht.localdomain> (raw)
In-Reply-To: <201409091138.s89BcpNI023387@d06av02.portsmouth.uk.ibm.com> from "Ulrich Weigand" at Sep 09, 2014 01:38:51 PM

> Peter Schauer wrote:
> 
> > I hope to be able to shed some light on this problem, although it
> > is more than fifteen years ago that I did some work for GDB on AIX.
> > 
> > From my notes back then, AIX 3 and AIX 4 had a very peculiar ptrace
> > implementation, where the current ptrace state of the inferior process
> > (including the current process registers) was maintained approximately
> > 512 bytes below the current user stack pointer of the process.
> > 
> > This resulted in problems with AIX inferior function calls.
> > If the called function takes one or more large aggregate parameters
> > by value, or if you pass a large amount of parameters, the ptrace
> > area gets corrupted, when the dummy function call parameters are
> > pushed on the user stack, due to this awkward AIX stack layout.
> 
> Thanks for providing this background!
> 
> > To work around this problem, the execution of a dummy instruction
> > (when altering the stack pointer) caused the kernel to move the ptrace
> > state area further below on the user stack, allowing GDB to write below
> > the current user stack safely.
> > In GDB 6.x, rs6000_push_dummy_call even secured the stack partially during
> > pushing of the arguments, via an additional call of
> > regcache_raw_write_signed to gdbarch_sp_regnum (gdbarch), which is
> > no longer present in current versions of GDB.
> 
> Well, I still see this:
>   /* Set the stack pointer.  According to the ABI, the SP is meant to
>      be set _before_ the corresponding stack space is used.  On AIX,
>      this even applies when the target has been completely stopped!
>      Not doing this can lead to conflicts with the kernel which thinks
>      that it still has control over this not-yet-allocated stack
>      region.  */
>   regcache_raw_write_signed (regcache, gdbarch_sp_regnum (gdbarch), sp);
> 
> and:
>       /* This is another instance we need to be concerned about
>          securing our stack space.  If we write anything underneath %sp
>          (r1), we might conflict with the kernel who thinks he is free
>          to use this area.  So, update %sp first before doing anything
>          else.  */
> 
>       regcache_raw_write_signed (regcache,
>                                  gdbarch_sp_regnum (gdbarch), sp);
> 
> Are there other instances where this is missing?

Ok, my bad, I was looking at the wrong push_dummy_call implementation
in the current GDB source.
rs6000_push_dummy_call in the new rs6000-aix-tdep.c file in the current GDB
source still contains the code in question from GDB 6.x, there is
nothing missing.

> > Executing the dummy instruction is very fragile, especially if signals
> > get involved during the execution, and it didn't even help, if more
> > than ~100 bytes of parameters were pushed on the user stack on AIX 4.
> > Back then, there was no other choice though.
> > 
> > Unfortunately I do not know, if this peculiar AIX stack layout is still
> > used in AIX 5 or later, maybe Ulrich Weigand could tell you more about it.
> 
> I don't know off-hand.  I'll try to find out.
>  
> > I think you could/should zap exec_one_dummy_insn, provided that you test
> > a dummy function call on the oldest AIX version that GDB has to support,
> > with a large aggregate parameter, which is passed by value.
> 
> The only version I have ready access to is AIX 7.1, and on this there
> are no testsuite regression (and in fact, quite a number of failures
> seem to go away!) when zapping exec_one_dummy_insn.

+1 for zapping exec_one_dummy_insn.

> I'm not sure which versions we need to / should support in GDB; I guess
> the oldest version where the OS itself is still supported by IBM is 6.1.

Maybe somebody could test if zapping exec_one_dummy_insn on AIX 6.1
has any negative effect, and then be done with it.

But even if that can't be tested, I am all in favour of getting rid
of it, perhaps with a detailed comment in the commit message for the
removal (or adding a link to this thread).

-- 
Peter Schauer			Peter.Schauer@mytum.de


  reply	other threads:[~2014-09-09 12:38 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-08 17:46 Pedro Alves
2014-09-08 19:24 ` Joel Brobecker
2014-09-08 21:34   ` Joel Brobecker
2014-09-08 22:50     ` Pedro Alves
2014-09-09  0:25       ` Peter Schauer
2014-09-09  0:16     ` Peter Schauer
2014-09-09 11:39       ` Ulrich Weigand
2014-09-09 12:38         ` Peter Schauer [this message]
2014-09-09 21:25           ` Ulrich Weigand
2014-09-10 12:21             ` Joel Brobecker
2014-09-10 13:15               ` Ulrich Weigand
2014-09-10 15:22                 ` Pedro Alves
2014-09-09 21:48   ` Ulrich Weigand
2014-09-10 12:29     ` Joel Brobecker
2014-09-10 14:45       ` Ulrich Weigand
2014-09-10 15:21         ` Pedro Alves
2014-09-10 15:50           ` Joel Brobecker
2014-09-10 16:00             ` Sergio Durigan Junior
2014-09-10 16:36             ` Ulrich Weigand
2014-09-10 18:59               ` New deprecation procedure Pedro Alves
2014-09-11 19:03                 ` Joel Brobecker
2014-09-12  8:51                   ` Ulrich Weigand
2014-09-10 15:50           ` eliminate deprecated_insert_raw_breakpoint. what's left Maciej W. Rozycki
2014-09-10 16:12             ` [IRIX] eliminate deprecated_insert_raw_breakpoint uses Pedro Alves
2014-09-10 22:44               ` Joel Brobecker
2014-09-10 23:02                 ` Pedro Alves
2014-09-11  3:27                   ` Joel Brobecker
2014-09-12 19:34                     ` Pedro Alves
2014-09-12 20:23                       ` Joel Brobecker
2014-10-07  0:25         ` eliminate deprecated_insert_raw_breakpoint. what's left Stan Shebs
2014-09-09 17:33 ` Pedro Alves
     [not found] <alpine.DEB.1.10.1409101553070.27075@tp.orcam.me.uk>
2014-09-10 16:45 ` Ulrich Weigand
2014-09-10 19:11   ` Maciej W. Rozycki
2014-09-11 11:50     ` Ulrich Weigand

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=201409091237.s89CbnwW017512@licht.localdomain \
    --to=peterschauer@gmx.net \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    --cc=uweigand@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