[3/4] Enable tracing of pseudo-registers on ARM

  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/arm-tdep.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
  

Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

> +/* Map the pseudo register number REG to the proper register number.  */
> +
> +static int
> +arm_pseudo_register_to_register (struct gdbarch *gdbarch, int reg)
> +{
> +  int rawnum = 0;
> +  int num_regs = gdbarch_num_regs (gdbarch);
> +
> +  /* Single precision pseudo registers. s0-s31.  */
> +  if (reg >= num_regs && reg < num_regs + 32)
> +    {
> +      rawnum = (reg - num_regs) / 2 + 26;

We should get double register number via user_reg_map_name_to_regnum,

      xsnprintf (name_buf, sizeof (name_buf), "d%d", (reg - num_regs) / 2);
      double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
						   strlen (name_buf));

> +    }
> +  /* Quadruple precision pseudo regisers. q0-q15.  */
> +  else if (reg >= num_regs + 32 && reg < num_regs + 32 + 16)
> +    {
> +      rawnum = (reg - num_regs - 32) * 2 + 26;

Likewise,

      xsnprintf (name_buf, sizeof (name_buf), "d%d", (reg - num_regs) * 2);
      double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
						   strlen (name_buf));

additionally, we need to check gdbarch_tdep (gdbarch)->have_neon_pseudos,

> +    }
> +  /* Error bad register number.  */
> +  else
> +    return -1;
> +
> +  return rawnum;
> +}

We also need a test case, and you can extend gdb.trace/tfile-avx.exp.
Probably, it can be renamed to gdb.trace/tracefile-pseudo-reg.exp, and
put x86 and arm tests in it.
  

On 12/02/16 16:13, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>> +/* Map the pseudo register number REG to the proper register number.  */
>> +
>> +static int
>> +arm_pseudo_register_to_register (struct gdbarch *gdbarch, int reg)
>> +{
>> +  int rawnum = 0;
>> +  int num_regs = gdbarch_num_regs (gdbarch);
>> +
>> +  /* Single precision pseudo registers. s0-s31.  */
>> +  if (reg >= num_regs && reg < num_regs + 32)
>> +    {
>> +      rawnum = (reg - num_regs) / 2 + 26;
>
> We should get double register number via user_reg_map_name_to_regnum,
>
>        xsnprintf (name_buf, sizeof (name_buf), "d%d", (reg - num_regs) / 2);
>        double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
> 						   strlen (name_buf));
> .
>> +    }
>> +  /* Quadruple precision pseudo regisers. q0-q15.  */
>> +  else if (reg >= num_regs + 32 && reg < num_regs + 32 + 16)
>> +    {
>> +      rawnum = (reg - num_regs - 32) * 2 + 26;
>
> Likewise,
>
>        xsnprintf (name_buf, sizeof (name_buf), "d%d", (reg - num_regs) * 2);
>        double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
> 						   strlen (name_buf));
>
> additionally, we need to check gdbarch_tdep (gdbarch)->have_neon_pseudos,
>
>> +    }
>> +  /* Error bad register number.  */
>> +  else
>> +    return -1;
>> +
>> +  return rawnum;
>> +}
>
> We also need a test case, and you can extend gdb.trace/tfile-avx.exp.
> Probably, it can be renamed to gdb.trace/tracefile-pseudo-reg.exp, and
> put x86 and arm tests in it.
>

I'd like to point out that this testcase is near-useless for testing 
ax_pseudo_register_collect or pseudo_register_to_register at the moment 
- while gdb computes a mask of what registers need to be collected, 
gdbserver just ignores it and collects all registers if any register at 
all is to be collected.  In turn, gdb allows you to display the state of 
all registers, even ones not included in the mask.  In fact, the 
tfile-avx.exp test passes just fine if you change it to collect any 
unrelated register.  My commit with ax_pseudo_register_collect only made 
it work because gdb needs to have that function return success, the 
actual returned mask could just as well be wrong...

The other hook, pseudo_register_push_stack, is much easier to test - 
it's invoked when a pseudo is used in an actual agent expression, eg. if 
you use it in a tracepoint condition, or as part of the address of 
collected memory area.  However, it cannot be used on SIMD registers (at 
least on x86, I don't know much about arm), as they don't fit in an 
ULONGEST...

Matter of fact, our support for >64-bit quantities in tracepoints is 
very poor at the moment - they can only be collected wholesale when 
they're single registers or contig memory areas.  Use in expressions is 
out (if you happen to have something interesting in low 32 bits of a 
vector reg, sorry).  Likewise, stiching them together with DW_op_piece 
(or whatever that was called) also fails (see 
https://sourceware.org/bugzilla/show_bug.cgi?id=17015).  We could 
definitely use some improvement there...
  

Marcin Kościelnicki <koriakin@0x04.net> writes:

> I'd like to point out that this testcase is near-useless for testing
> ax_pseudo_register_collect or pseudo_register_to_register at the
> moment - while gdb computes a mask of what registers need to be
> collected, gdbserver just ignores it and collects all registers if any
> register at all is to be collected.  In turn, gdb allows you to
> display the state of all registers, even ones not included in the
> mask.  In fact, the tfile-avx.exp test passes just fine if you change
> it to collect any unrelated register.  My commit with
> ax_pseudo_register_collect only made it work because gdb needs to have
> that function return success, the actual returned mask could just as
> well be wrong...

The usefulness I can think of is that GDB can check whether the pseudo
register exists to collect.  User may want to collect Q registers, but
they don't exist on the target.

>
> The other hook, pseudo_register_push_stack, is much easier to test - 
> it's invoked when a pseudo is used in an actual agent expression,
> eg. if you use it in a tracepoint condition, or as part of the address
> of collected memory area.  However, it cannot be used on SIMD
> registers (at least on x86, I don't know much about arm), as they
> don't fit in an ULONGEST...

The same issue on both arm and aarch64, AFAIK.

>
> Matter of fact, our support for >64-bit quantities in tracepoints is
> very poor at the moment - they can only be collected wholesale when
> they're single registers or contig memory areas.  Use in expressions
> is out (if you happen to have something interesting in low 32 bits of
> a vector reg, sorry).  Likewise, stiching them together with
> DW_op_piece (or whatever that was called) also fails (see
> https://sourceware.org/bugzilla/show_bug.cgi?id=17015).  We could
> definitely use some improvement there...

Yeah, agreed.
  

On 02/15/2016 10:27 AM, Yao Qi wrote:
> Marcin Kościelnicki <koriakin@0x04.net> writes:
> 

>>
>> Matter of fact, our support for >64-bit quantities in tracepoints is
>> very poor at the moment - they can only be collected wholesale when
>> they're single registers or contig memory areas.  Use in expressions
>> is out (if you happen to have something interesting in low 32 bits of
>> a vector reg, sorry).  Likewise, stiching them together with
>> DW_op_piece (or whatever that was called) also fails (see
>> https://sourceware.org/bugzilla/show_bug.cgi?id=17015).  We could
>> definitely use some improvement there...
> 
> Yeah, agreed.
> 

I think the that ultimate long term solution would pass actual DWARF
expressions to the target side as collect actions.  AX predates DWARF; probably
if we were starting now we'd base it on DWARF.  Then for the most part,
we'd stop getting into trouble with mapping DWARF constructs to AX.

I imagine we'd reuse gdb/dwarf2expr.c somehow, similarly to get-next-pcs,
and that we'd maybe lower/rewrite some of the the DWARF before passing
it to the target, to e.g., maybe avoid relying on debug info types or
the frame/unwind machinery.

+Gary, since given Infinity is based on DWARF expressions, it may
be Gary's already looked at factoring out gdb/dwarf2expr.c.

Not a trivial project though...

Thanks,
Pedro Alves