From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11270 invoked by alias); 20 Nov 2013 18:27:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 11235 invoked by uid 89); 20 Nov 2013 18:27:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.1 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_HELO_PASS,SPF_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Nov 2013 18:26:47 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rAKIQdFL000393 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 20 Nov 2013 13:26:39 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id rAKIQc7i028093; Wed, 20 Nov 2013 13:26:38 -0500 Message-ID: <528CFEDE.1040505@redhat.com> Date: Wed, 20 Nov 2013 18:27:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Tom Tromey CC: gdb-patches@sourceware.org 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) References: <1384375873-32160-1-git-send-email-tromey@redhat.com> <1384375873-32160-2-git-send-email-tromey@redhat.com> <52850730.1060109@redhat.com> <87d2lxpo1l.fsf@fleche.redhat.com> <528B7F15.7040605@redhat.com> <87vbzomm78.fsf@fleche.redhat.com> <528B8FF6.7000406@redhat.com> <87siusl10r.fsf@fleche.redhat.com> <528BB700.4000802@redhat.com> <87fvqskumd.fsf@fleche.redhat.com> In-Reply-To: <87fvqskumd.fsf@fleche.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-11/txt/msg00604.txt.bz2 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 * 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