Re: [RFA] epilogue unwinder for i386 (reverse 1/2)

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Fri, 03 Jul 2009 17:34:59 -0700
> From: Michael Snyder <msnyder@vmware.com>
> 
> Michael Snyder wrote:
> > This comes out of a discussion with Daniel, about how gcc
> > does not generate the right dwarf info to allow correct
> > frame unwinding in function epilogues, causing frame_unwind
> > to return bad results.
> > 
> > It's necessary for reverse-step, which will frequently step
> > backward to the return instruction of a function.  But it also
> > provides an improvement for forward debugging, in that now,
> > without this change, if you STEPI until you are at the return
> > instruction, you will get a bad backtrace.
> > 
> > The infrun changes that take advantage of this patch will follow
> > separately.
> > 
> > Michael
> 
> Oops, the patch wasn't meant to have that "#if 0" in it...
> corrected patch below.

Still has the #if 0 in there.

I also think you should add a comment about the specific ordering of
this unwinder.  It has to come before the dwarf2 unwinder because GCC
doesn't provide proper CFI for the epilogue, right?

Further comments inline below.

> 2009-07-03  Michael Snyder  <msnyder@vmware.com>
> 
> 	* i386-tdep.c: Add a frame unwinder for function epilogues.
> 	(i386_in_function_epilogue_p): New function.
> 	(i386_epilogue_frame_sniffer): New function.
> 	(i386_epilogue_frame_cache): New function.
> 	(i386_epilogue_frame_this_id): New function.
> 	(i386_epilogue_frame_unwind): New struct frame_unwind.
> 	(i386_gdbarch_init): Hook the new unwinder.
> 
> Index: i386-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386-tdep.c,v
> retrieving revision 1.280
> diff -u -p -r1.280 i386-tdep.c
> --- i386-tdep.c	2 Jul 2009 17:25:54 -0000	1.280
> +++ i386-tdep.c	4 Jul 2009 00:37:12 -0000
> @@ -1487,6 +1487,89 @@ static const struct frame_unwind i386_fr
>    NULL,
>    default_frame_sniffer
>  };
> +
> +/* Normal frames, but in a function epilogue.  */
> +
> +/* The epilogue is defined here as the RET instruction, which will
> +   follow any instruction such as LEAVE or POP EBP that destroys the
> +   function's stack frame.  */
> +
> +static int
> +i386_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> +{
> +  gdb_byte insn;
> +
> +  if (target_read_memory (pc, &insn, 1) != 0)
> +    return 0;	/* Can't read memory at pc.  */

For consistency's sake, can you drop the != 0 here?

> +  if (insn != 0xc3)	/* RET */
> +    return 0;

Please use lowercase for instruction mnemonics.