You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
247 lines
7.7 KiB
247 lines
7.7 KiB
6 years ago
|
commit 194cca41192efa65f710967e3149bbc813c12b22
|
||
|
Author: Pedro Alves <palves@redhat.com>
|
||
|
Date: Thu Nov 21 15:20:09 2013 +0000
|
||
|
|
||
|
Make use of the frame stash to detect wider stack cycles.
|
||
|
|
||
|
Given we already have the frame id stash, which holds the ids of all
|
||
|
frames in the chain, detecting corrupted stacks with wide stack cycles
|
||
|
with non-consecutive dup frame ids is just as cheap as just detecting
|
||
|
cycles in consecutive frames:
|
||
|
|
||
|
#0 frame_id1
|
||
|
#1 frame_id2
|
||
|
#2 frame_id3
|
||
|
#3 frame_id1
|
||
|
#4 frame_id2
|
||
|
#5 frame_id3
|
||
|
#6 frame_id1
|
||
|
... forever ...
|
||
|
|
||
|
We just need to check whether the stash already knows about a given
|
||
|
frame id instead of comparing the ids of the previous/this frames.
|
||
|
|
||
|
Tested on x86_64 Fedora 17.
|
||
|
|
||
|
gdb/
|
||
|
2013-11-22 Pedro Alves <palves@redhat.com>
|
||
|
Tom Tromey <tromey@redhat.com>
|
||
|
|
||
|
* frame.c (frame_stash_add): Now returns whether a frame with the
|
||
|
same ID was already known.
|
||
|
(compute_frame_id): New function, factored out from get_frame_id.
|
||
|
(get_frame_id): No longer lazilly compute the frame id here.
|
||
|
(get_prev_frame_if_no_cycle): New function. Detects wider stack
|
||
|
cycles.
|
||
|
(get_prev_frame_1): Use it instead of get_prev_frame_raw directly,
|
||
|
and checking for stack cycles here.
|
||
|
|
||
|
Index: gdb-7.6.1/gdb/frame.c
|
||
|
===================================================================
|
||
|
--- gdb-7.6.1.orig/gdb/frame.c
|
||
|
+++ gdb-7.6.1/gdb/frame.c
|
||
|
@@ -189,23 +189,31 @@ frame_stash_create (void)
|
||
|
NULL);
|
||
|
}
|
||
|
|
||
|
-/* Internal function to add a frame to the frame_stash hash table. Do
|
||
|
- not store frames below 0 as they may not have any addresses to
|
||
|
- calculate a hash. */
|
||
|
+/* Internal function to add a frame to the frame_stash hash table.
|
||
|
+ Returns false if a frame with the same ID was already stashed, true
|
||
|
+ otherwise. */
|
||
|
|
||
|
-static void
|
||
|
+static int
|
||
|
frame_stash_add (struct frame_info *frame)
|
||
|
{
|
||
|
- /* Do not stash frames below level 0. */
|
||
|
- if (frame->level >= 0)
|
||
|
- {
|
||
|
- struct frame_info **slot;
|
||
|
+ struct frame_info **slot;
|
||
|
|
||
|
- slot = (struct frame_info **) htab_find_slot (frame_stash,
|
||
|
- frame,
|
||
|
- INSERT);
|
||
|
- *slot = frame;
|
||
|
- }
|
||
|
+ /* Do not try to stash the sentinel frame. */
|
||
|
+ gdb_assert (frame->level >= 0);
|
||
|
+
|
||
|
+ slot = (struct frame_info **) htab_find_slot (frame_stash,
|
||
|
+ frame,
|
||
|
+ INSERT);
|
||
|
+
|
||
|
+ /* If we already have a frame in the stack with the same id, we
|
||
|
+ either have a stack cycle (corrupted stack?), or some bug
|
||
|
+ elsewhere in GDB. In any case, ignore the duplicate and return
|
||
|
+ an indication to the caller. */
|
||
|
+ if (*slot != NULL)
|
||
|
+ return 0;
|
||
|
+
|
||
|
+ *slot = frame;
|
||
|
+ return 1;
|
||
|
}
|
||
|
|
||
|
/* Internal function to search the frame stash for an entry with the
|
||
|
@@ -397,6 +405,34 @@ skip_artificial_frames (struct frame_inf
|
||
|
return frame;
|
||
|
}
|
||
|
|
||
|
+/* Compute the frame's uniq ID that can be used to, later, re-find the
|
||
|
+ frame. */
|
||
|
+
|
||
|
+static void
|
||
|
+compute_frame_id (struct frame_info *fi)
|
||
|
+{
|
||
|
+ gdb_assert (!fi->this_id.p);
|
||
|
+
|
||
|
+ if (frame_debug)
|
||
|
+ fprintf_unfiltered (gdb_stdlog, "{ compute_frame_id (fi=%d) ",
|
||
|
+ fi->level);
|
||
|
+ /* Find the unwinder. */
|
||
|
+ if (fi->unwind == NULL)
|
||
|
+ frame_unwind_find_by_frame (fi, &fi->prologue_cache);
|
||
|
+ /* Find THIS frame's ID. */
|
||
|
+ /* Default to outermost if no ID is found. */
|
||
|
+ fi->this_id.value = outer_frame_id;
|
||
|
+ fi->unwind->this_id (fi, &fi->prologue_cache, &fi->this_id.value);
|
||
|
+ gdb_assert (frame_id_p (fi->this_id.value));
|
||
|
+ fi->this_id.p = 1;
|
||
|
+ if (frame_debug)
|
||
|
+ {
|
||
|
+ fprintf_unfiltered (gdb_stdlog, "-> ");
|
||
|
+ fprint_frame_id (gdb_stdlog, fi->this_id.value);
|
||
|
+ fprintf_unfiltered (gdb_stdlog, " }\n");
|
||
|
+ }
|
||
|
+}
|
||
|
+
|
||
|
/* Return a frame uniq ID that can be used to, later, re-find the
|
||
|
frame. */
|
||
|
|
||
|
@@ -406,29 +442,7 @@ get_frame_id (struct frame_info *fi)
|
||
|
if (fi == NULL)
|
||
|
return null_frame_id;
|
||
|
|
||
|
- if (!fi->this_id.p)
|
||
|
- {
|
||
|
- if (frame_debug)
|
||
|
- fprintf_unfiltered (gdb_stdlog, "{ get_frame_id (fi=%d) ",
|
||
|
- fi->level);
|
||
|
- /* Find the unwinder. */
|
||
|
- if (fi->unwind == NULL)
|
||
|
- frame_unwind_find_by_frame (fi, &fi->prologue_cache);
|
||
|
- /* Find THIS frame's ID. */
|
||
|
- /* Default to outermost if no ID is found. */
|
||
|
- fi->this_id.value = outer_frame_id;
|
||
|
- fi->unwind->this_id (fi, &fi->prologue_cache, &fi->this_id.value);
|
||
|
- gdb_assert (frame_id_p (fi->this_id.value));
|
||
|
- fi->this_id.p = 1;
|
||
|
- if (frame_debug)
|
||
|
- {
|
||
|
- fprintf_unfiltered (gdb_stdlog, "-> ");
|
||
|
- fprint_frame_id (gdb_stdlog, fi->this_id.value);
|
||
|
- fprintf_unfiltered (gdb_stdlog, " }\n");
|
||
|
- }
|
||
|
- frame_stash_add (fi);
|
||
|
- }
|
||
|
-
|
||
|
+ gdb_assert (fi->this_id.p);
|
||
|
return fi->this_id.value;
|
||
|
}
|
||
|
|
||
|
@@ -1678,6 +1692,42 @@ frame_register_unwind_location (struct f
|
||
|
}
|
||
|
}
|
||
|
|
||
|
+/* Get the previous raw frame, and check that it is not identical to
|
||
|
+ same other frame frame already in the chain. If it is, there is
|
||
|
+ most likely a stack cycle, so we discard it, and mark THIS_FRAME as
|
||
|
+ outermost, with UNWIND_SAME_ID stop reason. Unlike the other
|
||
|
+ validity tests, that compare THIS_FRAME and the next frame, we do
|
||
|
+ this right after creating the previous frame, to avoid ever ending
|
||
|
+ up with two frames with the same id in the frame chain. */
|
||
|
+
|
||
|
+static struct frame_info *
|
||
|
+get_prev_frame_if_no_cycle (struct frame_info *this_frame)
|
||
|
+{
|
||
|
+ struct frame_info *prev_frame;
|
||
|
+
|
||
|
+ prev_frame = get_prev_frame_raw (this_frame);
|
||
|
+ if (prev_frame == NULL)
|
||
|
+ return NULL;
|
||
|
+
|
||
|
+ compute_frame_id (prev_frame);
|
||
|
+ if (frame_stash_add (prev_frame))
|
||
|
+ return prev_frame;
|
||
|
+
|
||
|
+ /* Another frame with the same id was already in the stash. We just
|
||
|
+ detected a cycle. */
|
||
|
+ if (frame_debug)
|
||
|
+ {
|
||
|
+ fprintf_unfiltered (gdb_stdlog, "-> ");
|
||
|
+ fprint_frame (gdb_stdlog, NULL);
|
||
|
+ fprintf_unfiltered (gdb_stdlog, " // this frame has same ID }\n");
|
||
|
+ }
|
||
|
+ this_frame->stop_reason = UNWIND_SAME_ID;
|
||
|
+ /* Unlink. */
|
||
|
+ prev_frame->next = NULL;
|
||
|
+ this_frame->prev = NULL;
|
||
|
+ return NULL;
|
||
|
+}
|
||
|
+
|
||
|
/* Return a "struct frame_info" corresponding to the frame that called
|
||
|
THIS_FRAME. Returns NULL if there is no such frame.
|
||
|
|
||
|
@@ -1689,7 +1739,6 @@ get_prev_frame_1 (struct frame_info *thi
|
||
|
{
|
||
|
struct frame_id this_id;
|
||
|
struct gdbarch *gdbarch;
|
||
|
- struct frame_info *prev_frame;
|
||
|
|
||
|
gdb_assert (this_frame != NULL);
|
||
|
gdbarch = get_frame_arch (this_frame);
|
||
|
@@ -1732,7 +1781,7 @@ get_prev_frame_1 (struct frame_info *thi
|
||
|
until we have unwound all the way down to the previous non-inline
|
||
|
frame. */
|
||
|
if (get_frame_type (this_frame) == INLINE_FRAME)
|
||
|
- return get_prev_frame_raw (this_frame);
|
||
|
+ return get_prev_frame_if_no_cycle (this_frame);
|
||
|
|
||
|
/* Check that this frame is unwindable. If it isn't, don't try to
|
||
|
unwind to the prev frame. */
|
||
|
@@ -1838,31 +1887,7 @@ get_prev_frame_1 (struct frame_info *thi
|
||
|
}
|
||
|
}
|
||
|
|
||
|
- prev_frame = get_prev_frame_raw (this_frame);
|
||
|
-
|
||
|
- /* Check that this and the prev frame are not identical. If they
|
||
|
- are, there is most likely a stack cycle. Unlike the tests above,
|
||
|
- we do this right after creating the prev frame, to avoid ever
|
||
|
- ending up with two frames with the same id in the frame
|
||
|
- chain. */
|
||
|
- if (prev_frame != NULL
|
||
|
- && frame_id_eq (get_frame_id (prev_frame),
|
||
|
- get_frame_id (this_frame)))
|
||
|
- {
|
||
|
- if (frame_debug)
|
||
|
- {
|
||
|
- fprintf_unfiltered (gdb_stdlog, "-> ");
|
||
|
- fprint_frame (gdb_stdlog, NULL);
|
||
|
- fprintf_unfiltered (gdb_stdlog, " // this frame has same ID }\n");
|
||
|
- }
|
||
|
- this_frame->stop_reason = UNWIND_SAME_ID;
|
||
|
- /* Unlink. */
|
||
|
- prev_frame->next = NULL;
|
||
|
- this_frame->prev = NULL;
|
||
|
- return NULL;
|
||
|
- }
|
||
|
-
|
||
|
- return prev_frame;
|
||
|
+ return get_prev_frame_if_no_cycle (this_frame);
|
||
|
}
|
||
|
|
||
|
/* Construct a new "struct frame_info" and link it previous to
|