diff mbox

[v3,2/4] Remove previous frame if an error occurs when computing frame id during unwind.

Message ID 1398855344-25278-3-git-send-email-aburgess@broadcom.com
State Superseded
Headers show

Commit Message

Andrew Burgess April 30, 2014, 10:55 a.m. UTC
In get_prev_frame_if_no_cycle, if we throw an error during compute_frame_id
then we are left in a state where THIS_FRAME has a PREV_FRAME attached, but
PREV_FRAME has no frame id.  This is an unexpected state that causes
internal errors and assertions to fire.

This patch adds a cleanup that removes the previous frame created by
get_prev_frame_raw if we get an error.

OK to apply?

Thanks,
Andrew



gdb/ChangeLog:

	* frame.c (remove_prev_frame): New function.
	(get_prev_frame_if_no_cycle): Create / discard cleanup using
	remove_prev_frame.

---
 gdb/frame.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 14 deletions(-)

Comments

Pedro Alves May 16, 2014, 3:37 p.m. UTC | #1
On 04/30/2014 11:55 AM, Andrew Burgess wrote:
> In get_prev_frame_if_no_cycle, if we throw an error during compute_frame_id
> then we are left in a state where THIS_FRAME has a PREV_FRAME attached, but
> PREV_FRAME has no frame id.  This is an unexpected state that causes
> internal errors and assertions to fire.
> 
> This patch adds a cleanup that removes the previous frame created by
> get_prev_frame_raw if we get an error.
> 
> OK to apply?

This is OK.  Thank you.

(I guess it could go in immediately if you adjust
the test to expect the error and only try each command
once.)

I was looking at the rest of the series, but something came
up and I'm afraid I won't be able to reply today.  Sigh.  :-(
diff mbox

Patch

diff --git a/gdb/frame.c b/gdb/frame.c
index 97d54e9..5f05968 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1733,6 +1733,22 @@  frame_register_unwind_location (struct frame_info *this_frame, int regnum,
     }
 }
 
+/* Called during frame unwinding to remove a previous frame pointer from a
+   frame passed in ARG.  */
+
+static void
+remove_prev_frame (void *arg)
+{
+  struct frame_info *this_frame, *prev_frame;
+
+  this_frame = (struct frame_info *) arg;
+  prev_frame = this_frame->prev;
+  gdb_assert (prev_frame != NULL);
+
+  prev_frame->next = NULL;
+  this_frame->prev = NULL;
+}
+
 /* 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
@@ -1745,28 +1761,36 @@  static struct frame_info *
 get_prev_frame_if_no_cycle (struct frame_info *this_frame)
 {
   struct frame_info *prev_frame;
+  struct cleanup *prev_frame_cleanup;
 
   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;
+  /* The cleanup will remove the previous frame that get_prev_frame_raw
+     linked onto THIS_FRAME.  */
+  prev_frame_cleanup = make_cleanup (remove_prev_frame, this_frame);
 
-  /* Another frame with the same id was already in the stash.  We just
-     detected a cycle.  */
-  if (frame_debug)
+  compute_frame_id (prev_frame);
+  if (!frame_stash_add (prev_frame))
     {
-      fprintf_unfiltered (gdb_stdlog, "-> ");
-      fprint_frame (gdb_stdlog, NULL);
-      fprintf_unfiltered (gdb_stdlog, " // this frame has same ID }\n");
+      /* 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;
+      prev_frame = NULL;
     }
-  this_frame->stop_reason = UNWIND_SAME_ID;
-  /* Unlink.  */
-  prev_frame->next = NULL;
-  this_frame->prev = NULL;
-  return NULL;
+
+  discard_cleanups (prev_frame_cleanup);
+  return prev_frame;
 }
 
 /* Return a "struct frame_info" corresponding to the frame that called