This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc, frame] Always check for unsaved PC
- From: Daniel Jacobowitz <drow at false dot org>
- To: gdb-patches at sourceware dot org
- Date: Fri, 10 Nov 2006 15:15:48 -0500
- Subject: Re: [rfc, frame] Always check for unsaved PC
- References: <20060819161139.GC25238@nevyn.them.org> <200608201427.k7KERnD0001824@elgar.sibelius.xs4all.nl>
On Sun, Aug 20, 2006 at 04:27:49PM +0200, Mark Kettenis wrote:
> > Date: Sat, 19 Aug 2006 12:11:39 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> >
> > This patch, combined with my previous two frame patches for today,
> > fixes the infinite backtrace Olav Zarges reported on gdb@.
Actually, the second patch (the most controversial of the bunch) wasn't
necessary for that specific problem. So until someday I have time to
revisit that one, let's just consider this one on its own...
There wasn't any objection in the followup discussion to this patch.
Accordingly, I have committed it, along with a small fix (Michael
Snyder noticed that I had the wrong expectations for get_next_frame's
return value, it never returns the sentinel frame).
> > There's an associated FIXME; I discovered that three targets don't do
> > it this way. That doesn't break this patch, but it's inconsistent, and
> > does a bit of extra computation. So if there's general agreement that
> > this patch is a good idea, I'll go through and fix them, and try to
> > document more clearly that you aren't supposed to do it that way.
> > Then I can remove the FIXME.
>
> I'll have a go at alpha if you don't mind.
That refered to the FIXME above frame_register_unwind_location. Mark,
would you still like to do this, or shall I do all three affected
targets? It should be fairly mechanical.
--
Daniel Jacobowitz
CodeSourcery
2006-11-10 Daniel Jacobowitz <dan@codesourcery.com>
* frame.c (frame_register_unwind_location): New function.
(get_prev_frame_1): Check for UNWIND_NO_SAVED_PC.
(frame_stop_reason_string): Handle UNWIND_NO_SAVED_PC.
* frame.h (enum unwind_stop_reason): Add UNWIND_NO_SAVED_PC.
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.214
diff -u -p -r1.214 frame.c
--- frame.c 18 Oct 2006 19:52:05 -0000 1.214
+++ frame.c 10 Nov 2006 20:06:05 -0000
@@ -1023,6 +1023,36 @@ reinit_frame_cache (void)
}
}
+/* Find where a register is saved (in memory or another register).
+ The result of frame_register_unwind is just where it is saved
+ relative to this particular frame.
+
+ FIXME: alpha, m32c, and h8300 actually do the transitive operation
+ themselves. */
+
+static void
+frame_register_unwind_location (struct frame_info *this_frame, int regnum,
+ int *optimizedp, enum lval_type *lvalp,
+ CORE_ADDR *addrp, int *realnump)
+{
+ gdb_assert (this_frame == NULL || this_frame->level >= 0);
+
+ while (this_frame != NULL)
+ {
+ frame_register_unwind (this_frame, regnum, optimizedp, lvalp,
+ addrp, realnump, NULL);
+
+ if (*optimizedp)
+ break;
+
+ if (*lvalp != lval_register)
+ break;
+
+ regnum = *realnump;
+ this_frame = get_next_frame (this_frame);
+ }
+}
+
/* Return a "struct frame_info" corresponding to the frame that called
THIS_FRAME. Returns NULL if there is no such frame.
@@ -1111,6 +1141,42 @@ get_prev_frame_1 (struct frame_info *thi
return NULL;
}
+ /* Check that this and the next frame do not unwind the PC register
+ to the same memory location. If they do, then even though they
+ have different frame IDs, the new frame will be bogus; two
+ functions can't share a register save slot for the PC. This can
+ happen when the prologue analyzer finds a stack adjustment, but
+ no PC save. This check does assume that the "PC register" is
+ roughly a traditional PC, even if the gdbarch_unwind_pc method
+ frobs it. */
+ if (this_frame->level > 0
+ && get_frame_type (this_frame) == NORMAL_FRAME
+ && get_frame_type (this_frame->next) == NORMAL_FRAME)
+ {
+ int optimized, realnum;
+ enum lval_type lval, nlval;
+ CORE_ADDR addr, naddr;
+
+ frame_register_unwind_location (this_frame, PC_REGNUM, &optimized,
+ &lval, &addr, &realnum);
+ frame_register_unwind_location (get_next_frame (this_frame), PC_REGNUM,
+ &optimized, &nlval, &naddr, &realnum);
+
+ if (lval == lval_memory && lval == nlval && addr == naddr)
+ {
+ if (frame_debug)
+ {
+ fprintf_unfiltered (gdb_stdlog, "-> ");
+ fprint_frame (gdb_stdlog, NULL);
+ fprintf_unfiltered (gdb_stdlog, " // no saved PC }\n");
+ }
+
+ this_frame->stop_reason = UNWIND_NO_SAVED_PC;
+ this_frame->prev = NULL;
+ return NULL;
+ }
+ }
+
/* Allocate the new frame but do not wire it in to the frame chain.
Some (bad) code in INIT_FRAME_EXTRA_INFO tries to look along
frame->next to pull some fancy tricks (of course such code is, by
@@ -1611,6 +1677,9 @@ frame_stop_reason_string (enum unwind_st
case UNWIND_SAME_ID:
return _("previous frame identical to this frame (corrupt stack?)");
+ case UNWIND_NO_SAVED_PC:
+ return _("frame did not save the PC");
+
case UNWIND_NO_REASON:
case UNWIND_FIRST_ERROR:
default:
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.149
diff -u -p -r1.149 frame.h
--- frame.h 18 Oct 2006 19:52:05 -0000 1.149
+++ frame.h 10 Nov 2006 20:06:05 -0000
@@ -428,6 +428,10 @@ enum unwind_stop_reason
this is a sign of unwinder failure. It could also indicate
stack corruption. */
UNWIND_SAME_ID,
+
+ /* The frame unwinder didn't find any saved PC, but we needed
+ one to unwind further. */
+ UNWIND_NO_SAVED_PC,
};
/* Return the reason why we can't unwind past this frame. */