[v3] Enable tracing of pseudo-registers on ARM

  In this v3:
* Use gdbarch_remote_register_number to get the remote/tsec register number
Thanks to Pedro for pointing me in the right direction.
-

This patch implements the ax_pseudo_register_push_stack and
ax_pseudo_register_collect gdbarch functions so that a pseudo-register can
be traced.

No regressions, tested on ubuntu 14.04 ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }

gdb/ChangeLog:

	* arm-tdep.c (arm_pseudo_register_to_register): New function.
	(arm_ax_pseudo_register_collect): New function.
	(arm_ax_pseudo_register_push_stack): New function.
	(arm_gdbarch_init): Set
	gdbarch_ax_pseudo_register_{collect,push_stack} functions.

gdb/testsuite/ChangeLog:

	* gdb.trace/tfile-avx.c: Move to...
	* gdb.trace/tracefile-pseudo-reg.c: Here.
	* gdb.trace/tfile-avx.exp: Move to...
	* gdb.trace/tracefile-pseudo-reg.exp: Here.
---
 gdb/arm-tdep.c                                   | 71 ++++++++++++++++++
 gdb/testsuite/gdb.trace/tfile-avx.c              | 53 -------------
 gdb/testsuite/gdb.trace/tfile-avx.exp            | 73 ------------------
 gdb/testsuite/gdb.trace/tracefile-pseudo-reg.c   | 65 ++++++++++++++++
 gdb/testsuite/gdb.trace/tracefile-pseudo-reg.exp | 94 ++++++++++++++++++++++++
 5 files changed, 230 insertions(+), 126 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.trace/tfile-avx.c
 delete mode 100644 gdb/testsuite/gdb.trace/tfile-avx.exp
 create mode 100644 gdb/testsuite/gdb.trace/tracefile-pseudo-reg.c
 create mode 100644 gdb/testsuite/gdb.trace/tracefile-pseudo-reg.exp
  

On 02/19/2016 07:28 PM, Antoine Tremblay wrote:

> +/* Map the pseudo register number REG to the proper register number.  */
> +
> +static int
> +arm_pseudo_register_to_register (struct gdbarch *gdbarch, int reg)
> +{

> +  /* Get the remote/tdesc register number.  */
> +  double_regnum = gdbarch_remote_register_number (gdbarch, double_regnum);

Hmm, I don't think it should be the responsibility of this function to
map gdb to remote numbers though.  Here I think we should just map
gdb pseudo to gdb raw.

> +
> +  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?

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?

Thanks,
Pedro Alves
  

Pedro Alves writes:

> On 02/19/2016 07:28 PM, Antoine Tremblay wrote:
>
>> +/* Map the pseudo register number REG to the proper register number.  */
>> +
>> +static int
>> +arm_pseudo_register_to_register (struct gdbarch *gdbarch, int reg)
>> +{
>
>> +  /* Get the remote/tdesc register number.  */
>> +  double_regnum = gdbarch_remote_register_number (gdbarch, double_regnum);
>
> Hmm, I don't think it should be the responsibility of this function to
> map gdb to remote numbers though.  Here I think we should just map
> gdb pseudo to gdb raw.

Yes I had created that function for arm_ax_pseudo_register_* functions
but yes maybe it would be better at a lower level and allow this
function to be used by something else.
>
>> +
>> +  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?
>

Yes now that you mention it it would make sense in ax_reg/reg_mask.

> 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?
>
I would assume so indeed!

I'll make this a small series send another patch to apply prior to this
one with the change to ax_reg, ax_reg_mask.

Thanks,
Antoine
  

Pedro Alves <palves@redhat.com> writes:

> Hmm, I don't think it should be the responsibility of this function to
> map gdb to remote numbers though.  Here I think we should just map
> gdb pseudo to gdb raw.

Yes, I agree.  Each backend should map pseudo to gdb raw, and the common
code should map the gdb raw to target raw number.
  

Pedro Alves writes:

> Hmm, seems to me that gdb raw -> target raw mapping should be
> either here, or perhaps even in ax_reg / ax_reg_mask?
>
> 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?
>

Is there a good reason gdbserver actually ignores that ?

It seems all the code is there for it to consider it on gdb's
side. encode_actions, stringify_collection_list etc... The only thing
missing seems to be gdbserver interpretation of the R action.

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)? And that it is set to use tdesc
registers (so that gdbarch_remote_register_number maps to
tdesc_remote_register).

Thanks,
Antoine
  

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.

> 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 maybe coupled with better
gdbserver handling of the R action.

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.
  

On 02/22/2016 04:51 PM, Antoine Tremblay wrote:
> 
> Pedro Alves writes:
> 
>> Hmm, seems to me that gdb raw -> target raw mapping should be
>> either here, or perhaps even in ax_reg / ax_reg_mask?
>>
>> 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?
>>
> 
> Is there a good reason gdbserver actually ignores that ?

I don't recall any, other than collecting everything is expedient
and good enough...

> 
> It seems all the code is there for it to consider it on gdb's
> side. encode_actions, stringify_collection_list etc... The only thing
> missing seems to be gdbserver interpretation of the R action.

Right.  Obviously you'd need to consider how to represent the
partial register set in the trace frame as well.  Just marking
some registers as unavailable while still crafting a whole register
block in the trace buffer is pointless, obviously.

> 
> 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?


 And that it is set to use tdesc
> registers (so that gdbarch_remote_register_number maps to
> tdesc_remote_register).

Thanks,
Pedro Alves
  

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
  

On 24/02/16 19:11, Pedro Alves wrote:
> On 02/22/2016 04:51 PM, Antoine Tremblay wrote:
>>
>> Pedro Alves writes:
>>
>>> Hmm, seems to me that gdb raw -> target raw mapping should be
>>> either here, or perhaps even in ax_reg / ax_reg_mask?
>>>
>>> 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?
>>>
>>
>> Is there a good reason gdbserver actually ignores that ?
>
> I don't recall any, other than collecting everything is expedient
> and good enough...
>
>>
>> It seems all the code is there for it to consider it on gdb's
>> side. encode_actions, stringify_collection_list etc... The only thing
>> missing seems to be gdbserver interpretation of the R action.
>
> Right.  Obviously you'd need to consider how to represent the
> partial register set in the trace frame as well.  Just marking
> some registers as unavailable while still crafting a whole register
> block in the trace buffer is pointless, obviously.
>
>>
>> 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?

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

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?
>
>
>   And that it is set to use tdesc
>> registers (so that gdbarch_remote_register_number maps to
>> tdesc_remote_register).
>
> Thanks,
> Pedro Alves
>
  

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?
> 
> 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
  

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
  

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

The only requirement for this to work properly is that the arch uses
tdesc_use_registers, otherwise the default mapping function to tdesc is
identity to GDB numbers.

s390 uses that so it should be fine.

Thanks,
Antoine
  

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
  

On 02/24/2016 06:55 PM, Antoine Tremblay wrote:

> The only requirement for this to work properly is that the arch uses
> tdesc_use_registers, otherwise the default mapping function to tdesc is
> identity to GDB numbers.

Even then, if the target doesn't report a tdesc, register numbers
on the target side must match gdb's.  So it still works.  The reason
the current code doesn't consider tdesc numbers is that AX predates
xml target descriptions, and back then gdb numbers was all you had.

> s390 uses that so it should be fine.

Thanks,
Pedro Alves