This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH] Don't let two frames with the same id end up in the frame chain. (Re: [PATCH 1/2] avoid infinite loop with bad debuginfo)
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 20 Nov 2013 18:26:38 +0000
- Subject: [PATCH] Don't let two frames with the same id end up in the frame chain. (Re: [PATCH 1/2] avoid infinite loop with bad debuginfo)
- Authentication-results: sourceware.org; auth=none
- References: <1384375873-32160-1-git-send-email-tromey at redhat dot com> <1384375873-32160-2-git-send-email-tromey at redhat dot com> <52850730 dot 1060109 at redhat dot com> <87d2lxpo1l dot fsf at fleche dot redhat dot com> <528B7F15 dot 7040605 at redhat dot com> <87vbzomm78 dot fsf at fleche dot redhat dot com> <528B8FF6 dot 7000406 at redhat dot com> <87siusl10r dot fsf at fleche dot redhat dot com> <528BB700 dot 4000802 at redhat dot com> <87fvqskumd dot fsf at fleche dot redhat dot com>
On 11/19/2013 08:24 PM, Tom Tromey wrote:
> Pedro> I don't think so, because get_prev_frame_1 would not link in
> Pedro> the dup frame. The loop in question would never see it.
>
> Pedro> Hmm, I think one of us is missing something.
>
> Haha, yeah, that usually means me :-)
Haha, I wish. :-)
>
> No worries. I think I understand this bit now.
>
> Pedro> So the bad loop can only ever happen (outside the unwinder code)
> Pedro> if we ever let outselves get in the dup frame_id situation:
>
>>> #4 0x0000007fb7f0956c in clone () from /lib64/libc.so.6
>>> #5 0x0000007fb7f0956c in clone () from /lib64/libc.so.6
>>> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>
> Pedro> At least, I'm not seeing any other way.
>
> Yes, I see now.
OK, here's the same in patch form.
>
> Really not looking forward to writing the test.
Yeah, me neither. :-P
---------
Subject: Don't let two frames with the same id end up in the frame chain.
The UNWIND_SAME_ID check is done between THIS_FRAME and the next
frame. But at this point, it's already too late -- we ended up with
two frames with the same ID in the frame chain. Each frame having its
own ID is an invariant assumed throughout GDB. So this patch applies
the UNWIND_SAME_ID detection earlier, right after the previous frame
is unwond, discarding the dup frame if a cycle is detected.
gdb/
2013-11-20 Pedro Alves <palves@redhat.com>
* frame.c (get_prev_frame_1): Do the UNWIND_SAME_ID check between
this frame and the new previous frame, not between this frame and
the next frame.
---
gdb/frame.c | 43 ++++++++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/gdb/frame.c b/gdb/frame.c
index 63f20d5..535a5a6 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1666,6 +1666,7 @@ get_prev_frame_1 (struct frame_info *this_frame)
{
struct frame_id this_id;
struct gdbarch *gdbarch;
+ struct frame_info *prev_frame;
gdb_assert (this_frame != NULL);
gdbarch = get_frame_arch (this_frame);
@@ -1767,22 +1768,6 @@ get_prev_frame_1 (struct frame_info *this_frame)
}
}
- /* Check that this and the next frame are not identical. If they
- are, there is most likely a stack cycle. As with the inner-than
- test above, avoid comparing the inner-most and sentinel frames. */
- if (this_frame->level > 0
- && frame_id_eq (this_id, get_frame_id (this_frame->next)))
- {
- 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;
- 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
@@ -1830,7 +1815,31 @@ get_prev_frame_1 (struct frame_info *this_frame)
}
}
- return get_prev_frame_raw (this_frame);
+ 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;
}
/* Construct a new "struct frame_info" and link it previous to