Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.

[Marcin KoÅcielnicki <koriakin at 0x04 dot net>]

On 11/03/16 12:20, Andreas Arnez wrote:
> On Sun, Feb 07 2016, Marcin KoÅcielnicki wrote:
>
> > diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
> > index 97bd564..0b91ed1 100644
> > --- a/gdb/s390-linux-tdep.c
> > +++ b/gdb/s390-linux-tdep.c
> > @@ -627,6 +627,18 @@ s390_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
> >     return 0;
> >   }
> >
> > +/* The "gen_return_address" gdbarch method.  */
> > +
> > +static void
> > +s390_gen_return_address (struct gdbarch *gdbarch,
> > +			 struct agent_expr *ax, struct axs_value *value,
> > +			 CORE_ADDR scope)
> > +{
> > +  value->type = register_type (gdbarch, S390_R14_REGNUM);
> > +  value->kind = axs_lvalue_register;
> > +  value->u.reg = S390_R14_REGNUM;
> > +}
>
> Under which circumstances is this supposed to work?  And how reliable
> does it need to be?  The ABI only guarantees that r14 holds the return
> address at function entry.  Anywhere else it likely doesn't.
>
> --
> Andreas

Quoting gdbarch.sh:

# Generate bytecodes to collect the return address in a frame.
# Since the bytecodes run on the target, possibly with GDB not even
# connected, the full unwinding machinery is not available, and
# typically this function will issue bytecodes for one or more likely
# places that the return address may be found.
m:void:gen_return_address:struct agent_expr *ax, struct axs_value 
*value, CORE_ADDR scope:ax, value, scope::default_gen_return_address::0

ie. it's supposed to collect some value that will likely help the 
unwinder - if it collects the wrong thing, the unwinder (knowing the 
right place for sure) will simply consider the previous frame PC to be 
unavailable.

We could also try to collect 14*<wordsize>(%r11), hoping that's the save 
slot for %r14, but the interface unfortunately doesn't support 
collecting multiple values (no matter what the comment above says).

Unfortunately, this interface is just not very well-designed - both x86 
and aarch64 just take a shot in the dark like this patch.  A better way 
would be to reuse the existing unwinders and remove this hook 
altogether, or (for while-stepping, where we can't predict the PC) 
actually allow multiple values and aim at a few likely locations.  But 
IMO that's not in scope for this patchset.