Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Peter Schauer <peterschauer@gmx.net>
To: mark.kettenis@xs4all.nl (Mark Kettenis)
Cc: brobecker@adacore.com, jan.kratochvil@redhat.com,
	  gdb-patches@sourceware.org
Subject: Re: [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #3
Date: Sat, 31 Dec 2011 02:56:00 -0000	[thread overview]
Message-ID: <201112302227.pBUMRCjB021149@licht.localdomain> (raw)
In-Reply-To: <201112301038.pBUAcxQS004801@glazunov.sibelius.xs4all.nl> from "Mark Kettenis" at Dec 30, 2011 11:38:59 AM

> > Date: Fri, 30 Dec 2011 07:30:20 +0400
> > From: Joel Brobecker <brobecker@adacore.com>
> > 
> > > As ON_STACK is a valid option sure one may prefer to convert all the
> > > archs to ON_STACK instead of fixing AT_ENTRY_POINT; not preferred by
> > > me TBH.
> > 
> > I don't understand why, though. ON_STACK seems to be perfect, as
> > we control exactly what's there, and we know we're not going to
> > interfere with the rest of the code.
> 
> I believe it has always been the intention to make ON_STACK the
> default.  See the comment by Andrew Cagney in mips-tdep.c.  For some
> architectures it is the only viable option.
> 
> I'll probably switch i386/amd64 to use ON_STACK, since it allows for
> some cleanups in the DICOS support.
> 
> > Are there any situation where ON_STACK wouldn't be possible?
> 
> I don't think so.
> 
> > I have put in my TODO list to see if I can get rid of the last
> > use of AT_SYMBOL (in mips-tdep.c) and convert it to ON_STACK.
> 
> That'd be good since mips-tdep.c seems to be the only one left that
> still uses AT_SYMBOL.  Would be nice if we can remove that code.
> 
> > > 2011-12-30  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > > 
> > > 	Fix regression for gdb.cp/gdb2495.exp with gcc-4.7.
> > > 	* arch-utils.c (displaced_step_at_entry_point): Incrase BP_LEN skip to
> > > 	3 times.
> > > 	* infcall.c (call_function_by_hand) <AT_SYMBOL>: Move it upwards and
> > > 	fall through into AT_ENTRY_POINT.
> > > 	(call_function_by_hand) <AT_ENTRY_POINT>: New variable bp_len.  Adjust
> > > 	DUMMY_ADDR with it.
> > > 	* ppc-linux-tdep.c (ppc_linux_displaced_step_location): Increase
> > > 	PPC_INSN_SIZE skip to 3 times.
> > 
> > I keep staring at the diff, and I can't figure out how the AT_SYMBOL
> > case is falling through, or why it would be necessary. The lack of
> > understading why is probably related to the fact that I may still
> > have an incorrect understanding of the situation.
> 
> The idea is that AT_SYMBOL will fall back on putting the call dummy
> breakpoint at the entry point if the magic symbol name isn't found.
> Currently this is achieved by having duplicated code.  Jan's diff
> changes it in a FALLTHROUGH to make it more explicit.
> 
> > I think that either way, it's better to have the dummy calling
> > address at a location where there is no unwinding information.
> > ON_STACK seems to be a better place to guaranty that.  But that
> > being said, making sure that, for AT_ENTRY_POINT, the dummy
> > calling address is indeed our entry point cannot make things
> > worse.
> 
> I'm still a little bit worried that Jan's approach is making
> additional assumptions.  The chance that the 2nd instruction of the
> program will be executed again as part of the normal flow is probably
> not much higher than for the 1st instructions, but the 1st instruction
> could be a jump.  And I know Jan has checked his diff on ia64, but
> there might be other architectures that have the concept of
> instruction bundles where jumping into the middle of a bundle doesn't
> work.
> 
> I think I'd be in favour of switching as many targets as possible to
> ON_STACK, remove AT_SYMBOL and leave AT_ENTRY_POINT alone.  But I
> don't feel too strongly about this.  Targets that switch to ON_STACK
> won't be affected by assumptions in AT_ENTRY_POINT that turn out not
> to be true.  And if we manage to switch all targets to ON_STACK, we
> can simply delete AT_ENTRY_POINT.

It is funny that we tried to convert all targets to use AT_ENTRY in
the past and now you are heading towards the opposite direction.

Provided that ON_STACK will handle non-executable stacks properly in
all cases, AT_ENTRY_POINT has some small additional advantage:
If the called function clobbers the stack and overwrites the breakpoint
instruction, the inferior will no longer stop after the inferior function
call when using ON_STACK.

Apart from that, I found no reasons in my age old notes why ON_STACK
could cause problems.
Let me know if you want some more historical notes on why AT_ENTRY_POINT
was introduced at all.


  parent reply	other threads:[~2011-12-30 22:27 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-22 20:49 [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 Jan Kratochvil
2011-12-27  6:23 ` Joel Brobecker
2011-12-28 16:30   ` Jan Kratochvil
2011-12-28 18:47     ` [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #2 Jan Kratochvil
2011-12-28 20:40       ` Mark Kettenis
2011-12-30  2:45         ` [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #3 Jan Kratochvil
2011-12-30  8:46           ` Joel Brobecker
2011-12-30 11:11             ` Mark Kettenis
2011-12-30 14:16               ` Jan Kratochvil
2011-12-31  2:56               ` Peter Schauer [this message]
2011-12-30 11:25             ` Jan Kratochvil
2012-01-01 22:22               ` Jan Kratochvil
2012-01-02  2:45                 ` Joel Brobecker
2012-01-02  2:58                   ` Jan Kratochvil
2012-01-03 14:45                     ` Regression on PowerPC (Re: [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #3) Ulrich Weigand
2012-01-03 15:52                       ` Joel Brobecker
2012-01-04 14:01                       ` [revert] " Jan Kratochvil
2012-01-04 14:09                         ` Joel Brobecker
2012-03-08 23:24                         ` [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #4 [Re: [revert] Regression on PowerPC] Jan Kratochvil
2012-03-09  7:22                           ` cancel: " Jan Kratochvil
2012-01-02 14:10 ` [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 Pedro Alves
2012-01-02 14:20   ` Jan Kratochvil
2012-01-02 14:44     ` Pedro Alves
2012-01-02 14:53       ` Jan Kratochvil

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=201112302227.pBUMRCjB021149@licht.localdomain \
    --to=peterschauer@gmx.net \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=mark.kettenis@xs4all.nl \
    /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