From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15647 invoked by alias); 27 Dec 2010 13:10:12 -0000 Received: (qmail 15524 invoked by uid 22791); 27 Dec 2010 13:10:09 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 27 Dec 2010 13:10:04 +0000 Received: (qmail 7509 invoked from network); 27 Dec 2010 13:10:02 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 27 Dec 2010 13:10:02 -0000 From: Pedro Alves To: Hui Zhu Subject: Re: [RFA/RFC] mips tracepoint: fix Bug 12013 Date: Mon, 27 Dec 2010 13:52:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.33-29-realtime; KDE/4.4.5; x86_64; ; ) Cc: gdb-patches@sourceware.org, Kevin Buettner References: <201012221126.57873.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201012271309.59475.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-12/txt/msg00469.txt.bz2 On Saturday 25 December 2010 17:09:21, Hui Zhu wrote: > On Wed, Dec 22, 2010 at 19:26, Pedro Alves wrote: ... > I make a patch according to your comments. Thanks! I'm liking this. Would be super cool to have these implemented on x86/x86_64 as well. > 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. Well, not to the register mask. It really isn't garanteed that a pseudo register maps to a raw register. It could map to some value stored at some memory address, so the new callback may do other things than flipping a bit in the raw registers to collect mask (hence the ax_reg_mask function name). I can see why you named the callbacks like that, and I'll approve the patch with such names anyway, though I'd prefer naming them something more descriptive like gdbarch_ax_pseudo_register_collect / gdbarch_ax_pseudo_register_push_stack. E.g., # Assemble agent expression bytecode to collect pseudo-register REG. # Return -1 if something goes wrong, 0 otherwise. M:int:ax_pseudo_register_collect:struct agent_expr *ax, int reg:ax, reg # Assemble agent expression bytecode to push the value of pseudo-register # REG on the interpreter stack. # Return -1 if something goes wrong, 0 otherwise. M:int:ax_pseudo_register_push_stack:struct agent_expr *ax, int reg:ax, reg > 2010-12-26 Hui Zhu > > * 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. Capitalize, plural: "New callbacks." > * mips-tdep.c (ax.h): New include. > (mips_ax_pseudo_reg, mips_ax_pseudo_reg_mask): New function. functions. > (mips_gdbarch_init): Set mips_ax_pseudo_reg and > mips_ax_pseudo_reg_mask. You need to mention that gdbarch.c and gdbarch.h were regenerated. > /* Given an agent expression AX, fill in requirements and other descriptive > --- a/gdbarch.sh > +++ b/gdbarch.sh ... > @@ -919,6 +928,7 @@ struct target_desc; > struct displaced_step_closure; > struct core_regset_section; > struct syscall; > +struct agent_expr; You forgot to mention this in the changelog. > +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; Hmm, I'm not sure, but don't you need to sign extend? mips-tdep.c:mips_pseudo_register_read treats this as signed. Why do you apply the "and 0xffffffff" logic when mips64_transfers_32bit_regs_p is false? > + } Please use the functions in ax-general.c that generate the bytecode instead of writing to the bytecode buffer directly (and forgetting to ensure there's enough room in the buffer): ax_const_l/ax_simple, etc. The patch you posted got line-wrapped/mangled, and I couldn't apply it even after trying to manually fix it. Can you please make the necessary tweaks to your email client to make that not happen? (Me, to send patches with gmail, I find it simpler to use an email client / SMTP than using gmail's web interface) -- Pedro Alves