This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 5/6] gdb/x86: Implement ax_pseudo_register_collect hook.
- From: Pedro Alves <palves at redhat dot com>
- To: Marcin KoÅcielnicki <koriakin at 0x04 dot net>, gdb-patches at sourceware dot org
- Date: Wed, 10 Feb 2016 13:31:22 +0000
- Subject: Re: [PATCH 5/6] gdb/x86: Implement ax_pseudo_register_collect hook.
- Authentication-results: sourceware.org; auth=none
- References: <1454773157-31569-1-git-send-email-koriakin at 0x04 dot net> <1454792070-25410-1-git-send-email-koriakin at 0x04 dot net>
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