Infinite loop in make_cv_type

Infinite loop in make_cv_type

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]

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.

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.

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

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

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.

R.

<date>  Richard Earnshaw  (rearnsha@arm.com)

	* gdbtypes.c (make_cv_type): Handle being asked to modify an 
	existing type in the chain.
	(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.




[-- Attachment #2: gdb-gdbtypes.patch --]
[-- Type: text/x-patch , Size: 5226 bytes --]

Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.41
diff -p -r1.41 gdbtypes.c
*** gdbtypes.c	2002/02/08 17:34:33	1.41
--- gdbtypes.c	2002/02/22 11:30:14
*************** alloc_type (struct objfile *objfile)
*** 155,160 ****
--- 155,177 ----
    return (type);
  }
  
+ /* Reset the memory of a type structure.  */
+ struct type *
+ realloc_type (struct type *type, struct objfile *objfile)
+ {
+   memset ((char *) type, 0, sizeof (struct type));
+ 
+   /* Initialize the fields that might not be zero. */
+ 
+   TYPE_CODE (type) = TYPE_CODE_UNDEF;
+   TYPE_OBJFILE (type) = objfile;
+   TYPE_VPTR_FIELDNO (type) = -1;
+   TYPE_CV_TYPE (type) = type;	/* chain back to itself */
+   TYPE_AS_TYPE (type) = type;	/* ditto */
+ 
+   return (type);
+ }
+ 
  /* Lookup a pointer to a type TYPE.  TYPEPTR, if nonzero, points
     to a pointer to memory where the pointer type should be stored.
     If *TYPEPTR is zero, update it to point to the pointer type we return.
*************** make_pointer_type (struct type *type, st
*** 190,197 ****
      {
        ntype = *typeptr;
        objfile = TYPE_OBJFILE (ntype);
!       memset ((char *) ntype, 0, sizeof (struct type));
!       TYPE_OBJFILE (ntype) = objfile;
      }
  
    TYPE_TARGET_TYPE (ntype) = type;
--- 207,213 ----
      {
        ntype = *typeptr;
        objfile = TYPE_OBJFILE (ntype);
!       realloc_type (ntype, objfile);
      }
  
    TYPE_TARGET_TYPE (ntype) = type;
*************** make_reference_type (struct type *type, 
*** 257,264 ****
      {
        ntype = *typeptr;
        objfile = TYPE_OBJFILE (ntype);
!       memset ((char *) ntype, 0, sizeof (struct type));
!       TYPE_OBJFILE (ntype) = objfile;
      }
  
    TYPE_TARGET_TYPE (ntype) = type;
--- 273,279 ----
      {
        ntype = *typeptr;
        objfile = TYPE_OBJFILE (ntype);
!       realloc_type (ntype, objfile);
      }
  
    TYPE_TARGET_TYPE (ntype) = type;
*************** make_function_type (struct type *type, s
*** 306,313 ****
      {
        ntype = *typeptr;
        objfile = TYPE_OBJFILE (ntype);
!       memset ((char *) ntype, 0, sizeof (struct type));
!       TYPE_OBJFILE (ntype) = objfile;
      }
  
    TYPE_TARGET_TYPE (ntype) = type;
--- 321,327 ----
      {
        ntype = *typeptr;
        objfile = TYPE_OBJFILE (ntype);
!       realloc_type (ntype, objfile);
      }
  
    TYPE_TARGET_TYPE (ntype) = type;
*************** make_cv_type (int cnst, int voltl, struc
*** 413,421 ****
    register struct type *tmp_type = type;	/* tmp type */
    struct objfile *objfile;
  
!   ntype = TYPE_CV_TYPE (type);
  
!   while (ntype != type)
      {
        if ((TYPE_CONST (ntype) == cnst) &&
  	  (TYPE_VOLATILE (ntype) == voltl))
--- 427,435 ----
    register struct type *tmp_type = type;	/* tmp type */
    struct objfile *objfile;
  
!   ntype = type;
  
!   do
      {
        if ((TYPE_CONST (ntype) == cnst) &&
  	  (TYPE_VOLATILE (ntype) == voltl))
*************** make_cv_type (int cnst, int voltl, struc
*** 428,436 ****
--- 442,462 ----
  	      return ntype;
  	    }
  	}
+ 
+       /* We have storage and it matches something already in this type.  We
+ 	 must update it.  */
+       if (typeptr && *typeptr == ntype)
+ 	{
+ 	  /* Remove this from the chain (so that we can put it back again
+ 	     below).  */
+ 	  TYPE_CV_TYPE (tmp_type) = TYPE_CV_TYPE (ntype);
+ 	  break;
+ 	}
+ 
        tmp_type = ntype;
        ntype = TYPE_CV_TYPE (ntype);
      }
+   while (ntype != type);
  
    if (typeptr == 0 || *typeptr == 0)	/* We'll need to allocate one.  */
      {
*************** make_cv_type (int cnst, int voltl, struc
*** 467,473 ****
      TYPE_FLAGS (ntype) &= ~TYPE_FLAG_VOLATILE;
  
    /* Fix the chain of cv variants */
!   TYPE_CV_TYPE (ntype) = type;
    TYPE_CV_TYPE (tmp_type) = ntype;
  
    return ntype;
--- 493,499 ----
      TYPE_FLAGS (ntype) &= ~TYPE_FLAG_VOLATILE;
  
    /* Fix the chain of cv variants */
!   TYPE_CV_TYPE (ntype) = TYPE_CV_TYPE (tmp_type);
    TYPE_CV_TYPE (tmp_type) = ntype;
  
    return ntype;
*************** smash_to_member_type (struct type *type,
*** 877,884 ****
  
    objfile = TYPE_OBJFILE (type);
  
!   memset ((char *) type, 0, sizeof (struct type));
!   TYPE_OBJFILE (type) = objfile;
    TYPE_TARGET_TYPE (type) = to_type;
    TYPE_DOMAIN_TYPE (type) = domain;
    TYPE_LENGTH (type) = 1;	/* In practice, this is never needed.  */
--- 903,909 ----
  
    objfile = TYPE_OBJFILE (type);
  
!   realloc_type (type, objfile);
    TYPE_TARGET_TYPE (type) = to_type;
    TYPE_DOMAIN_TYPE (type) = domain;
    TYPE_LENGTH (type) = 1;	/* In practice, this is never needed.  */
*************** smash_to_method_type (struct type *type,
*** 900,907 ****
  
    objfile = TYPE_OBJFILE (type);
  
!   memset ((char *) type, 0, sizeof (struct type));
!   TYPE_OBJFILE (type) = objfile;
    TYPE_TARGET_TYPE (type) = to_type;
    TYPE_DOMAIN_TYPE (type) = domain;
    TYPE_ARG_TYPES (type) = args;
--- 925,931 ----
  
    objfile = TYPE_OBJFILE (type);
  
!   realloc_type (type, objfile);
    TYPE_TARGET_TYPE (type) = to_type;
    TYPE_DOMAIN_TYPE (type) = domain;
    TYPE_ARG_TYPES (type) = args;

Re: Infinite loop in make_cv_type

[Daniel Jacobowitz]

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

Re: Infinite loop in make_cv_type

[Michael Snyder]

Daniel Jacobowitz wrote:
> 
> 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.

Hang on, I'm pretty sure that the cv type chain
is supposed to do that.  The pointer loop is 
intentional.

Re: Infinite loop in make_cv_type

[Richard Earnshaw]

> Daniel Jacobowitz wrote:
> > 
> > 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.
> 
> Hang on, I'm pretty sure that the cv type chain
> is supposed to do that.  The pointer loop is 
> intentional.

Well if that's the case then the top loop in make_cv_type is completely 
bogus, since it expects to iterate until ntype==type (where type is the 
base type).

The comment in gdbtypes.h says that cv_type is a ring of the type and its 
variants.

R.

Re: Infinite loop in make_cv_type

[Richard Earnshaw]

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

Re: Infinite loop in make_cv_type

[Daniel Jacobowitz]

On Fri, Feb 22, 2002 at 09:18:46AM -0800, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> > 
> > 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.
> 
> Hang on, I'm pretty sure that the cv type chain
> is supposed to do that.  The pointer loop is 
> intentional.

Ring, not noose, I think.

I.E. anything pointing into the ring should be in fact on the ring.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: Infinite loop in make_cv_type

[Daniel Jacobowitz]

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

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

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

Re: Infinite loop in make_cv_type

[Richard Earnshaw]

drow@mvista.com said:
> 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?

It's gcc-current as of about 2 days ago.  Configured for arm-netbsd 
(native).

> You might want to grab the entire stab out of the object file with
> objdump -G.

The particular stab line is coming from libstdc++.so.

> A testcase might help me sort through this a little better; I was the
> last person to grub through the cv-type stuff. 

I'll see what I can come up with.

R.

Re: Infinite loop in make_cv_type

[Richard Earnshaw]

This gets even more bizarre.  I set a some breakpoints, one on 
make_cv_type, and another on dbx_next_symbol_text.  Now at the point of 
making the cv variant for the first time the previous call to 
dbx_next_symbol_text had returned

$736 = 0x509b43 "__comp_ctor::811:_ZNSt14_STL_auto_lockC1ERSt15_STL_mutex_l
ock;2A.;__base_dtor::813=#810,21,812,21;:_ZNSt14_STL_auto_lockD2Ev;2A.;\\"
(top-gdb) 

But going up the stack from make_cv_type we find:

(top-gdb) up
#1  0x000dac20 in read_type (pp=0xefbfca04, objfile=0x297000)
    at /nfs/sun18/work/rearnsha/gnusrc/src/gdb/src/gdb/stabsread.c:2682
2682          type = make_cv_type (TYPE_CONST (type), 1, type,
(top-gdb) p *pp
$737 = 0x509c1d ",21;:_ZNSt14_STL_auto_lockaSERKS_;0A.;\\"

But this is part of the text that is returned by the next call to 
dbx_next_symbol_text...

(top-gdb) c
Continuing.

Breakpoint 12, dbx_next_symbol_text (objfile=0x297000)
    at /nfs/sun18/work/rearnsha/gnusrc/src/gdb/src/gdb/dbxread.c:999
999     }
$738 = 0x509bc6 "__comp_dtor::813:_ZNSt14_STL_auto_lockD1Ev;2A.;operator=::
814=#810,21,812,815=&816=k810,21;:_ZNSt14_STL_auto_lockaSERKS_;0A.;\\"

Any suggestions as to how the stabs reader might be getting ahead of 
itself?  Is there another function that might be returning the stabs 
string?  I don't think dbx_next_symbol_text has ever returned this 
earlier...

R.

Re: Infinite loop in make_cv_type

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]


> Any suggestions as to how the stabs reader might be getting ahead of 
> itself?  Is there another function that might be returning the stabs 
> string?  I don't think dbx_next_symbol_text has ever returned this 
> earlier...

Dead simple really.  The stabs on the ARM are broken into very short 
strings for historical reasons (there was once an assembler that couldn't 
cope with stabs strings of more than about 100 characters).  We are simply 
running off the end of a stabs string without calling STABS_CONTINUE.  
Thus we end up parsing the following string twice: once on the overrun and 
the second when dbx_next_symbol_text returns it.

OK to apply?

R.

<date>  Richard Earnshaw  (rearnsha@arm.com)

	* stabsread.c (read_member_functions): Call STABS_CONTINUE after 
	skipping a method.



[-- Attachment #2: gdb-stab.patch --]
[-- Type: text/x-patch , Size: 445 bytes --]

Index: stabsread.c
===================================================================
RCS file: /cvs/src/src/gdb/stabsread.c,v
retrieving revision 1.27
diff -p -r1.27 stabsread.c
*** stabsread.c	2002/02/20 18:40:52	1.27
--- stabsread.c	2002/02/22 19:39:29
*************** read_member_functions (struct field_info
*** 3104,3109 ****
--- 3104,3110 ----
  	    (*pp) ++;
  	  (*pp) ++;
  
+ 	  STABS_CONTINUE (pp, objfile);
  	  continue;
  	}
  

Re: Infinite loop in make_cv_type

[Daniel Jacobowitz]

On Fri, Feb 22, 2002 at 07:44:44PM +0000, Richard Earnshaw wrote:
> 
> > Any suggestions as to how the stabs reader might be getting ahead of 
> > itself?  Is there another function that might be returning the stabs 
> > string?  I don't think dbx_next_symbol_text has ever returned this 
> > earlier...
> 
> Dead simple really.  The stabs on the ARM are broken into very short 
> strings for historical reasons (there was once an assembler that couldn't 
> cope with stabs strings of more than about 100 characters).  We are simply 
> running off the end of a stabs string without calling STABS_CONTINUE.  
> Thus we end up parsing the following string twice: once on the overrun and 
> the second when dbx_next_symbol_text returns it.
> 
> OK to apply?
> 
> R.
> 
> <date>  Richard Earnshaw  (rearnsha@arm.com)
> 
> 	* stabsread.c (read_member_functions): Call STABS_CONTINUE after 
> 	skipping a method.
> 
> 

My fault, it figures.  I've never been clear when STABS_CONTINUE is
actually necessary.  I can't approve it, but this looks good to me.

The rest of the problems you found should be fixed, but are not urgent;
I'll try to get to gdb/277 in the next few weeks...

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer