Re: [RFA/RFC] mips tracepoint: fix Bug 12013

Hui Zhu
2010-12-25T17:09:55Z

On Wed, Dec 22, 2010 at 19:26, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 22 December 2010 06:04:28, Kevin Buettner wrote:
>> You might consider implementing a new gdbarch method which provides a
>> mapping from pseudo register numbers to raw register numbers.  The
>> trace machinery could use such a mapping to find the corresponding raw
>> register(s) when presented with a pseudo register.  I can think of
>> several potential pitfalls with this approach, but I think the idea is
>> worth exploring.
>
> Such mapping will necessarily be a temporary kludge -- if such a mapping
> were always possible, then we wouldn't have
> gdbarch_pseudo_register_read/gdbarch_pseudo_register_read callbacks, but
> we'd instead have a gdbarch_pseudo_register_to_raw_register callback,
> or some such.
>
> If you're going to add support for collecting pseudo registers, I'd
> much rather you added a "gdbarch_pseudo_register_collect" callback,
> that emitted the agent expression bits required to collect whatever
> gdbarch_pseudo_register_read needs to "read" the pseudo register
> in question at trace buffer inspection time (tfind mode).  In the
> mips case, you'd end up with only calls to ax_reg in that new
> callback, it looks to me.
> (Take a look at dwarf2loc.c:compile_dwarf_to_reg and it's ax_reg call,
> for example.)
>
> --
> Pedro Alves
>

Thanks Pedro.

I make a patch according to your comments.
It add to callback in gdbarch. They are gdbarch_ax_pseudo_reg and
gdbarch_ax_pseudo_reg_mask.

gdbarch_ax_pseudo_reg will be called by ax_reg. It assemble code to
push the value of pseudo register number REG on the stack.
gdbarch_ax_pseudo_reg_mask will be called by ax_reg_mask. It add
pseudo register REG to the register mask for expression AX.

And I add function mips_ax_pseudo_reg and mips_ax_pseudo_reg_mask for them.

After this patch, I try the GDB with KGTP in a mips64b board:
(gdb) actions
Enter actions for tracepoint 1, one per line.
End with a line saying just "end".
>collect *(unsigned char *)($sp)@512
>end
(gdb) tstart
(gdb) tstop
(gdb) tfind
Found trace frame 0, tracepoint 1
#0 vfs_readdir (file=0x9800000007b50200, filler=0xffffffff80213f10
<compat_filldir>,
buf=0x9800000005b8fe70) at /home/teawater/kernel/linux-2.6/fs/readdir.c:25
25 struct inode *inode = file->f_path.dentry->d_inode;
(gdb) bt
#0 vfs_readdir (file=0x9800000007b50200, filler=0xffffffff80213f10
<compat_filldir>,
buf=0x9800000005b8fe70) at /home/teawater/kernel/linux-2.6/fs/readdir.c:25
#1 0xffffffff80216208 in compat_sys_getdents (fd=<value optimized
out>, dirent=0x7fa56660, count=5960)
at /home/teawater/kernel/linux-2.6/fs/compat.c:1017
#2 0xffffffff801121a4 in handle_sysn32 ()
at /home/teawater/kernel/linux-2.6/arch/mips/kernel/scall64-n32.S:61

Please help me review it.

Best,
Hui

2010-12-26 Hui Zhu <teawater@gmail.com>

* ax-gdb.c (gen_expr): Remove pseudo-register check code.
* ax-general.c (user-regs.h): New include.
(ax_reg): Call gdbarch_ax_pseudo_reg.
(ax_reg_mask): Call gdbarch_ax_pseudo_reg_mask.
* gdbarch.sh (ax_pseudo_reg, ax_pseudo_reg_mask): new callback.
* mips-tdep.c (ax.h): New include.
(mips_ax_pseudo_reg, mips_ax_pseudo_reg_mask): New function.
(mips_gdbarch_init): Set mips_ax_pseudo_reg and
mips_ax_pseudo_reg_mask.
---
ax-gdb.c | 4 ---
ax-general.c | 75 +++++++++++++++++++++++++++++++++++++++++------------------
gdbarch.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
gdbarch.h | 20 +++++++++++++++
gdbarch.sh | 10 +++++++
mips-tdep.c | 55 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 204 insertions(+), 26 deletions(-)

--- a/ax-gdb.c
+++ b/ax-gdb.c
@@ -1978,10 +1978,6 @@ gen_expr (struct expression *exp, union
if (reg == -1)
internal_error (__FILE__, __LINE__,
_("Register $%s not available"), name);
- if (reg >= gdbarch_num_regs (exp->gdbarch))
- error (_("'%s' is a pseudo-register; "
- "GDB cannot yet trace pseudoregister contents."),
- name);
value->kind = axs_lvalue_register;
value->u.reg = reg;
value->type = register_type (exp->gdbarch, reg);
--- a/ax-general.c
+++ b/ax-general.c
@@ -28,6 +28,8 @@
#include "value.h"
#include "gdb_string.h"

+#include "user-regs.h"
+
static void grow_expr (struct agent_expr *x, int n);

static void append_const (struct agent_expr *x, LONGEST val, int n);
@@ -272,14 +274,28 @@ ax_const_d (struct agent_expr *x, LONGES
void
ax_reg (struct agent_expr *x, int reg)
{
- /* Make sure the register number is in range. */
- if (reg < 0 || reg > 0xffff)
- error (_("GDB bug: ax-general.c (ax_reg): register number out of range"));
- grow_expr (x, 3);
- x->buf[x->len] = aop_reg;
- x->buf[x->len + 1] = (reg >> 8) & 0xff;
- x->buf[x->len + 2] = (reg) & 0xff;
- x->len += 3;
+ if (reg >= gdbarch_num_regs (x->gdbarch))
+ {
+ /* This is a pseudo-register. */
+ if (!gdbarch_ax_pseudo_reg_p (x->gdbarch))
+ error (_("'%s' is a pseudo-register; "
+ "GDB cannot yet trace its contents."),
+ user_reg_map_regnum_to_name (x->gdbarch, reg));
+ if (gdbarch_ax_pseudo_reg (x->gdbarch, x, reg))
+ error (_("Trace '%s' failed."),
+ user_reg_map_regnum_to_name (x->gdbarch, reg));
+ }
+ else
+ {
+ /* Make sure the register number is in range. */
+ if (reg < 0 || reg > 0xffff)
+ error (_("GDB bug: ax-general.c (ax_reg): register number out
of range"));
+ grow_expr (x, 3);
+ x->buf[x->len] = aop_reg;
+ x->buf[x->len + 1] = (reg >> 8) & 0xff;
+ x->buf[x->len + 2] = (reg) & 0xff;
+ x->len += 3;
+ }
}

/* Assemble code to operate on a trace state variable. */
@@ -413,23 +429,38 @@ ax_print (struct ui_file *f, struct agen
void
ax_reg_mask (struct agent_expr *ax, int reg)
{
- int byte = reg / 8;
-
- /* Grow the bit mask if necessary. */
- if (byte >= ax->reg_mask_len)
+ if (reg >= gdbarch_num_regs (ax->gdbarch))
{
- /* It's not appropriate to double here. This isn't a
- string buffer. */
- int new_len = byte + 1;
- unsigned char *new_reg_mask = xrealloc (ax->reg_mask,
- new_len * sizeof (ax->reg_mask[0]));
- memset (new_reg_mask + ax->reg_mask_len, 0,
- (new_len - ax->reg_mask_len) * sizeof (ax->reg_mask[0]));
- ax->reg_mask_len = new_len;
- ax->reg_mask = new_reg_mask;
+ /* This is a pseudo-register. */
+ if (!gdbarch_ax_pseudo_reg_mask_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_reg_mask (ax->gdbarch, ax, reg))
+ error (_("Trace '%s' failed."),
+ user_reg_map_regnum_to_name (ax->gdbarch, reg));
}
+ else
+ {
+ int byte = reg / 8;

- ax->reg_mask[byte] |= 1 << (reg % 8);
+ /* Grow the bit mask if necessary. */
+ if (byte >= ax->reg_mask_len)
+ {
+ /* It's not appropriate to double here. This isn't a
+ string buffer. */
+ int new_len = byte + 1;
+ unsigned char *new_reg_mask = xrealloc (ax->reg_mask,
+ new_len
+ * sizeof (ax->reg_mask[0]));
+ memset (new_reg_mask + ax->reg_mask_len, 0,
+ (new_len - ax->reg_mask_len) * sizeof (ax->reg_mask[0]));
+ ax->reg_mask_len = new_len;
+ ax->reg_mask = new_reg_mask;
+ }
+
+ ax->reg_mask[byte] |= 1 << (reg % 8);
+ }
}

/* Given an agent expression AX, fill in requirements and other descriptive
--- a/gdbarch.c
+++ b/gdbarch.c
@@ -164,6 +164,8 @@ struct gdbarch
gdbarch_pseudo_register_write_ftype *pseudo_register_write;
int num_regs;
int num_pseudo_regs;
+ gdbarch_ax_pseudo_reg_ftype *ax_pseudo_reg;
+ gdbarch_ax_pseudo_reg_mask_ftype *ax_pseudo_reg_mask;
int sp_regnum;
int pc_regnum;
int ps_regnum;
@@ -314,6 +316,8 @@ struct gdbarch startup_gdbarch =
0, /* pseudo_register_write */
0, /* num_regs */
0, /* num_pseudo_regs */
+ 0, /* ax_pseudo_reg */
+ 0, /* ax_pseudo_reg_mask */
-1, /* sp_regnum */
-1, /* pc_regnum */
-1, /* ps_regnum */
@@ -594,6 +598,8 @@ verify_gdbarch (struct gdbarch *gdbarch)
if (gdbarch->num_regs == -1)
fprintf_unfiltered (log, "\n\tnum_regs");
/* Skip verify of num_pseudo_regs, invalid_p == 0 */
+ /* Skip verify of ax_pseudo_reg, has predicate */
+ /* Skip verify of ax_pseudo_reg_mask, has predicate */
/* Skip verify of sp_regnum, invalid_p == 0 */
/* Skip verify of pc_regnum, invalid_p == 0 */
/* Skip verify of ps_regnum, invalid_p == 0 */
@@ -761,6 +767,18 @@ gdbarch_dump (struct gdbarch *gdbarch, s
"gdbarch_dump: auto_wide_charset = <%s>\n",
host_address_to_string (gdbarch->auto_wide_charset));
fprintf_unfiltered (file,
+ "gdbarch_dump: gdbarch_ax_pseudo_reg_p() = %d\n",
+ gdbarch_ax_pseudo_reg_p (gdbarch));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: ax_pseudo_reg = <%s>\n",
+ host_address_to_string (gdbarch->ax_pseudo_reg));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: gdbarch_ax_pseudo_reg_mask_p() = %d\n",
+ gdbarch_ax_pseudo_reg_mask_p (gdbarch));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: ax_pseudo_reg_mask = <%s>\n",
+ host_address_to_string (gdbarch->ax_pseudo_reg_mask));
+ fprintf_unfiltered (file,
"gdbarch_dump: believe_pcc_promotion = %s\n",
plongest (gdbarch->believe_pcc_promotion));
fprintf_unfiltered (file,
@@ -1741,6 +1759,54 @@ set_gdbarch_num_pseudo_regs (struct gdba
}

int
+gdbarch_ax_pseudo_reg_p (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ return gdbarch->ax_pseudo_reg != NULL;
+}
+
+int
+gdbarch_ax_pseudo_reg (struct gdbarch *gdbarch, struct agent_expr *ax, int reg)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->ax_pseudo_reg != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_ax_pseudo_reg called\n");
+ return gdbarch->ax_pseudo_reg (gdbarch, ax, reg);
+}
+
+void
+set_gdbarch_ax_pseudo_reg (struct gdbarch *gdbarch,
+ gdbarch_ax_pseudo_reg_ftype ax_pseudo_reg)
+{
+ gdbarch->ax_pseudo_reg = ax_pseudo_reg;
+}
+
+int
+gdbarch_ax_pseudo_reg_mask_p (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ return gdbarch->ax_pseudo_reg_mask != NULL;
+}
+
+int
+gdbarch_ax_pseudo_reg_mask (struct gdbarch *gdbarch, struct
agent_expr *ax, int reg)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->ax_pseudo_reg_mask != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_ax_pseudo_reg_mask called\n");
+ return gdbarch->ax_pseudo_reg_mask (gdbarch, ax, reg);
+}
+
+void
+set_gdbarch_ax_pseudo_reg_mask (struct gdbarch *gdbarch,
+ gdbarch_ax_pseudo_reg_mask_ftype
ax_pseudo_reg_mask)
+{
+ gdbarch->ax_pseudo_reg_mask = ax_pseudo_reg_mask;
+}
+
+int
gdbarch_sp_regnum (struct gdbarch *gdbarch)
{
gdb_assert (gdbarch != NULL);
--- a/gdbarch.h
+++ b/gdbarch.h
@@ -53,6 +53,7 @@ struct target_desc;
struct displaced_step_closure;
struct core_regset_section;
struct syscall;
+struct agent_expr;

/* The architecture associated with the connection to the target.

@@ -232,6 +233,25 @@ extern void set_gdbarch_num_regs (struct
extern int gdbarch_num_pseudo_regs (struct gdbarch *gdbarch);
extern void set_gdbarch_num_pseudo_regs (struct gdbarch *gdbarch, int
num_pseudo_regs);

+/* Assemble code to push the value of pseudo register number REG on the
+ stack.
+ Return -1 if something goes wrong, 0 otherwise. */
+
+extern int gdbarch_ax_pseudo_reg_p (struct gdbarch *gdbarch);
+
+typedef int (gdbarch_ax_pseudo_reg_ftype) (struct gdbarch *gdbarch,
struct agent_expr *ax, int reg);
+extern int gdbarch_ax_pseudo_reg (struct gdbarch *gdbarch, struct
agent_expr *ax, int reg);
+extern void set_gdbarch_ax_pseudo_reg (struct gdbarch *gdbarch,
gdbarch_ax_pseudo_reg_ftype *ax_pseudo_reg);
+
+/* Add pseudo register REG to the register mask for expression AX.
+ Return -1 if something goes wrong, 0 otherwise. */
+
+extern int gdbarch_ax_pseudo_reg_mask_p (struct gdbarch *gdbarch);
+
+typedef int (gdbarch_ax_pseudo_reg_mask_ftype) (struct gdbarch
*gdbarch, struct agent_expr *ax, int reg);
+extern int gdbarch_ax_pseudo_reg_mask (struct gdbarch *gdbarch,
struct agent_expr *ax, int reg);
+extern void set_gdbarch_ax_pseudo_reg_mask (struct gdbarch *gdbarch,
gdbarch_ax_pseudo_reg_mask_ftype *ax_pseudo_reg_mask);
+
/* GDB's standard (or well known) register numbers. These can map onto
a real register or a pseudo (computed) register or not be defined at
all (-1).
--- a/gdbarch.sh
+++ b/gdbarch.sh
@@ -427,6 +427,15 @@ v:int:num_regs:::0:-1
# combinations of other registers, or they may be computed by GDB.
v:int:num_pseudo_regs:::0:0::0

+# Assemble code to push the value of pseudo register number REG on the
+# stack.
+# Return -1 if something goes wrong, 0 otherwise.
+M:int:ax_pseudo_reg:struct agent_expr *ax, int reg:ax, reg
+
+# Add pseudo register REG to the register mask for expression AX.
+# Return -1 if something goes wrong, 0 otherwise.
+M:int:ax_pseudo_reg_mask:struct agent_expr *ax, int reg:ax, reg
+
# GDB's standard (or well known) register numbers. These can map onto
# a real register or a pseudo (computed) register or not be defined at
# all (-1).
@@ -919,6 +928,7 @@ struct target_desc;
struct displaced_step_closure;
struct core_regset_section;
struct syscall;
+struct agent_expr;

/* The architecture associated with the connection to the target.

--- a/mips-tdep.c
+++ b/mips-tdep.c
@@ -58,6 +58,7 @@
#include "dwarf2-frame.h"
#include "user-regs.h"
#include "valprint.h"
+#include "ax.h"

static const struct objfile_data *mips_pdr_data;

@@ -616,6 +617,57 @@ mips_pseudo_register_write (struct gdbar
internal_error (__FILE__, __LINE__, _("bad register size"));
}

+static int
+mips_ax_pseudo_reg (struct gdbarch *gdbarch, struct agent_expr *ax, int reg)
+{
+ int rawnum = reg % gdbarch_num_regs (gdbarch);
+ gdb_assert (reg >= gdbarch_num_regs (gdbarch)
+ && reg < 2 * gdbarch_num_regs (gdbarch));
+ if (register_size (gdbarch, rawnum) >= register_size (gdbarch, reg))
+ {
+ ax_reg (ax, rawnum);
+
+ if (register_size (gdbarch, rawnum) > register_size (gdbarch, reg))
+ {
+ if (gdbarch_tdep (gdbarch)->mips64_transfers_32bit_regs_p
+ && gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
+ {
+ ax->buf[ax->len] = aop_const8;
+ ax->buf[ax->len + 1] = 32;
+ ax->buf[ax->len + 2] = aop_rsh_unsigned;
+ ax->len += 3;
+ }
+ else
+ {
+ ax->buf[ax->len] = aop_const32;
+ ax->buf[ax->len + 1] = 0xff;
+ ax->buf[ax->len + 2] = 0xff;
+ ax->buf[ax->len + 3] = 0xff;
+ ax->buf[ax->len + 4] = 0xff;
+ ax->buf[ax->len + 5] = aop_bit_and;
+ ax->len += 6;
+ }
+ }
+ }
+ else
+ internal_error (__FILE__, __LINE__, _("bad register size"));
+
+ return 0;
+}
+
+static int
+mips_ax_pseudo_reg_mask (struct gdbarch *gdbarch,
+ struct agent_expr *ax, int reg)
+{
+ int rawnum = reg % gdbarch_num_regs (gdbarch);
+ gdb_assert (reg >= gdbarch_num_regs (gdbarch)
+ && reg < 2 * gdbarch_num_regs (gdbarch));
+
+ ax_reg_mask (ax, rawnum);
+
+ return 0;
+}
+
/* Table to translate MIPS16 register field to actual register number. */
static int mips16_to_32_reg[8] = { 16, 17, 2, 3, 4, 5, 6, 7 };

@@ -5933,6 +5985,9 @@ mips_gdbarch_init (struct gdbarch_info i
set_gdbarch_pseudo_register_read (gdbarch, mips_pseudo_register_read);
set_gdbarch_pseudo_register_write (gdbarch, mips_pseudo_register_write);

+ set_gdbarch_ax_pseudo_reg (gdbarch, mips_ax_pseudo_reg);
+ set_gdbarch_ax_pseudo_reg_mask (gdbarch, mips_ax_pseudo_reg_mask);
+
set_gdbarch_elf_make_msymbol_special (gdbarch,
mips_elf_make_msymbol_special);