Re: [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #3

Peter Schauer
2011-12-30T22:27:38Z

> > 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.