From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21838 invoked by alias); 21 Aug 2008 20:16:28 -0000 Received: (qmail 21829 invoked by uid 22791); 21 Aug 2008 20:16:28 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 21 Aug 2008 20:15:47 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id DB4B5981E9; Thu, 21 Aug 2008 20:15:45 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id C38EF9809F; Thu, 21 Aug 2008 20:15:45 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1KWGZN-0001bX-1S; Thu, 21 Aug 2008 16:15:45 -0400 Date: Thu, 21 Aug 2008 20:16:00 -0000 From: Daniel Jacobowitz To: Ulrich Weigand Cc: gdb-patches@sourceware.org Subject: Re: [rfc, v2] Fix frame_id_inner comparison false positives Message-ID: <20080821201545.GA4790@caradoc.them.org> Mail-Followup-To: Ulrich Weigand , gdb-patches@sourceware.org References: <20080625135107.GB8020@caradoc.them.org> <200808211942.m7LJgZZ5010975@d12av02.megacenter.de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200808211942.m7LJgZZ5010975@d12av02.megacenter.de.ibm.com> User-Agent: Mutt/1.5.17 (2008-05-11) X-IsSubscribed: yes 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 X-SW-Source: 2008-08/txt/msg00564.txt.bz2 On Thu, Aug 21, 2008 at 09:42:35PM +0200, Ulrich Weigand wrote: > > On the other hand, if the user has set a backtrace depth limit, then > > frame_find_by_id may fail if a function with a deep stack is called. > > We'll discard the dummy frame when it's still valid and get an extra > > sigtrap. This behavior should be straightforward to write a test case > > for: set the backtrace limit to ten, call a function which recurses > > twenty times and then hits a breakpoint, call another function, raise > > the backtrace limit and see if we've lost the outer dummy frame. > > This seems simply a bug of frame_find_by_id. This function should be > using get_prev_frame_1 instead of get_prev_frame. There is no reason > why re-identifying a frame you had already selected previously should > suddenly fail just because some user-interface setting was changed ... Well, here I'm not sure about. I don't consider 'set backtrace limit' to be just a user interface change; we use it as a safety net. For example. Suppose you have an unterminated M68K stack, and the top says '0xffffffff'. GDB will see that, assume it's a return address, check whether there's code there, decide there isn't (memory read fails), and use the stub unwinder - which adds 4 to the stack pointer, restores the return address, and goes along its merry recursive way. This is a bug in the 68k unwinder, of course. But it's a very easy bug to have and a very hard bug to catch. So I don't like algorithms that make us unwind past the immediate frame. The only place your patch switched to using frame_find_by_id instead of frame_id_inner was the dummy frame check, though. So maybe the right thing to do is just to remove that stale dummy frame check, and clear the dummy list when we get a new inferior? And stick to get_prev_frame, at least for the moment. Other than that, your logic looks entirely right to me. I like the clever corruption check. -- Daniel Jacobowitz CodeSourcery