This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] add 'rs6000_in_function_epilogue_p()' (Revised)
> From: Paul Gilliam <pgilliam@us.ibm.com>
> Date: Thu, 12 Jan 2006 16:23:15 -0800
[ Bleah, I'd really wish people stopped sending MIME mail, especially
with that stupid quoted-printable encoding. I hate editing out all
those gratuitous equal signs. ]
> > I kind of lost track of this. Here is a link to the revised
> > patch. It should address Mark's concerns,
Unfortunately it doesn't.
> 2006-01-11 Paul Gilliam <pgilliam@us.ibm.com>
>
> * ppc-tdep.h: Add a define for the hard limit used when scanning an
> epilogue.
> * rs6000-tdep.c: Add new subroutine, 'rs6000_in_function_epilogue_p()'
> and put it into the architecture vector.
It's probably your stupid mailer that converts tabs into spaces or
something, but please make sure the indentation of your Changelog
entry is ok. Actually the whole entry is wrong. Please *do* read the
GNU coding standards. Your changelog should probably read like this:
2006-01-11 Paul Gilliam <pgilliam@us.ibm.com>
* 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.
Also see a few comments in the code below.
> 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 11 Jan 2006 23:03:43 -0000
> @@ -502,6 +502,114 @@
> 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;
> + /* b = (insn >> 11) & 0x01f */
> + int subcode = (insn >> 1) & 0x3ff;
> + /* rc = insn & 0x001 */
Please remove these comments.
> +/* 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 *fr;
Could you name this variable frame instead of the cryptic fr?
> + /* 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;
> +
> + epilogue_end = pc + PPC_MAX_EPILOGUE_INSTRUCTIONS * PPC_INSN_SIZE;
> + if (epilogue_end > func_end) epilogue_end = func_end;
> +
> + /* Get the current frame. This may be cheap, since we might have
> + just called it in watchpoint_check, before calling
> + gdbarch_in_function_epilogue_p. */
I don't think this comment is actually adding any useful information,
so I'd prefer it if you dropped it.
> + fr = 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 (fr, 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 (fr, 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 */
With those changes, this is ok.
Mark