[patch] [0/5] Types reference counting (for VLA+Python)

[patch] [0/5] Types reference counting (for VLA+Python)

[Jan Kratochvil]

Hi,

currently GDB knows only permanent types and objfile-associated types.  GDB
currently does not need more.  The only special kind are types copied from
closed objfiles for value history.  As the value history is always kept in full
length such types are in fact also permanent.

The VLA (variable-length-arrays) patchset needs to create possibly a new type
on each GDB stop (if inferior `variable[length]' will have different `length'
on each GDB stop).  The Python branch also dynamically creates new types used
by GDB itself but possibly completely unreferenced and deallocatable later.

To be true there exist some artificial temporary types such as in value_cast
/* FIXME-type-allocation: need a way to free this type when we are done with
   it.  */
, these can be freed by this implemented garbage collecting infrastructure.
These are already leaking in current FSF GDB; fix based on this framework is
currently not a part of this patchset.

This is a work based on Tom Tromey's original one but it got changed a lot so
blames to me first.

The basic decisions of this patchset are about the differences between permant
types (currently having TYPE_OBJFILE == NULL) and garbage collectable
(=reclaimable) types:

* Should the default allocation of types (alloc_type (NULL)) provide permanent
  type or a garbage collectable type?
  = Currently it creates a permanent type.
  It also means a fogotten GDB code part causes a type leak, not a crash.
  IMO there are more place where GDB allocates permanent types so the patchset
  is smaller this way.  Tried the opposite first but got lost in the patches.

* Should be the permanent<->reclaimable types differentiated by the
  TYPE_OBJFILE value itself or should both have NULL TYPE_OBJFILE and having
  some other differentiation factor?
  = Currently both have TYPE_OBJFILE (type) == NULL, one can find out if a type
  is reclaimable by:
      struct type_group_link link, *found;
      link.type = type;
      found = htab_find (type_group_link_table, &link);
      /* Not a permanent type?  */
      if (found)

  My former patchset was using:
      #define OBJFILE_INTERNAL ((struct objfile *) 1L)	/* =permanent */
      #define OBJFILE_MALLOC ((struct objfile *) 0L)	/* =reclaimable */
  But some GDB code finds out if a type is objfile-associated by a NULL
  comparison (no longer working for OBJFILE_MALLOC types).


There also exist cross-type references (such as TYPE_TARGET_TYPE and many
others).  While one type may be no longer in use GDB one must not free it as it
could create a stale reference in the other type.  Therefore some inter-type
references tracking is neeed.

The Tom Tromey's patchset changed the alloc_type prototype to always require
a parent (=related one) type to be specified:
-alloc_type (struct objfile *objfile)
+alloc_type (struct objfile *objfile, struct type *parent)
But for example create_array_type needs two parents (target type and index
type).  Also one needs to be careful to never create a reference to a type
without explicitely specifying the relation for memory management.

As I had to write a types-crawling sanity checker to check for any such missing
explicit references (by alloc_type's parent parameter) it got easier to already
make types relationship runtime autodetected - and not just checked - but this
function (now called type_group_link_check).

Regression tested on x86_64-unknown-linux-gnu with FSF GDB and with the
archer-jankratochvil-vla branch
(http://sourceware.org/gdb/wiki/ArcherBranchManagement).  Not tested with the
Python branch (which will need to explicitely define the allocated types as
reclaimable).



Thanks,
Jan

Re: [patch] [0/5] Types reference counting (for VLA+Python)

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> currently GDB knows only permanent types and objfile-associated
Jan> types.  GDB currently does not need more.  The only special kind
Jan> are types copied from closed objfiles for value history.  As the
Jan> value history is always kept in full length such types are in
Jan> fact also permanent.

FWIW, you can get a memory leak here with CVS gdb by setting a
convenience variable to a value whose type comes from an objfile,
closing all the objfiles ("file"), and then assigning a new value to
the convenience variable.  In this case nothing frees the type.

Jan> To be true there exist some artificial temporary types such as in
Jan> value_cast
[...]
Jan> , these can be freed by this implemented garbage collecting
Jan> infrastructure.  These are already leaking in current FSF GDB;
Jan> fix based on this framework is currently not a part of this
Jan> patchset.

Why is that?  (It doesn't matter in terms of acceptance, I'm just
curious.)

Jan> This is a work based on Tom Tromey's original one but it got
Jan> changed a lot so blames to me first.

:-)

Jan> The basic decisions of this patchset are about the differences
Jan> between permant types (currently having TYPE_OBJFILE == NULL) and
Jan> garbage collectable (=reclaimable) types:

Jan> * Should the default allocation of types (alloc_type (NULL))
Jan> provide permanent type or a garbage collectable type?
Jan>   = Currently it creates a permanent type.

Too bad the other approach didn't work out.  Where do all these
permanent types come from?  Not just init_type, I gather.

It seems strange to have a type-crawling garbage collector *and*
reference counting.  But I suppose this is because enumerating the
root set is tricky.

Jan>   Not tested with the Python branch (which will need to
Jan> explicitely define the allocated types as reclaimable).

I'll handle this, assuming the timing is right.  We're accumulating
quite a backlog of patches to be applied after the pretty-printing
series :-(

I think your approach seems sane in general.  I will comment on the
individual patches soon.

Tom

Re: [patch] [0/5] Types reference counting (for VLA+Python)

[Jan Kratochvil]

On Thu, 16 Apr 2009 21:32:11 +0200, Tom Tromey wrote:
> FWIW, you can get a memory leak here with CVS gdb by setting a
> convenience variable to a value whose type comes from an objfile,
> closing all the objfiles ("file"), and then assigning a new value to
> the convenience variable.  In this case nothing frees the type.

There will be still many leaks like this one, more places should change
alloc_type to alloc_type_discardable and possibly also include forgotten
type_incref/type_decref.  Going to patch this specific case separately.


> Jan> To be true there exist some artificial temporary types such as in
> Jan> value_cast
> [...]
> Jan> , these can be freed by this implemented garbage collecting
> Jan> infrastructure.  These are already leaking in current FSF GDB;
> Jan> fix based on this framework is currently not a part of this
> Jan> patchset.
> 
> Why is that?  (It doesn't matter in terms of acceptance, I'm just
> curious.)

value_cast for TYPE_CODE_ARRAY has a comment:
          /* FIXME-type-allocation: need a way to free this type when
             we are done with it.  */
because it does
                                     create_array_type ((struct type *) NULL,
                                                        element_type,
                                                        range_type));
used for resulting ARG2.  ARG2 will get freed by free_all_values but as this
new array type was not marked as reclaimable it will be leaked as permanent.


> Jan> * Should the default allocation of types (alloc_type (NULL))
> Jan> provide permanent type or a garbage collectable type?
> Jan>   = Currently it creates a permanent type.
> 
> Too bad the other approach didn't work out.  Where do all these
> permanent types come from?  Not just init_type, I gather.

For example _initialize_values() does something like init_type but using
alloc_type for it.  create_array_type needs to inherit the behavior from both
its range type and the index type.


> It seems strange to have a type-crawling garbage collector *and*
> reference counting.  But I suppose this is because enumerating the
> root set is tricky.

As already found there are (IMO) few places needing to allocate reclaimable
types it may be doable.  Thanks for the hint, I will try to rework it or at
least verify if it is viable.


Thanks,
Jan