From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23739 invoked by alias); 22 Feb 2002 15:55:01 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 23595 invoked from network); 22 Feb 2002 15:54:54 -0000 Received: from unknown (HELO nevyn.them.org) (128.2.145.6) by sources.redhat.com with SMTP; 22 Feb 2002 15:54:54 -0000 Received: from drow by nevyn.them.org with local (Exim 3.34 #1 (Debian)) id 16eI29-0003A6-00; Fri, 22 Feb 2002 10:54:53 -0500 Date: Fri, 22 Feb 2002 07:55:00 -0000 From: Daniel Jacobowitz To: Richard.Earnshaw@arm.com Cc: gdb-patches@sources.redhat.com Subject: Re: Infinite loop in make_cv_type Message-ID: <20020222105453.A11432@nevyn.them.org> Mail-Followup-To: Richard.Earnshaw@arm.com, gdb-patches@sources.redhat.com References: <200202221140.LAA24233@cam-mail2.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200202221140.LAA24233@cam-mail2.cambridge.arm.com> User-Agent: Mutt/1.3.23i X-SW-Source: 2002-02/txt/msg00618.txt.bz2 On Fri, Feb 22, 2002 at 11:40:38AM +0000, Richard Earnshaw wrote: > While testing cplusfuncs.exp on ARM/NetBSD (a.out) with gcc-3 current, gdb > is getting stuck in an infinite loop in gdbtypes.c:make_cv_type and I'm > trying to work out what this is supposed to do. The scenario I'm seeing > is that the type ring has become corrupted as follows along the > TYPE_CV_TYPE chain > > type > | > V > var1<----+ > | | > +------+ > > Given that this is supposed to be a loop, it's clearly bogus. Definitely. Just checking - CVS head GDB? I fixed a problem with identical symptoms in the stabs reader about three weeks ago. 2002-02-03 Daniel Jacobowitz PR gdb/280 * gdbtypes.c (replace_type): New function. * gdbtypes.h (replace_type): Add prototype. * stabsread.c (read_type): Use replace_type. > The reason for this seems to be that dbx_lookup_type is returning the > address of var1 as the place to put the modified type; ie it's asking > make_cv_type to modify an existing type variant. I'm not entirely sure > that this is correct, but it may be that this is how the stabs reader > creates a type -- ie it creates it, and then modifies it as it reads in > more attributes. No, that's not right. Once the type is created cv-qualifiers should never be added to it, unless you've got some really really bogus stabs. Could you supply the stabs that were being parsed when gdb hung? > There are several issues with make_cv_type: > > 1) Why is the top loop not executed at all when the cv loop has no > variants? It could be that we want the base type to be returned, and we > never can as the code is written (we end up creating an identical copy). We don't want the base type to be returned from any of the current callers, so I imagine no one fixed that. Er, there seems to be an exception in check_typedef for stubbed/opaque types. Ew. > 2) The chain updating at the end of the function is written in a bizarre > way, in that it assumes we will be inserting in the last entry before the > base type. While this has so-far been the case (because of the way the > top loop was written), it doesn't seem like the normal way to express such > an insertion operation (it implies that we could be dropping part of the > chain). True. This could possibly cause your symptom, too. > 3) There's no support for updating an existing entry in the loop. > > Adding the patch below solves that problem, but we then segfaults on > another type because the TYPE_CV_TYPE loop has been smashed by the memset > in make_pointer_type. This appears to happen at several places in that > file, and it seems to me that we really need a function realloc_type() > that is analogous to alloc_type, but recycles the type in a sensible > manner. I've written such a function as well. > > Of course, it could be that the stabs reader is doing something completely > bogus by passing the addresses of existing types into the gdbtypes code, > in which case that will have to be rewritten to prevent this; but it > doesn't seem like that was the intention. While 1) and 2) should be fixed, 3) should not be. See the comment I added to stabsread on February 2nd: /* This is the absolute wrong way to construct types. Every other debug format has found a way around this problem and the related problems with unnecessarily stubbed types; someone motivated should attempt to clean up the issue here as well. Once a type pointed to has been created it should not be modified. */ replace_type (type, xtype); TYPE_NAME (type) = NULL; TYPE_TAG_NAME (type) = NULL; Before I added that, it said: *type = *xtype; which obviously destroyed the CV chains. I'm going to smash make_cv_type sometime soon, by the way. See gdb/277. > Richard Earnshaw (rearnsha@arm.com) > > * gdbtypes.c (make_cv_type): Handle being asked to modify an > existing type in the chain. I'd rather that we detect this case and abort; and that we never hit the abort :) > (realloc_type): Cleanly recyle memory for a type. > (make_pointer_type): Use realloc_type to recycle an existing type. > (make_reference_type): Likewise. > (make_function_type): Likewise. > (smash_to_member_type): Likewise. > (smash_to_method_type): Likewise. The other places are quite probably correct. There's still an issue of some subtlety of what to do with the existing cv-chain; we don't want it getting lost. Rather, we want to update all of it. Thus gdb/277. -- Daniel Jacobowitz Carnegie Mellon University MontaVista Software Debian GNU/Linux Developer