This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: Infinite loop in make_cv_type
- From: Daniel Jacobowitz <drow at mvista dot com>
- To: Richard dot Earnshaw at arm dot com
- Cc: gdb-patches at sources dot redhat dot com
- Date: Fri, 22 Feb 2002 10:54:53 -0500
- Subject: Re: Infinite loop in make_cv_type
- References: <200202221140.LAA24233@cam-mail2.cambridge.arm.com>
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 <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?
> 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.
> <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.
--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer