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

Antoine Tremblay
2016-02-24T18:47:00Z

Pedro Alves writes:

> 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.
>
Nervermind that seems like I got confused.

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

OK.

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

Thanks,
Antoine