This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Infinite loop in make_cv_type


> 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  <drow@mvista.com>
> 
>         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).

> 
> > 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.
> 
> > <date>  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.

R.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]