Frame lost after 'leave' and before 'ret' instruction on i386?

Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Hello,

I noticed that after 'leave' asm instruction (and before 'ret') one frame will be lost (skipped).

For example:

(gdb) disassemble doSth
Dump of assembler code for function doSth:
0x080484f4 <doSth+0>:   push   %ebp
0x080484f5 <doSth+1>:   mov    %esp,%ebp
0x080484f7 <doSth+3>:   sub    $0x8,%esp
0x080484fa <doSth+6>:   sub    $0xc,%esp
0x080484fd <doSth+9>:   push   $0x8048570
0x08048502 <doSth+14>:  call   0x8048348 <printf@plt>
0x08048507 <doSth+19>:  add    $0x10,%esp
0x0804850a <doSth+22>:  leave
0x0804850b <doSth+23>:  ret
End of assembler dump.
(gdb) info registers
...
eip            0x804850b        0x804850b <doSth+23>
...
(gdb) bt
#0  0x080483c3 in doSth () at main.c:6
#1  0xb7ebffdc in __libc_start_main () from /lib/libc.so.6
#2  0x08048331 in _start ()

(note 'main' that really made the call is missing... e.g., just before "leave" was executed:
(gdb) bt
#0  doSth () at main.c:6
#1  0x080483da in main () at main.c:10
)

What happens is, we have restored the stack pointer but gdb is unaware of that fact and tries to unwind using already unwound value...

how is this supposed to work?

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Daniel Jacobowitz]

On Fri, May 16, 2008 at 11:28:20AM -0400, Aleksandar Ristovski wrote:
> What happens is, we have restored the stack pointer but gdb is unaware of that fact and tries to unwind using already unwound value...
>
> how is this supposed to work?

Poorly.

The biggest problem, in my opinion, is that GCC emits bogus unwind
info that does not describe epilogues.  I don't know about other
compilers.  So far no one has fixed this in a way that didn't bloat
debug info to an unacceptable size.

When DWARF unwinding is not in use, there's the in_epilogue_p hook.
Prologue analyzers could also detect epilogues using the same
technique.  But this is slow and complicated.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Daniel Jacobowitz wrote:
> On Fri, May 16, 2008 at 11:28:20AM -0400, Aleksandar Ristovski wrote:
>> What happens is, we have restored the stack pointer but gdb is unaware of that fact and tries to unwind using already unwound value...
>>
>> how is this supposed to work?
> 
> Poorly.
> 
> The biggest problem, in my opinion, is that GCC emits bogus unwind
> info that does not describe epilogues.  I don't know about other
> compilers.  So far no one has fixed this in a way that didn't bloat
> debug info to an unacceptable size.
> 
> When DWARF unwinding is not in use, there's the in_epilogue_p hook.
> Prologue analyzers could also detect epilogues using the same
> technique.  But this is slow and complicated.
> 

assuming the problem occurs only while single stepping, couldn't we look for 'ret' 'leave' and such and 'figure out' that stack has changed and are now stepping through still the same function but with previous frame?

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Daniel Jacobowitz]

On Fri, May 16, 2008 at 12:02:29PM -0400, Aleksandar Ristovski wrote:
> assuming the problem occurs only while single stepping

It doesn't; it also breaks software watchpoints, breakpoints in the
epilogue, and random stops with control-c.  So we don't know how we
got to the current instruction, which makes it tricky to scan backwards.

It could probably still be done though.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Mark Kettenis]

> Date: Fri, 16 May 2008 11:28:20 -0400
> From: Aleksandar Ristovski <aristovski@qnx.com>
> 
> Hello,
> 
> I noticed that after 'leave' asm instruction (and before 'ret') one frame will be lost (skipped).
> 
> For example:
> 
> (gdb) disassemble doSth
> Dump of assembler code for function doSth:
> 0x080484f4 <doSth+0>:   push   %ebp
> 0x080484f5 <doSth+1>:   mov    %esp,%ebp
> 0x080484f7 <doSth+3>:   sub    $0x8,%esp
> 0x080484fa <doSth+6>:   sub    $0xc,%esp
> 0x080484fd <doSth+9>:   push   $0x8048570
> 0x08048502 <doSth+14>:  call   0x8048348 <printf@plt>
> 0x08048507 <doSth+19>:  add    $0x10,%esp
> 0x0804850a <doSth+22>:  leave
> 0x0804850b <doSth+23>:  ret
> End of assembler dump.
> (gdb) info registers
> ...
> eip            0x804850b        0x804850b <doSth+23>
> ...
> (gdb) bt
> #0  0x080483c3 in doSth () at main.c:6
> #1  0xb7ebffdc in __libc_start_main () from /lib/libc.so.6
> #2  0x08048331 in _start ()
> 
> (note 'main' that really made the call is missing... e.g., just before "leave" was executed:
> (gdb) bt
> #0  doSth () at main.c:6
> #1  0x080483da in main () at main.c:10
> )
> 
> What happens is, we have restored the stack pointer but gdb is unaware of that fact and tries to unwind using already unwound value...
> 
> how is this supposed to work?

The compiler is supposed to give us unwind info for the function's
epilogue, but unfortunately GCC doesn't.  There's not much we can do
about it in GDB.  Fortunately it isnt an issue that users see a lot.

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Mark Kettenis wrote:
> The compiler is supposed to give us unwind info for the function's
> epilogue, but unfortunately GCC doesn't.  There's not much we can do
> about it in GDB.  Fortunately it isnt an issue that users see a lot.
> 

Out of curiosity: what is the problem with specifying epilogue? Am I wrong when I say that there can be only so many epilogue types (i.e. compiler will not generate infinite number of epilogues)? Therefore, only one index in a 'epilogue description table' would suffice...

(or am I over-simplifying?)

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Daniel Jacobowitz]

On Fri, May 16, 2008 at 01:28:50PM -0400, Aleksandar Ristovski wrote:
> Out of curiosity: what is the problem with specifying epilogue? Am I
> wrong when I say that there can be only so many epilogue types
> (i.e. compiler will not generate infinite number of epilogues)?

Any time the epilogue is multi-instruction, the compiler is free to
schedule instructions in the middle of it, and GCC does.  Plus it
varies based on the size of the stack frame.  On some platforms an
epilogue is hard to distinguish from an indirect function call.
Sibling calls have mini-epilogues in front of them, too.  And there
can be lots of copies of the epilogue, and they don't have to be at
the end of the function; they can be inlined at return sites.

So yes, it's a little harder than that :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Mark Kettenis]

> Date: Fri, 16 May 2008 13:28:50 -0400
> From: Aleksandar Ristovski <aristovski@qnx.com>
> 
> Mark Kettenis wrote:
> > The compiler is supposed to give us unwind info for the function's
> > epilogue, but unfortunately GCC doesn't.  There's not much we can do
> > about it in GDB.  Fortunately it isnt an issue that users see a lot.
> > 
> 
> Out of curiosity: what is the problem with specifying epilogue? Am I
> wrong when I say that there can be only so many epilogue types
> (i.e. compiler will not generate infinite number of epilogues)?
> Therefore, only one index in a 'epilogue description table' would
> suffice...
> 
> (or am I over-simplifying?)

A compiler may reschedule instructions into the epilogue (this is what
makes writing prologue analyzers quite difficult).  I don't know
whether GCC does that or not.

I guess the real problem is that nobody has been motivated enough to
fix the problem.

Better ask questions on the GCC mailing lists though if you want an
answer that's not just a guess ;).

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Daniel Jacobowitz wrote:
> 
> When DWARF unwinding is not in use, there's the in_epilogue_p hook.
> Prologue analyzers could also detect epilogues using the same
> technique.  But this is slow and complicated.
> 

Interestingly, the only mention of in_epilogue_p I found is in ChangeLog-2004, but not a trace of it in the code? As if it was silently removed without mentioning it in the ChangeLog.

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Daniel Jacobowitz wrote:
> 
> When DWARF unwinding is not in use, there's the in_epilogue_p hook.
> Prologue analyzers could also detect epilogues using the same
> technique.  But this is slow and complicated.
> 

Interestingly, the only mention of in_epilogue_p I found is in ChangeLog-2004, but not a trace of it in the code? As if it was silently removed without mentioning it in the ChangeLog.

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Daniel Jacobowitz]

On Thu, May 22, 2008 at 10:29:41AM -0400, Aleksandar Ristovski wrote:
> Daniel Jacobowitz wrote:
>>
>> When DWARF unwinding is not in use, there's the in_epilogue_p hook.
>> Prologue analyzers could also detect epilogues using the same
>> technique.  But this is slow and complicated.
>>
>
> Interestingly, the only mention of in_epilogue_p I found is in
> ChangeLog-2004, but not a trace of it in the code? As if it was
> silently removed without mentioning it in the ChangeLog.

As if one changelog writer made the same typo I did; it's
gdbarch_in_function_epilogue_p.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Daniel Jacobowitz wrote:
> On Thu, May 22, 2008 at 10:29:41AM -0400, Aleksandar Ristovski wrote:
>> Daniel Jacobowitz wrote:
>>> When DWARF unwinding is not in use, there's the in_epilogue_p hook.
>>> Prologue analyzers could also detect epilogues using the same
>>> technique.  But this is slow and complicated.
>>>
>> Interestingly, the only mention of in_epilogue_p I found is in
>> ChangeLog-2004, but not a trace of it in the code? As if it was
>> silently removed without mentioning it in the ChangeLog.
> 
> As if one changelog writer made the same typo I did; it's
> gdbarch_in_function_epilogue_p.
> 

Ah.

So from what I can see, they all rely on symbol info (find_pc_partial_function).

Apparently I am missing something: if I were to write an "in_function_epilogue_p" for i386, why would I need the debug info? As a minimum, I want to check if we are at one of 'ret' instructions - no debug info needed.

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Daniel Jacobowitz wrote:
> On Thu, May 22, 2008 at 10:29:41AM -0400, Aleksandar Ristovski wrote:
>> Daniel Jacobowitz wrote:
>>> When DWARF unwinding is not in use, there's the in_epilogue_p hook.
>>> Prologue analyzers could also detect epilogues using the same
>>> technique.  But this is slow and complicated.
>>>
>> Interestingly, the only mention of in_epilogue_p I found is in
>> ChangeLog-2004, but not a trace of it in the code? As if it was
>> silently removed without mentioning it in the ChangeLog.
> 
> As if one changelog writer made the same typo I did; it's
> gdbarch_in_function_epilogue_p.
> 

Ah.

So from what I can see, they all rely on symbol info (find_pc_partial_function).

Apparently I am missing something: if I were to write an "in_function_epilogue_p" for i386, why would I need the debug info? As a minimum, I want to check if we are at one of 'ret' instructions - no debug info needed.

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Daniel Jacobowitz]

On Thu, May 22, 2008 at 10:51:12AM -0400, Aleksandar Ristovski wrote:
> So from what I can see, they all rely on symbol info (find_pc_partial_function).

Generally, so that you don't go marching off into the code of another
function.  If you only look at instructions forwards, and do not skip
any branches, you can probably do without it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Daniel Jacobowitz wrote:
> On Thu, May 22, 2008 at 10:51:12AM -0400, Aleksandar Ristovski wrote:
>> So from what I can see, they all rely on symbol info (find_pc_partial_function).
> 
> Generally, so that you don't go marching off into the code of another
> function.  If you only look at instructions forwards, and do not skip
> any branches, you can probably do without it.
> 

Am I again missing something big or is this really being called only from breakpoint.c?

I would imagine it should be called from other spots as well. I find handle_inferior_status quite complicated and do not fully understand it, but shouldn't it be called from there as well (when stepi-ing, we need to know if the frame can be created or not).

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Daniel Jacobowitz wrote:
> On Thu, May 22, 2008 at 10:51:12AM -0400, Aleksandar Ristovski wrote:
>> So from what I can see, they all rely on symbol info (find_pc_partial_function).
> 
> Generally, so that you don't go marching off into the code of another
> function.  If you only look at instructions forwards, and do not skip
> any branches, you can probably do without it.
> 

Am I again missing something big or is this really being called only from breakpoint.c?

I would imagine it should be called from other spots as well. I find handle_inferior_status quite complicated and do not fully understand it, but shouldn't it be called from there as well (when stepi-ing, we need to know if the frame can be created or not).

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Daniel Jacobowitz]

On Thu, May 22, 2008 at 12:00:31PM -0400, Aleksandar Ristovski wrote:
> Am I again missing something big or is this really being called only from breakpoint.c?
>
> I would imagine it should be called from other spots as well. I find
> handle_inferior_status quite complicated and do not fully understand
> it, but shouldn't it be called from there as well (when stepi-ing,
> we need to know if the frame can be created or not).

Why would we need to know whether we were in an epilogue?  If you want
to be able to backtrace, this hook isn't going to help you; your
unwinders have to cope.  I hacked the ARM one to do this once but I
don't see a general solution.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Michael Snyder]

On Thu, 2008-05-22 at 10:51 -0400, Aleksandar Ristovski wrote:
> 
> So from what I can see, they all rely on symbol info (find_pc_partial_function).
> 
> Apparently I am missing something: if I were to write an "in_function_epilogue_p" 
> for i386, why would I need the debug info? As a minimum, I want to
> check if we are at one of 'ret' instructions - no debug info needed.

We've never given the same level of attention to epilogues as we
have for prologues.  I'm not aware of anyone ever having added an
epilogue disassembler a la 'scan_prologue'.

If you wanted to do that -- typically for scan_prologue, we check
to see if we have enough symbol info first, and only resort to
instruction parsing if we have to.

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

Hello, 

The patch is an attempt to solve the issue from the subject. It introduces a check for in_function_epilogue_p and introduces architecture level prev_register_from_epilogue.

The patch is a draft (works but is not complete). 

Rationale: Dealing with epilogue in frame unwinders needs to be done only in sentinel frame (older frames will never be in epilogue). 

Comments appreciated.


Thanks,

Aleksandar Ristovski
QNX Software Systems


ChangeLog (for reference only):

	* gdbarch.c, gdbarch.h: Regenerated.
	* gdbarch.sh: Introduce gdbarch_prev_frame_from_epilogue and provide
	default_prev_frame_from_epilogue.
	* i386-tdep.c (i386_in_function_epilogue_p, 
	i386_prev_register_from_epilogue): New functions.
	(struct frame_unwind_cache): Struct definition.
	(i386_gdbarch_init): Register i386_prev_register_from_epilogue and
	i386_in_function_epilogue_p.
	* sentinel-frame.c (sentinel_frame_prev_register): Check if we are
	in epilogue and if so, call gdbarch provided prev_frame_from_epilogue.
	If this returns NULL do as before.

[-- Attachment #2: epilogue.diff --]
[-- Type: text/plain, Size: 11475 bytes --]

Index: gdb/gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.427
diff -u -p -r1.427 gdbarch.c
--- gdb/gdbarch.c	16 May 2008 00:27:23 -0000	1.427
+++ gdb/gdbarch.c	23 May 2008 15:13:39 -0000
@@ -52,7 +52,12 @@
 /* Static function declarations */
 
 static void alloc_gdbarch_data (struct gdbarch *);
-
+static struct value *
+default_prev_register_from_epilogue (struct gdbarch *gdbarch,
+				     struct frame_info *this_frame, 
+				     void **this_prologue_cache, 
+				     int regnum,
+				     CORE_ADDR pc);
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG
@@ -237,6 +242,7 @@ struct gdbarch
   gdbarch_target_signal_from_host_ftype *target_signal_from_host;
   gdbarch_target_signal_to_host_ftype *target_signal_to_host;
   gdbarch_record_special_symbol_ftype *record_special_symbol;
+  gdbarch_prev_register_from_epilogue_ftype *prev_register_from_epilogue;
 };
 
 
@@ -366,6 +372,7 @@ struct gdbarch startup_gdbarch =
   default_target_signal_from_host,  /* target_signal_from_host */
   default_target_signal_to_host,  /* target_signal_to_host */
   0,  /* record_special_symbol */
+  default_prev_register_from_epilogue,  /* prev_register_from_epilogue */
   /* startup_gdbarch() */
 };
 
@@ -449,6 +456,7 @@ gdbarch_alloc (const struct gdbarch_info
   gdbarch->displaced_step_location = NULL;
   gdbarch->target_signal_from_host = default_target_signal_from_host;
   gdbarch->target_signal_to_host = default_target_signal_to_host;
+  gdbarch->prev_register_from_epilogue = default_prev_register_from_epilogue;
   /* gdbarch_alloc() */
 
   return gdbarch;
@@ -617,6 +625,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of target_signal_from_host, invalid_p == 0 */
   /* Skip verify of target_signal_to_host, invalid_p == 0 */
   /* Skip verify of record_special_symbol, has predicate */
+  /* Skip verify of prev_register_from_epilogue, invalid_p == 0 */
   buf = ui_file_xstrdup (log, &dummy);
   make_cleanup (xfree, buf);
   if (strlen (buf) > 0)
@@ -889,6 +898,9 @@ gdbarch_dump (struct gdbarch *gdbarch, s
                       "gdbarch_dump: pointer_to_address = <0x%lx>\n",
                       (long) gdbarch->pointer_to_address);
   fprintf_unfiltered (file,
+                      "gdbarch_dump: prev_register_from_epilogue = <0x%lx>\n",
+                      (long) gdbarch->prev_register_from_epilogue);
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_print_float_info_p() = %d\n",
                       gdbarch_print_float_info_p (gdbarch));
   fprintf_unfiltered (file,
@@ -3190,6 +3202,23 @@ set_gdbarch_record_special_symbol (struc
   gdbarch->record_special_symbol = record_special_symbol;
 }
 
+struct value *
+gdbarch_prev_register_from_epilogue (struct gdbarch *gdbarch, struct frame_info *this_frame, void **this_prologue_cache, int regnum, CORE_ADDR pc)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->prev_register_from_epilogue != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_prev_register_from_epilogue called\n");
+  return gdbarch->prev_register_from_epilogue (gdbarch, this_frame, this_prologue_cache, regnum, pc);
+}
+
+void
+set_gdbarch_prev_register_from_epilogue (struct gdbarch *gdbarch,
+                                         gdbarch_prev_register_from_epilogue_ftype prev_register_from_epilogue)
+{
+  gdbarch->prev_register_from_epilogue = prev_register_from_epilogue;
+}
+
 
 /* Keep a registry of per-architecture data-pointers required by GDB
    modules. */
@@ -3600,6 +3629,16 @@ deprecated_current_gdbarch_select_hack (
   reinit_frame_cache ();
 }
 
+static struct value *
+default_prev_register_from_epilogue (struct gdbarch *gdbarch,
+				     struct frame_info *this_frame, 
+				     void **this_prologue_cache, 
+				     int regnum,
+				     CORE_ADDR pc)
+{
+  return NULL;
+}
+
 extern void _initialize_gdbarch (void);
 
 void
Index: gdb/gdbarch.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.h,v
retrieving revision 1.382
diff -u -p -r1.382 gdbarch.h
--- gdb/gdbarch.h	16 May 2008 00:27:23 -0000	1.382
+++ gdb/gdbarch.h	23 May 2008 15:13:39 -0000
@@ -798,6 +798,12 @@ typedef void (gdbarch_record_special_sym
 extern void gdbarch_record_special_symbol (struct gdbarch *gdbarch, struct objfile *objfile, asymbol *sym);
 extern void set_gdbarch_record_special_symbol (struct gdbarch *gdbarch, gdbarch_record_special_symbol_ftype *record_special_symbol);
 
+/* Create architecture dependent sentinel_frame. */
+
+typedef struct value * (gdbarch_prev_register_from_epilogue_ftype) (struct gdbarch *gdbarch, struct frame_info *this_frame, void **this_prologue_cache, int regnum, CORE_ADDR pc);
+extern struct value * gdbarch_prev_register_from_epilogue (struct gdbarch *gdbarch, struct frame_info *this_frame, void **this_prologue_cache, int regnum, CORE_ADDR pc);
+extern void set_gdbarch_prev_register_from_epilogue (struct gdbarch *gdbarch, gdbarch_prev_register_from_epilogue_ftype *prev_register_from_epilogue);
+
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
 
 
Index: gdb/gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.468
diff -u -p -r1.468 gdbarch.sh
--- gdb/gdbarch.sh	16 May 2008 00:27:23 -0000	1.468
+++ gdb/gdbarch.sh	23 May 2008 15:13:39 -0000
@@ -703,6 +703,10 @@ m:int:target_signal_to_host:enum target_
 
 # Record architecture-specific information from the symbol table.
 M:void:record_special_symbol:struct objfile *objfile, asymbol *sym:objfile, sym
+
+# Create architecture dependent sentinel_frame.
+m:struct value *:prev_register_from_epilogue:struct frame_info *this_frame, void **this_prologue_cache, int regnum, CORE_ADDR pc:this_frame, this_prologue_cache, regnum, pc::default_prev_register_from_epilogue::0
+
 EOF
 }
 
@@ -1133,7 +1137,12 @@ cat <<EOF
 /* Static function declarations */
 
 static void alloc_gdbarch_data (struct gdbarch *);
-
+static struct value *
+default_prev_register_from_epilogue (struct gdbarch *gdbarch,
+				     struct frame_info *this_frame, 
+				     void **this_prologue_cache, 
+				     int regnum,
+				     CORE_ADDR pc);
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG
@@ -2017,6 +2026,16 @@ deprecated_current_gdbarch_select_hack (
   reinit_frame_cache ();
 }
 
+static struct value *
+default_prev_register_from_epilogue (struct gdbarch *gdbarch,
+				     struct frame_info *this_frame, 
+				     void **this_prologue_cache, 
+				     int regnum,
+				     CORE_ADDR pc)
+{
+  return NULL;
+}
+
 extern void _initialize_gdbarch (void);
 
 void
Index: gdb/i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.258
diff -u -p -r1.258 i386-tdep.c
--- gdb/i386-tdep.c	16 May 2008 00:27:23 -0000	1.258
+++ gdb/i386-tdep.c	23 May 2008 15:13:40 -0000
@@ -2449,7 +2449,68 @@ i386_fetch_pointer_argument (struct fram
   CORE_ADDR sp = get_frame_register_unsigned  (frame, I386_ESP_REGNUM);
   return read_memory_unsigned_integer (sp + (4 * (argi + 1)), 4);
 }
+\f
+/* Determine if we are at one of RET instructions. If we are, then
+   chances are SP has been restored to the previous frame.  */
+
+static int
+i386_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  gdb_byte insn_buf[4];
+  struct frame_info *current_frame = get_current_frame ();
+
+  if (!safe_frame_unwind_memory (current_frame, pc, insn_buf, 1))
+    return 0;
+
+  if (insn_buf[0] == 0xC3)
+    return 1;
+
+  return 0;
+}
+
+struct frame_unwind_cache
+{
+  struct regcache *regcache;
+};
 
+static struct value *
+i386_prev_register_from_epilogue (struct gdbarch *gdbarch, 
+				  struct frame_info *this_frame, 
+				  void **this_cache, 
+				  int regnum, 
+				  CORE_ADDR pc)
+{
+  struct value *value;
+  struct frame_unwind_cache *cache = *this_cache;
+  printf_unfiltered ("Register from epilogue\n");
+  /* Return the actual value.  */
+  value = allocate_value (register_type (gdbarch, regnum));
+  VALUE_LVAL (value) = lval_register;
+  VALUE_REGNUM (value) = regnum;
+  VALUE_FRAME_ID (value) = get_frame_id (this_frame);
+
+  if (regnum == I386_EBP_REGNUM)
+    {
+      CORE_ADDR esp;
+      /* This register has special meaning; it preserves ESP of the caller
+	 (set in prologue) but in epilogue it has already been restored to
+	 the value before the call.  To keep the rest of the stuff working,
+	 pretend we are still before the epilogue, i.e. return the ESP.  */
+      regcache_cooked_read (cache->regcache, I386_ESP_REGNUM, 
+			    value_contents_raw (value));
+      esp = value_as_address (value);
+      esp -= 4;
+      store_unsigned_integer (value_contents_raw (value), 4, esp);
+      return value;
+    }
+
+  /* Use the regcache_cooked_read() method so that it, on the fly,
+     constructs either a raw or pseudo register from the raw
+     register cache.  */
+  regcache_cooked_read (cache->regcache, regnum, value_contents_raw (value));
+
+  return value;
+}
 \f
 static struct gdbarch *
 i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -2587,6 +2648,7 @@ i386_gdbarch_init (struct gdbarch_info i
   set_gdbarch_return_value (gdbarch, i386_return_value);
 
   set_gdbarch_skip_prologue (gdbarch, i386_skip_prologue);
+  set_gdbarch_in_function_epilogue_p (gdbarch, i386_in_function_epilogue_p);
 
   /* Stack grows downward.  */
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
@@ -2625,6 +2687,8 @@ i386_gdbarch_init (struct gdbarch_info i
 
   frame_unwind_append_unwinder (gdbarch, &i386_sigtramp_frame_unwind);
   frame_unwind_append_unwinder (gdbarch, &i386_frame_unwind);
+  set_gdbarch_prev_register_from_epilogue (gdbarch, 
+					   i386_prev_register_from_epilogue);
 
   /* If we have a register mapping, enable the generic core file
      support, unless it has already been enabled.  */
Index: gdb/sentinel-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/sentinel-frame.c,v
retrieving revision 1.18
diff -u -p -r1.18 sentinel-frame.c
--- gdb/sentinel-frame.c	30 Apr 2008 21:16:45 -0000	1.18
+++ gdb/sentinel-frame.c	23 May 2008 15:13:40 -0000
@@ -49,8 +49,25 @@ sentinel_frame_prev_register (struct fra
 {
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   struct frame_unwind_cache *cache = *this_prologue_cache;
-  struct value *value;
+  struct value *value = NULL;
 
+  if (regnum != gdbarch_pc_regnum (gdbarch))
+    {
+      struct value *value_pc;
+      CORE_ADDR pc;
+      value_pc = sentinel_frame_prev_register (this_frame, 
+					       this_prologue_cache, 
+					       gdbarch_pc_regnum (gdbarch));
+      pc = value_as_address (value_pc);
+      if (gdbarch_in_function_epilogue_p (gdbarch, pc))
+	value = gdbarch_prev_register_from_epilogue (gdbarch, 
+						     this_frame, 
+						     this_prologue_cache,
+						     regnum,
+						     pc);
+      if (value)
+	return value;
+    }
   /* Return the actual value.  */
   value = allocate_value (register_type (gdbarch, regnum));
   VALUE_LVAL (value) = lval_register;
@@ -84,3 +101,4 @@ const struct frame_unwind sentinel_frame
 };
 
 const struct frame_unwind *const sentinel_frame_unwind = &sentinel_frame_unwinder;
+

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

Hello, 

The patch is an attempt to solve the issue from the subject. It introduces a check for in_function_epilogue_p and introduces architecture level prev_register_from_epilogue.

The patch is a draft (works but is not complete). 

Rationale: Dealing with epilogue in frame unwinders needs to be done only in sentinel frame (older frames will never be in epilogue). 

Comments appreciated.


Thanks,

Aleksandar Ristovski
QNX Software Systems


ChangeLog (for reference only):

	* gdbarch.c, gdbarch.h: Regenerated.
	* gdbarch.sh: Introduce gdbarch_prev_frame_from_epilogue and provide
	default_prev_frame_from_epilogue.
	* i386-tdep.c (i386_in_function_epilogue_p, 
	i386_prev_register_from_epilogue): New functions.
	(struct frame_unwind_cache): Struct definition.
	(i386_gdbarch_init): Register i386_prev_register_from_epilogue and
	i386_in_function_epilogue_p.
	* sentinel-frame.c (sentinel_frame_prev_register): Check if we are
	in epilogue and if so, call gdbarch provided prev_frame_from_epilogue.
	If this returns NULL do as before.

[-- Attachment #2: epilogue.diff --]
[-- Type: text/plain, Size: 11475 bytes --]

Index: gdb/gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.427
diff -u -p -r1.427 gdbarch.c
--- gdb/gdbarch.c	16 May 2008 00:27:23 -0000	1.427
+++ gdb/gdbarch.c	23 May 2008 15:13:39 -0000
@@ -52,7 +52,12 @@
 /* Static function declarations */
 
 static void alloc_gdbarch_data (struct gdbarch *);
-
+static struct value *
+default_prev_register_from_epilogue (struct gdbarch *gdbarch,
+				     struct frame_info *this_frame, 
+				     void **this_prologue_cache, 
+				     int regnum,
+				     CORE_ADDR pc);
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG
@@ -237,6 +242,7 @@ struct gdbarch
   gdbarch_target_signal_from_host_ftype *target_signal_from_host;
   gdbarch_target_signal_to_host_ftype *target_signal_to_host;
   gdbarch_record_special_symbol_ftype *record_special_symbol;
+  gdbarch_prev_register_from_epilogue_ftype *prev_register_from_epilogue;
 };
 
 
@@ -366,6 +372,7 @@ struct gdbarch startup_gdbarch =
   default_target_signal_from_host,  /* target_signal_from_host */
   default_target_signal_to_host,  /* target_signal_to_host */
   0,  /* record_special_symbol */
+  default_prev_register_from_epilogue,  /* prev_register_from_epilogue */
   /* startup_gdbarch() */
 };
 
@@ -449,6 +456,7 @@ gdbarch_alloc (const struct gdbarch_info
   gdbarch->displaced_step_location = NULL;
   gdbarch->target_signal_from_host = default_target_signal_from_host;
   gdbarch->target_signal_to_host = default_target_signal_to_host;
+  gdbarch->prev_register_from_epilogue = default_prev_register_from_epilogue;
   /* gdbarch_alloc() */
 
   return gdbarch;
@@ -617,6 +625,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of target_signal_from_host, invalid_p == 0 */
   /* Skip verify of target_signal_to_host, invalid_p == 0 */
   /* Skip verify of record_special_symbol, has predicate */
+  /* Skip verify of prev_register_from_epilogue, invalid_p == 0 */
   buf = ui_file_xstrdup (log, &dummy);
   make_cleanup (xfree, buf);
   if (strlen (buf) > 0)
@@ -889,6 +898,9 @@ gdbarch_dump (struct gdbarch *gdbarch, s
                       "gdbarch_dump: pointer_to_address = <0x%lx>\n",
                       (long) gdbarch->pointer_to_address);
   fprintf_unfiltered (file,
+                      "gdbarch_dump: prev_register_from_epilogue = <0x%lx>\n",
+                      (long) gdbarch->prev_register_from_epilogue);
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_print_float_info_p() = %d\n",
                       gdbarch_print_float_info_p (gdbarch));
   fprintf_unfiltered (file,
@@ -3190,6 +3202,23 @@ set_gdbarch_record_special_symbol (struc
   gdbarch->record_special_symbol = record_special_symbol;
 }
 
+struct value *
+gdbarch_prev_register_from_epilogue (struct gdbarch *gdbarch, struct frame_info *this_frame, void **this_prologue_cache, int regnum, CORE_ADDR pc)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->prev_register_from_epilogue != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_prev_register_from_epilogue called\n");
+  return gdbarch->prev_register_from_epilogue (gdbarch, this_frame, this_prologue_cache, regnum, pc);
+}
+
+void
+set_gdbarch_prev_register_from_epilogue (struct gdbarch *gdbarch,
+                                         gdbarch_prev_register_from_epilogue_ftype prev_register_from_epilogue)
+{
+  gdbarch->prev_register_from_epilogue = prev_register_from_epilogue;
+}
+
 
 /* Keep a registry of per-architecture data-pointers required by GDB
    modules. */
@@ -3600,6 +3629,16 @@ deprecated_current_gdbarch_select_hack (
   reinit_frame_cache ();
 }
 
+static struct value *
+default_prev_register_from_epilogue (struct gdbarch *gdbarch,
+				     struct frame_info *this_frame, 
+				     void **this_prologue_cache, 
+				     int regnum,
+				     CORE_ADDR pc)
+{
+  return NULL;
+}
+
 extern void _initialize_gdbarch (void);
 
 void
Index: gdb/gdbarch.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.h,v
retrieving revision 1.382
diff -u -p -r1.382 gdbarch.h
--- gdb/gdbarch.h	16 May 2008 00:27:23 -0000	1.382
+++ gdb/gdbarch.h	23 May 2008 15:13:39 -0000
@@ -798,6 +798,12 @@ typedef void (gdbarch_record_special_sym
 extern void gdbarch_record_special_symbol (struct gdbarch *gdbarch, struct objfile *objfile, asymbol *sym);
 extern void set_gdbarch_record_special_symbol (struct gdbarch *gdbarch, gdbarch_record_special_symbol_ftype *record_special_symbol);
 
+/* Create architecture dependent sentinel_frame. */
+
+typedef struct value * (gdbarch_prev_register_from_epilogue_ftype) (struct gdbarch *gdbarch, struct frame_info *this_frame, void **this_prologue_cache, int regnum, CORE_ADDR pc);
+extern struct value * gdbarch_prev_register_from_epilogue (struct gdbarch *gdbarch, struct frame_info *this_frame, void **this_prologue_cache, int regnum, CORE_ADDR pc);
+extern void set_gdbarch_prev_register_from_epilogue (struct gdbarch *gdbarch, gdbarch_prev_register_from_epilogue_ftype *prev_register_from_epilogue);
+
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
 
 
Index: gdb/gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.468
diff -u -p -r1.468 gdbarch.sh
--- gdb/gdbarch.sh	16 May 2008 00:27:23 -0000	1.468
+++ gdb/gdbarch.sh	23 May 2008 15:13:39 -0000
@@ -703,6 +703,10 @@ m:int:target_signal_to_host:enum target_
 
 # Record architecture-specific information from the symbol table.
 M:void:record_special_symbol:struct objfile *objfile, asymbol *sym:objfile, sym
+
+# Create architecture dependent sentinel_frame.
+m:struct value *:prev_register_from_epilogue:struct frame_info *this_frame, void **this_prologue_cache, int regnum, CORE_ADDR pc:this_frame, this_prologue_cache, regnum, pc::default_prev_register_from_epilogue::0
+
 EOF
 }
 
@@ -1133,7 +1137,12 @@ cat <<EOF
 /* Static function declarations */
 
 static void alloc_gdbarch_data (struct gdbarch *);
-
+static struct value *
+default_prev_register_from_epilogue (struct gdbarch *gdbarch,
+				     struct frame_info *this_frame, 
+				     void **this_prologue_cache, 
+				     int regnum,
+				     CORE_ADDR pc);
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG
@@ -2017,6 +2026,16 @@ deprecated_current_gdbarch_select_hack (
   reinit_frame_cache ();
 }
 
+static struct value *
+default_prev_register_from_epilogue (struct gdbarch *gdbarch,
+				     struct frame_info *this_frame, 
+				     void **this_prologue_cache, 
+				     int regnum,
+				     CORE_ADDR pc)
+{
+  return NULL;
+}
+
 extern void _initialize_gdbarch (void);
 
 void
Index: gdb/i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.258
diff -u -p -r1.258 i386-tdep.c
--- gdb/i386-tdep.c	16 May 2008 00:27:23 -0000	1.258
+++ gdb/i386-tdep.c	23 May 2008 15:13:40 -0000
@@ -2449,7 +2449,68 @@ i386_fetch_pointer_argument (struct fram
   CORE_ADDR sp = get_frame_register_unsigned  (frame, I386_ESP_REGNUM);
   return read_memory_unsigned_integer (sp + (4 * (argi + 1)), 4);
 }
+\f
+/* Determine if we are at one of RET instructions. If we are, then
+   chances are SP has been restored to the previous frame.  */
+
+static int
+i386_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  gdb_byte insn_buf[4];
+  struct frame_info *current_frame = get_current_frame ();
+
+  if (!safe_frame_unwind_memory (current_frame, pc, insn_buf, 1))
+    return 0;
+
+  if (insn_buf[0] == 0xC3)
+    return 1;
+
+  return 0;
+}
+
+struct frame_unwind_cache
+{
+  struct regcache *regcache;
+};
 
+static struct value *
+i386_prev_register_from_epilogue (struct gdbarch *gdbarch, 
+				  struct frame_info *this_frame, 
+				  void **this_cache, 
+				  int regnum, 
+				  CORE_ADDR pc)
+{
+  struct value *value;
+  struct frame_unwind_cache *cache = *this_cache;
+  printf_unfiltered ("Register from epilogue\n");
+  /* Return the actual value.  */
+  value = allocate_value (register_type (gdbarch, regnum));
+  VALUE_LVAL (value) = lval_register;
+  VALUE_REGNUM (value) = regnum;
+  VALUE_FRAME_ID (value) = get_frame_id (this_frame);
+
+  if (regnum == I386_EBP_REGNUM)
+    {
+      CORE_ADDR esp;
+      /* This register has special meaning; it preserves ESP of the caller
+	 (set in prologue) but in epilogue it has already been restored to
+	 the value before the call.  To keep the rest of the stuff working,
+	 pretend we are still before the epilogue, i.e. return the ESP.  */
+      regcache_cooked_read (cache->regcache, I386_ESP_REGNUM, 
+			    value_contents_raw (value));
+      esp = value_as_address (value);
+      esp -= 4;
+      store_unsigned_integer (value_contents_raw (value), 4, esp);
+      return value;
+    }
+
+  /* Use the regcache_cooked_read() method so that it, on the fly,
+     constructs either a raw or pseudo register from the raw
+     register cache.  */
+  regcache_cooked_read (cache->regcache, regnum, value_contents_raw (value));
+
+  return value;
+}
 \f
 static struct gdbarch *
 i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -2587,6 +2648,7 @@ i386_gdbarch_init (struct gdbarch_info i
   set_gdbarch_return_value (gdbarch, i386_return_value);
 
   set_gdbarch_skip_prologue (gdbarch, i386_skip_prologue);
+  set_gdbarch_in_function_epilogue_p (gdbarch, i386_in_function_epilogue_p);
 
   /* Stack grows downward.  */
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
@@ -2625,6 +2687,8 @@ i386_gdbarch_init (struct gdbarch_info i
 
   frame_unwind_append_unwinder (gdbarch, &i386_sigtramp_frame_unwind);
   frame_unwind_append_unwinder (gdbarch, &i386_frame_unwind);
+  set_gdbarch_prev_register_from_epilogue (gdbarch, 
+					   i386_prev_register_from_epilogue);
 
   /* If we have a register mapping, enable the generic core file
      support, unless it has already been enabled.  */
Index: gdb/sentinel-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/sentinel-frame.c,v
retrieving revision 1.18
diff -u -p -r1.18 sentinel-frame.c
--- gdb/sentinel-frame.c	30 Apr 2008 21:16:45 -0000	1.18
+++ gdb/sentinel-frame.c	23 May 2008 15:13:40 -0000
@@ -49,8 +49,25 @@ sentinel_frame_prev_register (struct fra
 {
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   struct frame_unwind_cache *cache = *this_prologue_cache;
-  struct value *value;
+  struct value *value = NULL;
 
+  if (regnum != gdbarch_pc_regnum (gdbarch))
+    {
+      struct value *value_pc;
+      CORE_ADDR pc;
+      value_pc = sentinel_frame_prev_register (this_frame, 
+					       this_prologue_cache, 
+					       gdbarch_pc_regnum (gdbarch));
+      pc = value_as_address (value_pc);
+      if (gdbarch_in_function_epilogue_p (gdbarch, pc))
+	value = gdbarch_prev_register_from_epilogue (gdbarch, 
+						     this_frame, 
+						     this_prologue_cache,
+						     regnum,
+						     pc);
+      if (value)
+	return value;
+    }
   /* Return the actual value.  */
   value = allocate_value (register_type (gdbarch, regnum));
   VALUE_LVAL (value) = lval_register;
@@ -84,3 +101,4 @@ const struct frame_unwind sentinel_frame
 };
 
 const struct frame_unwind *const sentinel_frame_unwind = &sentinel_frame_unwinder;
+

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Thiago Jung Bauermann]

Hi,

I frequently receive your messages in duplicate, but with slighly
different To: and Cc: fields. I don't know what's happening, but is
there a way to stop that?

Sorry to have to bring this up, keep up the good work. :-)
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Thiago Jung Bauermann wrote:
> Hi,
> 
> I frequently receive your messages in duplicate, but with slighly
> different To: and Cc: fields. I don't know what's happening, but is
> there a way to stop that?
> 

I thought everybody liked my e-mails... :-|

but ok, now I'm trying with sending to the newsgroup only (gmane.comp.gdb.devel) without Cc-ing the ML (by default Thunderbird's "reply all" will put Cc to ML)...

Apologies to everyone for annoyance.

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Aleksandar Ristovski]

Any comments on this? 

Aleksandar Ristovski wrote:
> Hello,
> The patch is an attempt to solve the issue from the subject. It 
> introduces a check for in_function_epilogue_p and introduces 
> architecture level prev_register_from_epilogue.
> 
> The patch is a draft (works but is not complete).
> Rationale: Dealing with epilogue in frame unwinders needs to be done 
> only in sentinel frame (older frames will never be in epilogue).
> Comments appreciated.
> 
> 
> Thanks,
> 
> Aleksandar Ristovski
> QNX Software Systems
> 
> 
> ChangeLog (for reference only):
> 
>     * gdbarch.c, gdbarch.h: Regenerated.
>     * gdbarch.sh: Introduce gdbarch_prev_frame_from_epilogue and provide
>     default_prev_frame_from_epilogue.
>     * i386-tdep.c (i386_in_function_epilogue_p, 
>     i386_prev_register_from_epilogue): New functions.
>     (struct frame_unwind_cache): Struct definition.
>     (i386_gdbarch_init): Register i386_prev_register_from_epilogue and
>     i386_in_function_epilogue_p.
>     * sentinel-frame.c (sentinel_frame_prev_register): Check if we are
>     in epilogue and if so, call gdbarch provided prev_frame_from_epilogue.
>     If this returns NULL do as before.
> 

Re: Frame lost after 'leave' and before 'ret' instruction on i386?

[Mark Kettenis]

> From: Aleksandar Ristovski <aristovski@qnx.com>
> Date:  Wed, 04 Jun 2008 09:12:20 -0400
> 
> Any comments on this? 

Isn't it possible to do this by adding a "normal" unwinder?  It should
be properly stacked such that it is checked before the dwarf unwinder
of course.  You can check the frame level to short-circuit things the
same way as your diff does.

Adding a hook in the sentinel frame stuff feels wrong...

> Aleksandar Ristovski wrote:
> > Hello,
> > The patch is an attempt to solve the issue from the subject. It 
> > introduces a check for in_function_epilogue_p and introduces 
> > architecture level prev_register_from_epilogue.
> > 
> > The patch is a draft (works but is not complete).
> > Rationale: Dealing with epilogue in frame unwinders needs to be done 
> > only in sentinel frame (older frames will never be in epilogue).
> > Comments appreciated.
> > 
> > 
> > Thanks,
> > 
> > Aleksandar Ristovski
> > QNX Software Systems
> > 
> > 
> > ChangeLog (for reference only):
> > 
> >     * gdbarch.c, gdbarch.h: Regenerated.
> >     * gdbarch.sh: Introduce gdbarch_prev_frame_from_epilogue and provide
> >     default_prev_frame_from_epilogue.
> >     * i386-tdep.c (i386_in_function_epilogue_p, 
> >     i386_prev_register_from_epilogue): New functions.
> >     (struct frame_unwind_cache): Struct definition.
> >     (i386_gdbarch_init): Register i386_prev_register_from_epilogue and
> >     i386_in_function_epilogue_p.
> >     * sentinel-frame.c (sentinel_frame_prev_register): Check if we are
> >     in epilogue and if so, call gdbarch provided prev_frame_from_epilogue.
> >     If this returns NULL do as before.
> > 
> 
> 
>