* Re: [patch/rfc] strengthn get_prev_frame() checks
2003-02-25 3:55 [patch/rfc] strengthn get_prev_frame() checks Andrew Cagney
@ 2003-02-25 3:59 ` Andrew Cagney
2003-02-25 23:53 ` Andrew Cagney
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cagney @ 2003-02-25 3:59 UTC (permalink / raw)
Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 248 bytes --]
> Hello,
>
> This improves the checks in get_prev_frame() that look for stuff like the top-of-stack or a corrupt stack.
>
> d10v (which uses this) showed no regressions, neither did i386.
>
> I'll commit `tomorrow'.
>
> Andrew
With patch....
[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 2983 bytes --]
2003-02-24 Andrew Cagney <cagney@redhat.com>
* frame.c (get_prev_frame): Add comment on check for
inside_entry_func. Only check for inside_entry_file when not a
dummy and not a sentinel. Check that the new frame is not inner
to the old frame.
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.67
diff -u -r1.67 frame.c
--- frame.c 20 Feb 2003 16:35:51 -0000 1.67
+++ frame.c 25 Feb 2003 03:51:35 -0000
@@ -1230,7 +1230,6 @@
return next_frame->prev;
next_frame->prev_p = 1;
- /* If we're inside the entry file, it isn't valid. */
/* NOTE: drow/2002-12-25: should there be a way to disable this
check? It assumes a single small entry file, and the way some
debug readers (e.g. dbxread) figure out which object is the
@@ -1238,8 +1237,26 @@
/* NOTE: cagney/2003-01-10: If there is a way of disabling this test
then it should probably be moved to before the ->prev_p test,
above. */
- if (inside_entry_file (get_frame_pc (next_frame)))
- return NULL;
+ /* If we're inside the entry file, it isn't valid. Don't apply this
+ test to a dummy frame - dummy frame PC's typically land in the
+ entry file. Don't apply this test to the sentinel frame.
+ Sentinel frames should always be allowed to unwind. */
+ if (next_frame->type != DUMMY_FRAME && next_frame->level >= 0
+ && inside_entry_file (get_frame_pc (next_frame)))
+ return NULL;
+
+#if 0
+ /* NOTE: cagney/2003-02-25: Don't enable until someone has found
+ evidence that this is needed. */
+ /* If we're already inside the entry function for the main objfile,
+ then it isn't valid. Don't apply this test to a dummy frame -
+ dummy frame PC's typically land in the entry func. Don't apply
+ this test to the sentinel frame. Sentinel frames should always
+ be allowed to unwind. */
+ if (next_frame->type != DUMMY_FRAME && next_frame->level >= 0
+ && inside_entry_func (get_frame_pc (fi)))
+ return 0;
+#endif
/* If any of the old frame initialization methods are around, use
the legacy get_prev_frame method. Just don't try to unwind a
@@ -1301,6 +1318,16 @@
struct frame_id id = frame_id_unwind (next_frame);
if (!frame_id_p (id))
return NULL;
+ /* Check that the new frame isn't inner to (younger, below, next)
+ the old frame - we've not gone backwards. Ignore the sentinel
+ frame where weird things happen. */
+ if (next_frame->level >= 0
+ && frame_id_inner (id, get_frame_id (next_frame)))
+ error ("Unwound frame inner to selected frame (corrupt stack?)");
+ /* Note that, due to frameless functions, the stronger test of the
+ new frame being outer to the old frame can't be used -
+ frameless functions differ by only their PC value. Ignore the
+ sentinel frame where weird things happen. */
prev_frame->frame = id.base;
}
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch/rfc] strengthn get_prev_frame() checks
2003-02-25 3:55 [patch/rfc] strengthn get_prev_frame() checks Andrew Cagney
2003-02-25 3:59 ` Andrew Cagney
@ 2003-02-25 23:53 ` Andrew Cagney
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cagney @ 2003-02-25 23:53 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 338 bytes --]
> Hello,
>
> This improves the checks in get_prev_frame() that look for stuff like the top-of-stack or a corrupt stack.
>
> d10v (which uses this) showed no regressions, neither did i386.
>
> I'll commit `tomorrow'.
>
> Andrew
The attached is what I actually checked in. It adds some debug messages
to the previous patch.
Andrew
[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 3916 bytes --]
2003-02-25 Andrew Cagney <cagney@redhat.com>
* frame.c (get_prev_frame): Add comment on check for
inside_entry_func. Only check for inside_entry_file when not a
dummy and not a sentinel. Check that the new frame is not inner
to the old frame.
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.68
diff -u -r1.68 frame.c
--- frame.c 25 Feb 2003 23:12:22 -0000 1.68
+++ frame.c 25 Feb 2003 23:50:52 -0000
@@ -1239,7 +1239,10 @@
return next_frame->prev;
next_frame->prev_p = 1;
- /* If we're inside the entry file, it isn't valid. */
+ /* If we're inside the entry file, it isn't valid. Don't apply this
+ test to a dummy frame - dummy frame PC's typically land in the
+ entry file. Don't apply this test to the sentinel frame.
+ Sentinel frames should always be allowed to unwind. */
/* NOTE: drow/2002-12-25: should there be a way to disable this
check? It assumes a single small entry file, and the way some
debug readers (e.g. dbxread) figure out which object is the
@@ -1247,7 +1250,8 @@
/* NOTE: cagney/2003-01-10: If there is a way of disabling this test
then it should probably be moved to before the ->prev_p test,
above. */
- if (inside_entry_file (get_frame_pc (next_frame)))
+ if (next_frame->type != DUMMY_FRAME && next_frame->level >= 0
+ && inside_entry_file (get_frame_pc (next_frame)))
{
if (frame_debug)
fprintf_unfiltered (gdb_stdlog,
@@ -1255,6 +1259,23 @@
return NULL;
}
+ /* If we're already inside the entry function for the main objfile,
+ then it isn't valid. Don't apply this test to a dummy frame -
+ dummy frame PC's typically land in the entry func. Don't apply
+ this test to the sentinel frame. Sentinel frames should always
+ be allowed to unwind. */
+ /* NOTE: cagney/2003-02-25: Don't enable until someone has found
+ hard evidence that this is needed. */
+ if (0
+ && next_frame->type != DUMMY_FRAME && next_frame->level >= 0
+ && inside_entry_func (get_frame_pc (next_frame)))
+ {
+ if (frame_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "Outermost frame - inside entry func\n");
+ return NULL;
+ }
+
/* If any of the old frame initialization methods are around, use
the legacy get_prev_frame method. Just don't try to unwind a
sentinel frame using that method - it doesn't work. All sentinal
@@ -1324,6 +1345,9 @@
/* FIXME: cagney/2002-12-18: Instead of this hack, should just
save the frame ID directly. */
struct frame_id id = frame_id_unwind (next_frame);
+ /* Check that the unwound ID is valid. As of 2003-02-24 the
+ x86-64 was returning an invalid frame ID when trying to do an
+ unwind a sentinel frame that belonged to a frame dummy. */
if (!frame_id_p (id))
{
if (frame_debug)
@@ -1331,6 +1355,20 @@
"Outermost frame - unwound frame ID invalid\n");
return NULL;
}
+ /* Check that the new frame isn't inner to (younger, below, next)
+ the old frame. If that happens the frame unwind is going
+ backwards. */
+ /* FIXME: cagney/2003-02-25: Ignore the sentinel frame since that
+ doesn't have a valid frame ID. Should instead set the sentinel
+ frame's frame ID to a `sentinel'. Leave it until after the
+ switch to storing the frame ID, instead of the frame base, in
+ the frame object. */
+ if (next_frame->level >= 0
+ && frame_id_inner (id, get_frame_id (next_frame)))
+ error ("Unwound frame inner-to selected frame (corrupt stack?)");
+ /* Note that, due to frameless functions, the stronger test of the
+ new frame being outer to the old frame can't be used -
+ frameless functions differ by only their PC value. */
prev_frame->frame = id.base;
}
^ permalink raw reply [flat|nested] 3+ messages in thread