Re: [PATCH v3] Enable tracing of pseudo-registers on ARM

Pedro Alves
2016-02-24T18:20:06Z

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