From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4129 invoked by alias); 20 Aug 2006 17:05:14 -0000 Received: (qmail 4120 invoked by uid 22791); 20 Aug 2006 17:05:13 -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; Sun, 20 Aug 2006 17:05:10 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1GEqjU-0005my-25; Sun, 20 Aug 2006 13:05:08 -0400 Date: Mon, 21 Aug 2006 15:58:00 -0000 From: Daniel Jacobowitz To: Mark Kettenis Cc: gdb-patches@sourceware.org Subject: Re: [rfc, frame] Always check for unsaved PC Message-ID: <20060820170508.GD20987@nevyn.them.org> Mail-Followup-To: Mark Kettenis , 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.11+cvs20060403 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-08/txt/msg00144.txt.bz2 On Sun, Aug 20, 2006 at 04:27:49PM +0200, Mark Kettenis wrote: > > If there were no stack frame, this would trigger > > the duplicate frame ID check, but there is; the function remains the > > same but the CFA moves continually down, and the frame repeats > > endlessly. > > > > Here's a simple way to detect this. Suppose we have two normal frames > > in a row. Unwind PC_REGNUM from both of them. Two functions having > > the same return address isn't a problem. But two functions sharing a > > stack slot for the return address is a big problem; that never happens > > normally, since the first one would pop it. So this indicates that the > > older frame didn't actually save it. > > This doesn't fly for architectures where the return address is > computed though, like on sparc. Right. I don't think we need to make this generic check work everywhere; what I'd like to do instead would be to allow the prologue unwinder to return its own "this is why I think unwinding is done" reason. The check doesn't really apply to SPARC in the same form. However, a frameless normal frame where the next frame is also a normal frame has the same problem, and could use the same unwind stop reason. I see that gdb doesn't use dwarf2 unwind tables on sparc. Does GCC generate them? How does GCC deal with the extra +8? It looks like it doesn't; it uses %o7 to handle unwinding and just looks at the location of the call site. > > There's one wart in this patch. frame_register_unwind doesn't tell us > > where a register was ultimately stored; it recurses for the register > > value, but fills in *optimizedp, *lvalp, *realnump, *addrp for where > > _this frame_ saved it. Which is reasonable and useful behavior. So, > > we need to iterate ourselves in order to find the final location. > > Don't we have that code already for evaluating variables? Can't that > code be re-used? We do have some similar code, but it can't readily be reused here. I want the frame prev_register to keep returning the non-transitive values; that's how we generate a useful "info frame" which says which registers were saved in this frame, instead of which registers were saved in any 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. Sure. It's simple: look for calls to frame_register and frame_register_unwind in the tdep files (alpha-tdep.c and alpha-mdebug-tdep.c), and compare them to the more common version which uses frame_unwind_register (I hate that naming...). The alpha version has the next frame fill in *lvalp and always recurses. The i386 version has the current frame fill in *lvalp and only recurses if we need the value. > I think the error message should be clear about which frame did not > save the PC. Could you give me an example of how to make it clearer? Here's what it looks like at the moment, IIRC (mocked up, I don't have easy access to the test right now): #0 0xffffe410 in __kernel_vsyscall () #1 0xb7e441cb in poll () from /lib/tls/i686/cmov/libc.so.6 #2 0x0810c713 in gdb_do_one_event () Backtrace stopped: frame did not save the PC And: (gdb) i frame 2 Stack frame at 0xbf8842c0: eip = 0x810c713 in gdb_do_one_event; saved eip 0x8109a43 Stops backtrace: frame did not save the PC called by frame at 0xbf8842f0 Arglist at 0xbf8842b8, args: Maybe "current frame" would be clearer? -- Daniel Jacobowitz CodeSourcery