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

Marcin Kościelnicki
2016-03-11T11:35:36Z

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.