From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24992 invoked by alias); 10 Nov 2006 20:16:01 -0000 Received: (qmail 24856 invoked by uid 22791); 10 Nov 2006 20:15:59 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Fri, 10 Nov 2006 20:15:50 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1Gicmy-0001DB-Gg for gdb-patches@sourceware.org; Fri, 10 Nov 2006 15:15:48 -0500 Date: Fri, 10 Nov 2006 20:16:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: Re: [rfc, frame] Always check for unsaved PC Message-ID: <20061110201548.GA1115@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org References: <20060819161139.GC25238@nevyn.them.org> <200608201427.k7KERnD0001824@elgar.sibelius.xs4all.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200608201427.k7KERnD0001824@elgar.sibelius.xs4all.nl> User-Agent: Mutt/1.5.13 (2006-08-11) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-11/txt/msg00069.txt.bz2 On Sun, Aug 20, 2006 at 04:27:49PM +0200, Mark Kettenis wrote: > > Date: Sat, 19 Aug 2006 12:11:39 -0400 > > From: Daniel Jacobowitz > > > > This patch, combined with my previous two frame patches for today, > > fixes the infinite backtrace Olav Zarges reported on gdb@. Actually, the second patch (the most controversial of the bunch) wasn't necessary for that specific problem. So until someday I have time to revisit that one, let's just consider this one on its own... There wasn't any objection in the followup discussion to this patch. Accordingly, I have committed it, along with a small fix (Michael Snyder noticed that I had the wrong expectations for get_next_frame's return value, it never returns the sentinel frame). > > There's an associated FIXME; I discovered that three targets don't do > > it this way. That doesn't break this patch, but it's inconsistent, and > > does a bit of extra computation. So if there's general agreement that > > this patch is a good idea, I'll go through and fix them, and try to > > document more clearly that you aren't supposed to do it that way. > > Then I can remove the FIXME. > > I'll have a go at alpha if you don't mind. That refered to the FIXME above frame_register_unwind_location. Mark, would you still like to do this, or shall I do all three affected targets? It should be fairly mechanical. -- Daniel Jacobowitz CodeSourcery 2006-11-10 Daniel Jacobowitz * frame.c (frame_register_unwind_location): New function. (get_prev_frame_1): Check for UNWIND_NO_SAVED_PC. (frame_stop_reason_string): Handle UNWIND_NO_SAVED_PC. * frame.h (enum unwind_stop_reason): Add UNWIND_NO_SAVED_PC. Index: frame.c =================================================================== RCS file: /cvs/src/src/gdb/frame.c,v retrieving revision 1.214 diff -u -p -r1.214 frame.c --- frame.c 18 Oct 2006 19:52:05 -0000 1.214 +++ frame.c 10 Nov 2006 20:06:05 -0000 @@ -1023,6 +1023,36 @@ reinit_frame_cache (void) } } +/* Find where a register is saved (in memory or another register). + The result of frame_register_unwind is just where it is saved + relative to this particular frame. + + FIXME: alpha, m32c, and h8300 actually do the transitive operation + themselves. */ + +static void +frame_register_unwind_location (struct frame_info *this_frame, int regnum, + int *optimizedp, enum lval_type *lvalp, + CORE_ADDR *addrp, int *realnump) +{ + gdb_assert (this_frame == NULL || this_frame->level >= 0); + + while (this_frame != NULL) + { + frame_register_unwind (this_frame, regnum, optimizedp, lvalp, + addrp, realnump, NULL); + + if (*optimizedp) + break; + + if (*lvalp != lval_register) + break; + + regnum = *realnump; + this_frame = get_next_frame (this_frame); + } +} + /* Return a "struct frame_info" corresponding to the frame that called THIS_FRAME. Returns NULL if there is no such frame. @@ -1111,6 +1141,42 @@ get_prev_frame_1 (struct frame_info *thi 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 + functions can't share a register save slot for the PC. This can + happen when the prologue analyzer finds a stack adjustment, but + no PC save. This check does assume that the "PC register" is + roughly a traditional PC, even if the gdbarch_unwind_pc method + frobs it. */ + if (this_frame->level > 0 + && get_frame_type (this_frame) == NORMAL_FRAME + && get_frame_type (this_frame->next) == NORMAL_FRAME) + { + int optimized, realnum; + enum lval_type lval, nlval; + CORE_ADDR addr, naddr; + + frame_register_unwind_location (this_frame, PC_REGNUM, &optimized, + &lval, &addr, &realnum); + frame_register_unwind_location (get_next_frame (this_frame), PC_REGNUM, + &optimized, &nlval, &naddr, &realnum); + + if (lval == lval_memory && lval == nlval && addr == naddr) + { + if (frame_debug) + { + fprintf_unfiltered (gdb_stdlog, "-> "); + fprint_frame (gdb_stdlog, NULL); + fprintf_unfiltered (gdb_stdlog, " // no saved PC }\n"); + } + + this_frame->stop_reason = UNWIND_NO_SAVED_PC; + this_frame->prev = NULL; + return NULL; + } + } + /* Allocate the new frame but do not wire it in to the frame chain. Some (bad) code in INIT_FRAME_EXTRA_INFO tries to look along frame->next to pull some fancy tricks (of course such code is, by @@ -1611,6 +1677,9 @@ frame_stop_reason_string (enum unwind_st case UNWIND_SAME_ID: return _("previous frame identical to this frame (corrupt stack?)"); + case UNWIND_NO_SAVED_PC: + return _("frame did not save the PC"); + case UNWIND_NO_REASON: case UNWIND_FIRST_ERROR: default: Index: frame.h =================================================================== RCS file: /cvs/src/src/gdb/frame.h,v retrieving revision 1.149 diff -u -p -r1.149 frame.h --- frame.h 18 Oct 2006 19:52:05 -0000 1.149 +++ frame.h 10 Nov 2006 20:06:05 -0000 @@ -428,6 +428,10 @@ enum unwind_stop_reason this is a sign of unwinder failure. It could also indicate stack corruption. */ UNWIND_SAME_ID, + + /* The frame unwinder didn't find any saved PC, but we needed + one to unwind further. */ + UNWIND_NO_SAVED_PC, }; /* Return the reason why we can't unwind past this frame. */