From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21002 invoked by alias); 13 May 2013 15:23:30 -0000 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 Received: (qmail 20992 invoked by uid 89); 13 May 2013 15:23:30 -0000 X-Spam-SWARE-Status: No, score=-6.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_XZ autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 13 May 2013 15:23:29 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4DFNRIn015885 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 13 May 2013 11:23:27 -0400 Received: from host2.jankratochvil.net (ovpn-116-26.ams2.redhat.com [10.36.116.26]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r4DFNM9p003277 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 13 May 2013 11:23:24 -0400 Date: Mon, 13 May 2013 15:23:00 -0000 From: Jan Kratochvil To: Markus Metzger Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 01/15] gdbarch: add instruction predicate methods Message-ID: <20130513152321.GA29683@host2.jankratochvil.net> References: <1367496216-21217-1-git-send-email-markus.t.metzger@intel.com> <1367496216-21217-2-git-send-email-markus.t.metzger@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1367496216-21217-2-git-send-email-markus.t.metzger@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2013-05/txt/msg00440.txt.bz2 On Thu, 02 May 2013 14:03:22 +0200, Markus Metzger wrote: [...] > diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c > index 3ab74f0..30013b3 100644 > --- a/gdb/amd64-tdep.c > +++ b/gdb/amd64-tdep.c > @@ -1365,6 +1365,22 @@ amd64_absolute_jmp_p (const struct amd64_insn *details) > } > Function comment/description. > static int > +amd64_jmp_p (const struct amd64_insn *details) > +{ > + const gdb_byte *insn = &details->raw_insn[details->opcode_offset]; > + > + /* jump short, relative. */ > + if (insn[0] == 0xeb) > + return 1; > + > + /* jump near, relative. */ > + if (insn[0] == 0xe9) > + return 1; > + > + return amd64_absolute_jmp_p (details); > +} > + > +static int > amd64_absolute_call_p (const struct amd64_insn *details) > { > const gdb_byte *insn = &details->raw_insn[details->opcode_offset]; > @@ -1435,6 +1451,53 @@ amd64_syscall_p (const struct amd64_insn *details, int *lengthp) > return 0; > } > > +/* Classify the instruction at ADDR using PRED. */ + /* Throw error if it cannot be read. */ > + > +static int > +amd64_classify_insn_at (struct gdbarch *gdbarch, CORE_ADDR addr, > + int (*pred) (const struct amd64_insn *)) > +{ > + struct amd64_insn details; > + gdb_byte *buf; > + int len, classification; > + > + len = gdbarch_max_insn_length (gdbarch); > + buf = xzalloc (len); xmalloc is sufficient. But as read_memory can throw we need make_cleanup with xfree. But that seems as too complicated, use just alloca here. (I see there is already the same bug in amd64_relocate_instruction.) > + > + read_memory (addr, buf, len); gdbarch_max_insn_length may be longer than the real instruction and readable memory but the same bug is in amd64_relocate_instruction so it can be fixed in the future together, OK with it as is. > + amd64_get_insn_details (buf, &details); > + > + classification = pred (&details); > + > + xfree (buf); > + > + return classification; > +} > + [...] > --- a/gdb/gdbarch.sh > +++ b/gdb/gdbarch.sh > @@ -972,6 +972,15 @@ m:void:iterate_over_objfiles_in_search_order:iterate_over_objfiles_in_search_ord > > # Ravenscar arch-dependent ops. > v:struct ravenscar_arch_ops *:ravenscar_ops:::NULL:NULL::0:host_address_to_string (gdbarch->ravenscar_ops) > + > +# Return non-zero if the instruction at ADDR is a call; zero otherwise. > +M:int:insn_call_p:CORE_ADDR addr:addr > + > +# Return non-zero if the instruction at ADDR is a return; zero otherwise. > +M:int:insn_ret_p:CORE_ADDR addr:addr > + > +# Return non-zero if the instruction at ADDR is a jump; zero otherwise. > +M:int:insn_jump_p:CORE_ADDR addr:addr These *_p functions with 'M' lead to calls like: if (!gdbarch_insn_call_p_p (gdbarch)) return NULL; There are already existing functions like amd64_ret_p but it would be better not to follow this naming for gdbarch. gdbarch can have methods like insn_is_call, insn_is_ret etc. Additionally you can also provide default function here always returning zero by M:int:insn_is_call:CORE_ADDR addr:addr::default_insn_is_call to simplify if (gdbarch_insn_ret_p_p (gdbarch) && gdbarch_insn_ret_p (gdbarch, lpc)) -> if (gdbarch_insn_ret_p (gdbarch, lpc)) but maybe you are already aware of it and chose it as is intentionally. > EOF > } > > diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c > index 930d6fc..1d011de 100644 > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -473,6 +473,20 @@ i386_absolute_jmp_p (const gdb_byte *insn) > } > Function comment/description. > static int > +i386_jmp_p (const gdb_byte *insn) > +{ > + /* jump short, relative. */ > + if (insn[0] == 0xeb) > + return 1; > + > + /* jump near, relative. */ > + if (insn[0] == 0xe9) > + return 1; > + > + return i386_absolute_jmp_p (insn); > +} > + > +static int > i386_absolute_call_p (const gdb_byte *insn) > { > /* call far, absolute. */ [...] Thanks, Jan