This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Mark, Daniel, is this OK now?

Michael Snyder wrote:
Mark Kettenis wrote:
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.

Sorry. ;-(
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?

Right. Since the others are similarly order-dependent, I will expand on their comments as well.


+  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?

OK.


+  if (insn != 0xc3)	/* RET */
+    return 0;
Please use lowercase for instruction mnemonics.

OK. Revised patch attached.






------------------------------------------------------------------------

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 5 Jul 2009 20:56:08 -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))
+ return 0; /* Can't read memory at pc. */
+
+ if (insn != 0xc3) /* 'ret' instruction. */
+ return 0;
+
+ return 1;
+}
+
+static int
+i386_epilogue_frame_sniffer (const struct frame_unwind *self,
+ struct frame_info *this_frame,
+ void **this_prologue_cache)
+{
+ if (frame_relative_level (this_frame) == 0)
+ return i386_in_function_epilogue_p (get_frame_arch (this_frame),
+ get_frame_pc (this_frame));
+ else
+ return 0;
+}
+
+static struct i386_frame_cache *
+i386_epilogue_frame_cache (struct frame_info *this_frame, void **this_cache)
+{
+ struct gdbarch *gdbarch = get_frame_arch (this_frame);
+ enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+ struct i386_frame_cache *cache;
+ gdb_byte buf[4];
+
+ if (*this_cache)
+ return *this_cache;
+
+ cache = i386_alloc_frame_cache ();
+ *this_cache = cache;
+
+ /* Cache base will be ESP plus cache->sp_offset (-4). */
+ get_frame_register (this_frame, I386_ESP_REGNUM, buf);
+ cache->base = extract_unsigned_integer (buf, 4, + byte_order) + cache->sp_offset;
+
+ /* Cache pc will be the frame func. */
+ cache->pc = get_frame_pc (this_frame);
+
+ /* The saved ESP will be at cache->base plus 8. */
+ cache->saved_sp = cache->base + 8;
+
+ /* The saved EIP will be at cache->base plus 4. */
+ cache->saved_regs[I386_EIP_REGNUM] = cache->base + 4;
+
+ return cache;
+}
+
+static void
+i386_epilogue_frame_this_id (struct frame_info *this_frame,
+ void **this_cache,
+ struct frame_id *this_id)
+{
+ struct i386_frame_cache *cache = i386_epilogue_frame_cache (this_frame,
+ this_cache);
+
+ (*this_id) = frame_id_build (cache->base + 8, cache->pc);
+}
+
+static const struct frame_unwind i386_epilogue_frame_unwind =
+{
+ NORMAL_FRAME,
+ i386_epilogue_frame_this_id,
+ i386_frame_prev_register,
+ NULL, + i386_epilogue_frame_sniffer
+};

/* Signal trampolines. */
@@ -5329,7 +5412,15 @@ i386_gdbarch_init (struct gdbarch_info i
/* Helper for function argument information. */
set_gdbarch_fetch_pointer_argument (gdbarch, i386_fetch_pointer_argument);
- /* Hook in the DWARF CFI frame unwinder. */
+ /* Hook the function epilogue frame unwinder. This unwinder is
+ appended to the list first, so that it supercedes the Dwarf
+ unwinder in function epilogues (where the Dwarf unwinder
+ currently fails). */
+ frame_unwind_append_unwinder (gdbarch, &i386_epilogue_frame_unwind);
+
+ /* Hook in the DWARF CFI frame unwinder. This unwinder is appended
+ to the list before the prologue-based unwinders, so that Dwarf
+ CFI info will be used if it is available. */
dwarf2_append_unwinders (gdbarch);
frame_base_set_default (gdbarch, &i386_frame_base);
@@ -5337,6 +5428,7 @@ i386_gdbarch_init (struct gdbarch_info i
/* Hook in ABI-specific overrides, if they have been registered. */
gdbarch_init_osabi (info, gdbarch);
+ /* Hook in the legacy prologue-based unwinders last (fallback). */
frame_unwind_append_unwinder (gdbarch, &i386_sigtramp_frame_unwind);
frame_unwind_append_unwinder (gdbarch, &i386_frame_unwind);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]