From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7864 invoked by alias); 22 Feb 2002 18:38:28 -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 7762 invoked from network); 22 Feb 2002 18:38:26 -0000 Received: from unknown (HELO nevyn.them.org) (128.2.145.6) by sources.redhat.com with SMTP; 22 Feb 2002 18:38:26 -0000 Received: from drow by nevyn.them.org with local (Exim 3.34 #1 (Debian)) id 16eKaP-0004rz-00; Fri, 22 Feb 2002 13:38:25 -0500 Date: Fri, 22 Feb 2002 10:38: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: <20020222133825.B18410@nevyn.them.org> Mail-Followup-To: Richard.Earnshaw@arm.com, gdb-patches@sources.redhat.com References: <20020222105453.A11432@nevyn.them.org> <200202221821.SAA04295@cam-mail2.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200202221821.SAA04295@cam-mail2.cambridge.arm.com> User-Agent: Mutt/1.3.23i X-SW-Source: 2002-02/txt/msg00626.txt.bz2 On Fri, Feb 22, 2002 at 06:21:42PM +0000, Richard Earnshaw wrote: > > Just checking - CVS head GDB? I fixed a problem with identical > > symptoms in the stabs reader about three weeks ago. > > > > Yep (cvs head). > > > 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? > > Hmm, it's something derived from this entry, though I'm not sure if that's > the entire string: > > $27 = 0x509bc6 "__comp_dtor::813:_ZNSt14_STL_auto_lockD1Ev;2A.;operator=::8 > 14=#810,21,812,815=&816=k810,21;:_ZNSt14_STL_auto_lockaSERKS_;0A.;\\" > > It's somewhere near that 816=k810. I suspect we are somehow parsing this > line twice (or part of it). This makes sense. The 'k' means const. 810 looks to be the number for this type, since it's the first argument to operator=. And the demangling of the current method says it takes a const ref to its own type (thus the ERKS_, if I remember my mangling rules correctly). What compiler is this? It doesn't look like GCC; I believe GCC always generates type-pairs instead of type-nums. Is this some ARM compiler that obeys the v3 ABI? You might want to grab the entire stab out of the object file with objdump -G. A testcase might help me sort through this a little better; I was the last person to grub through the cv-type stuff. > > > 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. > > I thought so originally, but just fixing that didn't solve the problem. > > > > > > 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. > > > Urg. It all seems a complete mess to me :-) Another question: Given that > there seems to be a fundamental base type for the cv and as chains why are > they loops rather than chains with an additional pointer back to the base > type? As things stand we potentially need to create a lattice if we have > multiple c-v variants and multiple address spaces (though we don't at > present - why not?). That's a nightmare to maintain. It is in fact a nightmare. That's why I'm trying to eliminate most of the chaining. Using a base type structure and a type_variant structure covering both address spaces and cv-qual fixes this. In addition, I want to move cv-qual and address-space-qual onto the same chain. That requires fixing a lot of code. -- Daniel Jacobowitz Carnegie Mellon University MontaVista Software Debian GNU/Linux Developer