This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3] Enable tracing of pseudo-registers on ARM
- From: Pedro Alves <palves at redhat dot com>
- To: Antoine Tremblay <antoine dot tremblay at ericsson dot com>
- Cc: gdb-patches at sourceware dot org, qiyaoltc at gmail dot com
- Date: Wed, 24 Feb 2016 18:19:58 +0000
- Subject: Re: [PATCH v3] Enable tracing of pseudo-registers on ARM
- Authentication-results: sourceware.org; auth=none
- References: <wwoklh6gpsne dot fsf at ericsson dot com> <1455910116-13237-1-git-send-email-antoine dot tremblay at ericsson dot com> <56C7796B dot 3030504 at redhat dot com> <wwokk2lv8bmd dot fsf at ericsson dot com>
On 02/23/2016 07:34 PM, Antoine Tremblay wrote:
>
> Pedro Alves writes:
>
>>> +
>>> + return double_regnum;
>>> +}
>>> +
>>> +/* Implementation of the ax_pseudo_register_collect gdbarch function. */
>>> +
>>> +static int
>>> +arm_ax_pseudo_register_collect (struct gdbarch *gdbarch,
>>> + struct agent_expr *ax, int reg)
>>> +{
>>> + int rawnum = arm_pseudo_register_to_register (gdbarch, reg);
>>> +
>>> + /* Error. */
>>> + if (rawnum < 0)
>>> + return 1;
>>> +
>>> + ax_reg_mask (ax, rawnum);
>>
>> Hmm, seems to me that gdb raw -> target raw mapping should be
>> either here, or perhaps even in ax_reg / ax_reg_mask?
>>
>
> After more investigation, this can't be in ax_reg / ax_reg_mask for
> pseudo registers as this function is solely reponsible to encode the
> right number here.
I don't follow.
ax_reg / ax_reg_mask today obviously work with gdb numbers:
/* Add register REG to the register mask for expression AX. */
void
ax_reg_mask (struct agent_expr *ax, int reg)
{
if (reg >= gdbarch_num_regs (ax->gdbarch))
{
/* This is a pseudo-register. */
if (!gdbarch_ax_pseudo_register_collect_p (ax->gdbarch))
error (_("'%s' is a pseudo-register; "
"GDB cannot yet trace its contents."),
user_reg_map_regnum_to_name (ax->gdbarch, reg));
if (gdbarch_ax_pseudo_register_collect (ax->gdbarch, ax, reg))
error (_("Trace '%s' failed."),
user_reg_map_regnum_to_name (ax->gdbarch, reg));
}
else
...
This is comparing gdb-side num_regs, and calling
gdbarch_ax_pseudo_register_collect, whose implementations expect
gdb register numbers. And it calls user_reg_map_regnum_to_name,
which works with gdb register numbers. Etc.
So it seems to me that we need to make ax_reg and ax_reg_mask
convert gdb -> remote numbers in their else branches.
>
>> Consider the case of an expression requiring the collection of
>> a _raw_ register, thus not even reaching here. Looking at
>> ax-gdb.c/ax-general.c I don't see where is anything mapping gdb raw numbers
>> to remote/tdesc numbers? So how does _that_ work? Are the register masks that gdb
>> is computing actually wrong for the target, and things just happen
>> to work because gdbserver ignores them and always collects all registers?
>
> However yes it should be in ax_reg/ax_reg_mask for non-pseudo registers,
> but this is not the objective of this patch, I suggest that such a
> change be the subject of another patch
Sure, but in that case, drop the gdb -> remote conversion entirely.
If with that things don't work for arm, let's fix ax_reg/ax_reg_mask
_first_.
> maybe coupled with better gdbserver handling of the R action.
I think this coupling would be a mistake. This can be handled
independently, if at all.
>
> I will send a v5 with the ax_pseudo_register_collect inside the
> arm_ax_pseudo_register_collect/arm_ax_pseudo_register_push stack function.
Thanks,
Pedro Alves