diff mbox

[RFC,2/4] Remove previous frame if we error during compute_frame_id

Message ID 533EC76F.4040204@broadcom.com
State Superseded
Headers

Commit Message

Andrew Burgess April 4, 2014, 2:53 p.m. UTC 
  Gah! The patch was corrupted in the last post. Try again:

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.

 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

Comments

Pedro Alves April 15, 2014, 7:02 p.m. UTC | #1 
On 04/04/2014 03:53 PM, Andrew Burgess wrote:
> Gah! The patch was corrupted in the last post. Try again:

Seems to still be corrupt.

$ git am /tmp/mbox
Applying: New tests for backtracing with a corrupted stack.
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:27: trailing whitespace.

/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:48: trailing whitespace.
# SUCC: EXIT [100.0%]
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:72: trailing whitespace.
# SUCC: EXIT [100.0%]
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:96: trailing whitespace.
# SUCC: EXIT [100.0%]
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:120: trailing whitespace.
# SUCC: EXIT [100.0%]
warning: squelched 10 whitespace errors
warning: 15 lines add whitespace errors.
Applying: Remove previous frame if we error during compute_frame_id
fatal: corrupt patch at line 27
Patch failed at 0002 Remove previous frame if we error during compute_frame_id
The copy of the patch that failed is found in:
   /home/pedro/gdb/mygit/src/.git/rebase-apply/patch
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

Are you using git send-email ?
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 *