From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32592 invoked by alias); 9 Feb 2006 17:46:54 -0000 Received: (qmail 32578 invoked by uid 22791); 9 Feb 2006 17:46:53 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 09 Feb 2006 17:46:51 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id k19Hkmqm001368 for ; Thu, 9 Feb 2006 12:46:48 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id k19Hkm115672 for ; Thu, 9 Feb 2006 12:46:48 -0500 Received: from localhost.localdomain (vpn50-79.rdu.redhat.com [172.16.50.79]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id k19HklOb005394 for ; Thu, 9 Feb 2006 12:46:48 -0500 Received: from ironwood.lan (ironwood.lan [192.168.64.8]) by localhost.localdomain (8.12.11/8.12.10) with ESMTP id k19HlOOG010273 for ; Thu, 9 Feb 2006 10:47:24 -0700 Date: Thu, 09 Feb 2006 17:46:00 -0000 From: Kevin Buettner To: gdb-patches@sources.redhat.com Subject: Re: [PATCH] add 'rs6000_in_function_epilogue_p()' (Revised) Message-ID: <20060209104652.74d56d20@ironwood.lan> In-Reply-To: <200601161915.54240.pgilliam@us.ibm.com> References: <200511301225.56802.pgilliam@us.ibm.com> <200601121623.15363.pgilliam@us.ibm.com> <200601132105.k0DL5ZcF011764@elgar.sibelius.xs4all.nl> <200601161915.54240.pgilliam@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-02/txt/msg00214.txt.bz2 Paul, I apologize for taking so long to get back to this. You're patch is okay to check in so long as a few changes to the formatting are made. See below. There's no need to ask for further approval on your updated patch, but do post the patch that you end up committing. Thanks, Kevin On Mon, 16 Jan 2006 19:15:54 -0800 Paul Gilliam wrote: > 2006-01-16 Paul Gilliam > > * ppc-tdep.h (PPC_MAX_EPILOGUE_INSTRUCTIONS): New define. > * rs6000-tdep.c (insn_changes_sp_or_jumps) > (rs6000_in_function_epilogue_p): New functions. > (rs6000_gdbarch_init): Set in_function_epilogue_p. > > Index: ppc-tdep.h > =================================================================== > RCS file: /cvs/src/src/gdb/ppc-tdep.h,v > retrieving revision 1.48 > diff -a -u -r1.48 ppc-tdep.h > --- ppc-tdep.h 28 Oct 2005 18:23:32 -0000 1.48 > +++ ppc-tdep.h 17 Jan 2006 02:14:16 -0000 > @@ -384,4 +384,7 @@ > /* Instruction size. */ > #define PPC_INSN_SIZE 4 > > +/* Estimate for the maximum number of instrctions in a function epilogue. */ > +#define PPC_MAX_EPILOGUE_INSTRUCTIONS 52 > + > #endif /* ppc-tdep.h */ > Index: rs6000-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v > retrieving revision 1.248 > diff -a -u -r1.248 rs6000-tdep.c > --- rs6000-tdep.c 1 Nov 2005 19:32:36 -0000 1.248 > +++ rs6000-tdep.c 17 Jan 2006 02:14:21 -0000 > @@ -502,6 +502,108 @@ > return pc; > } > > +static int > +insn_changes_sp_or_jumps (unsigned long insn) > +{ > + int opcode = (insn >> 26) & 0x03f; > + int sd = (insn >> 21) & 0x01f; > + int a = (insn >> 16) & 0x01f; > + int subcode = (insn >> 1) & 0x3ff; > + > + /* Changes the stack pointer. */ > + > + /* NOTE: There are many ways to change the value of a given register. > + The ways below are those used when the register is R1, the SP, > + in a funtion's epilogue. */ > + > + if (opcode == 31 && subcode == 444 && a == 1) > + return 1; /* mr R1,Rn */ > + if (opcode == 14 && sd == 1) > + return 1; /* addi R1,Rn,simm */ > + if (opcode == 58 && sd == 1) > + return 1; /* ld R1,ds(Rn) */ > + > + /* Transfers control. */ > + > + if (opcode == 18) > + return 1; /* b */ > + if (opcode == 16) > + return 1; /* bc */ > + if (opcode == 19 && subcode == 16) > + return 1; /* bclr */ > + if (opcode == 19 && subcode == 528) > + return 1; /* bcctr */ > + > + return 0; > +} > + > +/* Return true if we are in the function's epilogue, i.e. after the > + instruction that destroyed the function's stack frame. > + > + 1) scan forward from the point of execution: > + a) If you find an instruction that modifies the stack pointer > + or transfers control (except a return), execution is not in > + an epilogue, return. > + b) Stop scanning if you find a return instruction or reach the > + end of the function or reach the hard limit for the size of > + an epilogue. > + 2) scan backward from the point of execution: > + a) If you find an instruction that modifies the stack pointer, > + execution *is* in an epilogue, return. > + b) Stop scanning if you reach an instruction that transfers > + control or the beginning of the function or reach the hard > + limit for the size of an epilogue. */ > + > +static int > +rs6000_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc) > +{ > + bfd_byte insn_buf[PPC_INSN_SIZE]; > + CORE_ADDR scan_pc, func_start, func_end, epilogue_start, epilogue_end; > + unsigned long insn; > + struct frame_info *curfrm; I see that Mark suggested using the name `frame' instead of `curfrm'. This is okay with me. As Mark indicates, naming it `frame' will make it easy to revise this function in the future in the event that a frame is passed in instead of a pc value. > + > + /* Find the search limits based on function boundaries and hard limit. */ > + > + if (!find_pc_partial_function (pc, NULL, &func_start, &func_end)) > + return 0; > + > + epilogue_start = pc - PPC_MAX_EPILOGUE_INSTRUCTIONS * PPC_INSN_SIZE; > + if (epilogue_start < func_start) epilogue_start = func_start; Please put `epilogue_start = func_start;' on its own line indented two spaces from the column of the `if'. > + > + epilogue_end = pc + PPC_MAX_EPILOGUE_INSTRUCTIONS * PPC_INSN_SIZE; > + if (epilogue_end > func_end) epilogue_end = func_end; Likewise here. > + > + curfrm = get_current_frame (); > + > + /* Scan forward until next 'blr'. */ > + > + for (scan_pc = pc; scan_pc < epilogue_end; scan_pc += PPC_INSN_SIZE) > + { > + if (!safe_frame_unwind_memory (curfrm, scan_pc, insn_buf, PPC_INSN_SIZE)) > + return 0; > + insn = extract_signed_integer (insn_buf, PPC_INSN_SIZE); > + if (insn == 0x4e800020) > + break; > + if (insn_changes_sp_or_jumps (insn)) > + return 0; > + } > + > + /* Scan backward until adjustment to stack pointer (R1). */ > + > + for (scan_pc = pc - PPC_INSN_SIZE; > + scan_pc >= epilogue_start; > + scan_pc -= PPC_INSN_SIZE) > + { > + if (!safe_frame_unwind_memory (curfrm, scan_pc, insn_buf, PPC_INSN_SIZE)) > + return 0; > + insn = extract_signed_integer (insn_buf, PPC_INSN_SIZE); > + if (insn_changes_sp_or_jumps (insn)) > + return 1; > + } > + > + return 0; > +} > + > > /* Fill in fi->saved_regs */ > > @@ -3342,6 +3444,8 @@ > set_gdbarch_deprecated_extract_struct_value_address (gdbarch, rs6000_extract_struct_value_address); > > set_gdbarch_skip_prologue (gdbarch, rs6000_skip_prologue); > + set_gdbarch_in_function_epilogue_p (gdbarch, rs6000_in_function_epilogue_p); > + > set_gdbarch_inner_than (gdbarch, core_addr_lessthan); > set_gdbarch_breakpoint_from_pc (gdbarch, rs6000_breakpoint_from_pc);