Re: PATCH: don't corrupt cv_type chain

[Jim Blandy <jimb at redhat dot com>]

Daniel Jacobowitz <drow@mvista.com> writes:
> > *** gdb/gdbtypes.c	26 Apr 2002 20:08:18 -0000	1.44
> > --- gdb/gdbtypes.c	4 May 2002 00:18:05 -0000
> > ***************
> > *** 521,530 ****
> >   
> >   /* Replace the contents of ntype with the type *type.
> >   
> > !    This function should not be necessary, but is due to quirks in the stabs
> > !    reader.  This should go away.  It does not handle the replacement type
> > !    being cv-qualified; it could be easily fixed to, but it should go away,
> > !    remember?  */
> >   void
> >   replace_type (struct type *ntype, struct type *type)
> >   {
> > --- 521,530 ----
> >   
> >   /* Replace the contents of ntype with the type *type.
> >   
> > !    When building recursive types, it is necessary to update a type's
> > !    definition after people already have references to it.  The C
> > !    language's concept of an `incomplete type' is an acknowledgement of
> > !    this.  */
> >   void
> >   replace_type (struct type *ntype, struct type *type)
> >   {
> 
> First, you removed the editorial; I think it is correct still.  More on
> that at the bottom.  Second, you removed the fact that it will not
> properly handle the replacement type being cv-qualified.  That's
> important!

Yes, it is.  I'll put it back.


> > Index: gdb/stabsread.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/stabsread.c,v
> > retrieving revision 1.31
> > diff -c -r1.31 stabsread.c
> > *** gdb/stabsread.c	4 May 2002 00:02:50 -0000	1.31
> > --- gdb/stabsread.c	4 May 2002 00:18:08 -0000
> > ***************
> > *** 2537,2543 ****
> >   	       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;
> > --- 2537,2560 ----
> >   	       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.
> > ! 
> > !                Well, it's not *absolutely* wrong.  Constructing recursive
> > !                types (trees, linked lists) necessarily entails modifying
> > !                types after creating them.  Constructing any loop structure
> > !                entails side effects.  The Dwarf 2 reader does handle this
> > !                more gracefully (it never constructs more than once
> > !                instance of a type object, so it doesn't have to copy type
> > !                objects wholesale), but it still mutates type objects after
> > !                other folks have references to them.
> > ! 
> > !                Keep in mind that this circularity/mutation issue shows up
> > !                at the source language level, too: C's "incomplete types",
> > !                for example.  So the proper cleanup, I think, would be to
> > !                limit GDB's type smashing to match exactly those required
> > !                by the source language.  So GDB could have a
> > !                "complete_this_type" function, but never create unnecessary
> > !                copies of a type otherwise.  */
> >   	    replace_type (type, xtype);
> >   	    TYPE_NAME (type) = NULL;
> >   	    TYPE_TAG_NAME (type) = NULL;
> 
> DWARF-2 has to mutate types somewhat, certainly; but this sort of
> smashing is avoidable and quite disgusting.  There should be an
> explicit list of things it is safe to modify, rather than the axe that
> is replace_type.  Adding a size or a a field list is legitimate, but
> many of the other fields should not change.

Isn't this, essentially, what my comment says?