[5/6] gdb/x86: Implement ax_pseudo_register_collect hook.
Commit Message
Makes "collect $ymm15" action work.
gdb/ChangeLog:
* amd64-tdep.c (amd64_ax_pseudo_register_collect): New function.
(amd64_init_abi): Fill ax_pseudo_register_collect hook.
* i386-tdep.c (i386_ax_pseudo_register_collect): New function.
(i386_gdbarch_init): Fill ax_pseudo_register_collect hook.
* i386-tdep.h: Add i386_ax_pseudo_register_collect prototype.
---
gdb/ChangeLog | 8 ++++++
gdb/amd64-tdep.c | 26 ++++++++++++++++++
gdb/i386-tdep.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/i386-tdep.h | 4 +++
4 files changed, 119 insertions(+)
Comments
Great stuff. Minor comments follow...
Patch is missing intro comments to new functions:
/* Implementation of foo. / See foo.h. / etc. */
On 02/06/2016 08:54 PM, Marcin Kościelnicki wrote:
> +static int
> +amd64_ax_pseudo_register_collect (struct gdbarch *gdbarch,
> + struct agent_expr *ax, int regnum)
> +{
> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
Empty line after decl here.
> +int
> +i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
> + struct agent_expr *ax, int regnum)
> +{
> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
Ditto.
> + if (i386_mmx_regnum_p (gdbarch, regnum))
> + {
> + /* MMX to FPU register mapping depends on current TOS. Let's just
> + not care and collect everything... */
> + int i;
Ditto. (several instances more in the function)
> + ax_reg_mask (ax, I387_FSTAT_REGNUM (tdep));
> + for (i = 0; i < 8; i++)
> + ax_reg_mask (ax, I387_ST0_REGNUM (tdep) + i);
> + return 0;
> + }
> + else if (i386_byte_regnum_p (gdbarch, regnum))
> + {
> + /* Check byte pseudo registers last since this function will
> + be called from amd64_ax_pseudo_register_collect, which handles
> + byte pseudo registers differently. */
I don't understand this comment. amd64_ax_pseudo_register_collect
checks i386_byte_regnum_p before ever reaching here, afaics.
> + int gpnum = regnum - tdep->al_regnum;
> + ax_reg_mask (ax, gpnum % 4);
> + return 0;
> + }
> + else
> + internal_error (__FILE__, __LINE__, _("invalid regnum"));
> + return 1;
> +}
Thanks,
Pedro Alves
On 10/02/16 14:31, Pedro Alves wrote:
> Great stuff. Minor comments follow...
>
> Patch is missing intro comments to new functions:
>
> /* Implementation of foo. / See foo.h. / etc. */
OK.
>
> On 02/06/2016 08:54 PM, Marcin Kościelnicki wrote:
>
>> +static int
>> +amd64_ax_pseudo_register_collect (struct gdbarch *gdbarch,
>> + struct agent_expr *ax, int regnum)
>> +{
>> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
> Empty line after decl here.
OK.
>
>
>> +int
>> +i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
>> + struct agent_expr *ax, int regnum)
>> +{
>> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
> Ditto.
>
>> + if (i386_mmx_regnum_p (gdbarch, regnum))
>> + {
>> + /* MMX to FPU register mapping depends on current TOS. Let's just
>> + not care and collect everything... */
>> + int i;
>
> Ditto. (several instances more in the function)
>
>> + ax_reg_mask (ax, I387_FSTAT_REGNUM (tdep));
>> + for (i = 0; i < 8; i++)
>> + ax_reg_mask (ax, I387_ST0_REGNUM (tdep) + i);
>> + return 0;
>> + }
>
>
>> + else if (i386_byte_regnum_p (gdbarch, regnum))
>> + {
>> + /* Check byte pseudo registers last since this function will
>> + be called from amd64_ax_pseudo_register_collect, which handles
>> + byte pseudo registers differently. */
>
> I don't understand this comment. amd64_ax_pseudo_register_collect
> checks i386_byte_regnum_p before ever reaching here, afaics.
I've copied it from i386_pseudo_register_read_into_value - didn't make
that much sense to me either. Let's remove it from both places?
>
>> + int gpnum = regnum - tdep->al_regnum;
>> + ax_reg_mask (ax, gpnum % 4);
>> + return 0;
>> + }
>> + else
>> + internal_error (__FILE__, __LINE__, _("invalid regnum"));
>> + return 1;
>> +}
>
> Thanks,
> Pedro Alves
>
On 02/10/2016 01:34 PM, Marcin Kościelnicki wrote:
>> I don't understand this comment. amd64_ax_pseudo_register_collect
>> checks i386_byte_regnum_p before ever reaching here, afaics.
>
> I've copied it from i386_pseudo_register_read_into_value - didn't make
> that much sense to me either. Let's remove it from both places?
Yes please.
Thanks,
Pedro Alves
@@ -1,5 +1,13 @@
2016-02-06 Marcin Kościelnicki <koriakin@0x04.net>
+ * amd64-tdep.c (amd64_ax_pseudo_register_collect): New function.
+ (amd64_init_abi): Fill ax_pseudo_register_collect hook.
+ * i386-tdep.c (i386_ax_pseudo_register_collect): New function.
+ (i386_gdbarch_init): Fill ax_pseudo_register_collect hook.
+ * i386-tdep.h: Add i386_ax_pseudo_register_collect prototype.
+
+2016-02-06 Marcin Kościelnicki <koriakin@0x04.net>
+
* tracefile-tfile.c (tfile_fetch_registers): Fix off-by-one in bounds
check.
@@ -455,6 +455,30 @@ amd64_pseudo_register_write (struct gdbarch *gdbarch,
i386_pseudo_register_write (gdbarch, regcache, regnum, buf);
}
+static int
+amd64_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+ struct agent_expr *ax, int regnum)
+{
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ if (i386_byte_regnum_p (gdbarch, regnum))
+ {
+ int gpnum = regnum - tdep->al_regnum;
+ if (gpnum >= AMD64_NUM_LOWER_BYTE_REGS)
+ ax_reg_mask (ax, gpnum - AMD64_NUM_LOWER_BYTE_REGS);
+ else
+ ax_reg_mask (ax, gpnum);
+ return 0;
+ }
+ else if (i386_dword_regnum_p (gdbarch, regnum))
+ {
+ int gpnum = regnum - tdep->eax_regnum;
+ ax_reg_mask (ax, gpnum);
+ return 0;
+ }
+ else
+ return i386_ax_pseudo_register_collect (gdbarch, ax, regnum);
+}
+
/* Register classes as defined in the psABI. */
@@ -2997,6 +3021,8 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
amd64_pseudo_register_read_value);
set_gdbarch_pseudo_register_write (gdbarch,
amd64_pseudo_register_write);
+ set_gdbarch_ax_pseudo_register_collect (gdbarch,
+ amd64_ax_pseudo_register_collect);
set_tdesc_pseudo_register_name (gdbarch, amd64_pseudo_register_name);
@@ -3603,6 +3603,85 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
internal_error (__FILE__, __LINE__, _("invalid regnum"));
}
}
+
+int
+i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+ struct agent_expr *ax, int regnum)
+{
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ if (i386_mmx_regnum_p (gdbarch, regnum))
+ {
+ /* MMX to FPU register mapping depends on current TOS. Let's just
+ not care and collect everything... */
+ int i;
+ ax_reg_mask (ax, I387_FSTAT_REGNUM (tdep));
+ for (i = 0; i < 8; i++)
+ ax_reg_mask (ax, I387_ST0_REGNUM (tdep) + i);
+ return 0;
+ }
+ else if (i386_bnd_regnum_p (gdbarch, regnum))
+ {
+ regnum -= tdep->bnd0_regnum;
+ ax_reg_mask (ax, I387_BND0R_REGNUM (tdep) + regnum);
+ return 0;
+ }
+ else if (i386_k_regnum_p (gdbarch, regnum))
+ {
+ regnum -= tdep->k0_regnum;
+ ax_reg_mask (ax, tdep->k0_regnum + regnum);
+ return 0;
+ }
+ else if (i386_zmm_regnum_p (gdbarch, regnum))
+ {
+ regnum -= tdep->zmm0_regnum;
+ if (regnum < num_lower_zmm_regs)
+ {
+ ax_reg_mask (ax, I387_XMM0_REGNUM (tdep) + regnum);
+ ax_reg_mask (ax, tdep->ymm0h_regnum + regnum);
+ }
+ else
+ {
+ ax_reg_mask (ax, I387_XMM16_REGNUM (tdep) + regnum
+ - num_lower_zmm_regs);
+ ax_reg_mask (ax, I387_YMM16H_REGNUM (tdep) + regnum
+ - num_lower_zmm_regs);
+ }
+ ax_reg_mask (ax, tdep->zmm0h_regnum + regnum);
+ return 0;
+ }
+ else if (i386_ymm_regnum_p (gdbarch, regnum))
+ {
+ regnum -= tdep->ymm0_regnum;
+ ax_reg_mask (ax, I387_XMM0_REGNUM (tdep) + regnum);
+ ax_reg_mask (ax, tdep->ymm0h_regnum + regnum);
+ return 0;
+ }
+ else if (i386_ymm_avx512_regnum_p (gdbarch, regnum))
+ {
+ regnum -= tdep->ymm16_regnum;
+ ax_reg_mask (ax, I387_XMM16_REGNUM (tdep) + regnum);
+ ax_reg_mask (ax, tdep->ymm16h_regnum + regnum);
+ return 0;
+ }
+ else if (i386_word_regnum_p (gdbarch, regnum))
+ {
+ int gpnum = regnum - tdep->ax_regnum;
+ ax_reg_mask (ax, gpnum);
+ return 0;
+ }
+ else if (i386_byte_regnum_p (gdbarch, regnum))
+ {
+ /* Check byte pseudo registers last since this function will
+ be called from amd64_ax_pseudo_register_collect, which handles
+ byte pseudo registers differently. */
+ int gpnum = regnum - tdep->al_regnum;
+ ax_reg_mask (ax, gpnum % 4);
+ return 0;
+ }
+ else
+ internal_error (__FILE__, __LINE__, _("invalid regnum"));
+ return 1;
+}
/* Return the register number of the register allocated by GCC after
@@ -8423,6 +8502,8 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
set_gdbarch_pseudo_register_read_value (gdbarch,
i386_pseudo_register_read_value);
set_gdbarch_pseudo_register_write (gdbarch, i386_pseudo_register_write);
+ set_gdbarch_ax_pseudo_register_collect (gdbarch,
+ i386_ax_pseudo_register_collect);
set_tdesc_pseudo_register_type (gdbarch, i386_pseudo_register_type);
set_tdesc_pseudo_register_name (gdbarch, i386_pseudo_register_name);
@@ -360,6 +360,10 @@ extern void i386_pseudo_register_write (struct gdbarch *gdbarch,
struct regcache *regcache,
int regnum, const gdb_byte *buf);
+extern int i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+ struct agent_expr *ax,
+ int regnum);
+
/* Segment selectors. */
#define I386_SEL_RPL 0x0003 /* Requester's Privilege Level mask. */
#define I386_SEL_UPL 0x0003 /* User Privilige Level. */