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

[Antoine Tremblay <antoine.tremblay@ericsson.com>]

Pedro Alves writes:

> On 02/24/2016 06:20 PM, Marcin Kościelnicki wrote:
>> On 24/02/16 19:11, Pedro Alves wrote:
>>> On 02/22/2016 04:51 PM, Antoine Tremblay wrote:
>>>>
>
>>>> While looking at fixing this for all the archs involved it would be
>>>> much simpler to test if gdbserver would make use of it.
>>>>
>>>> As it is now, I'm concerned that calling gdbarch_remote_register_number
>>>> in ax_reg, ax_mask_reg could break things if the arch already considers
>>>> the gdb raw -> target raw mapping like s390 and x86 do already (I'm not
>>>> 100% sure the mapping is already ok)?
>>>
>>> WDTM?  Where do they do this already?

I meant that the pseudo register code could have considered this already
and use tdesc numbers, thus adding a mapping would cause problems if it
tried to map tdesc to tdesc rather then gdb to tdesc.

But looking more into it, and you confirmed below, it does not, and s390
does not either so it should be straight forward to fix. In fact x86
sems to be in sync with tdesc AFAICT.

>> 
>> FWIW, I failed to look at the numbering used when I wrote the x86 and 
>> s390 ax functions, so they're most likely wrong (I just copied the 
>> regnum computation logic from pseudo_read/write, which uses gdb 
>> numbers).  s390 hasn't landed yet, so it's only x86 that you'd have to 
>> fix now (and mips, I think, but that doesn't support tracepoints yet...).
>
> I don't think there's anything that needs fixing in the i386 implementation.
>
> The x86 implementation maps gdb pseudo register numbers to whatever
> raw gdb registers back the former up, like:
>
>       ax_reg_mask (ax, I387_FSTAT_REGNUM (tdep));
>
> That OK.
>
> The trouble is that in the end we send gdb numbers to the target in the
> ax, instead of tdesc/remote numbers.
>
> We never noticed because gdbserver always collects all raw registers
> anyway.
>
> Seems to me that the fix is to make ax_reg / ax_reg_mask take gdb raw
> numbers as input (as it does today), and then make it map those to
> tdesc/remote number just before it puts the reg number in the agent
> expression bytecode / reg mask.  And that covers all archs.
>
>> 
>> Testing this is possible if you write some conditions that involve 
>> reading pseudo-registers (since ax_pseudo_register_push_stack will be 
>> called), the problem is that I only implemented 
>> ax_pseudo_register_collect for x86...
>> 
>> Are you going to make some higher-level patch that will magically fix it 
>> for my s390 patch, or do I have to fix that on my own?
>
> I haven't memorized your s390 patch :-) but there's probably nothing to
> do on the s390-specific bits.
>
> Thanks,
> Pedro Alves