Re: [patch] [3/5] Types reference counting [make_function_type-objfile]

["Ulrich Weigand" <uweigand@de.ibm.com>]

Jan Kratochvil wrote:

> While jv-lang.c is also the case I originally found read_subroutine_type for
> function returning void - where such DW_TAG_subprogram has no DW_AT_type
> (return type) - and thus `builtin_type (gdbarch)->builtin_void' gets passed as
> TYPE to make_function_type originally deriving the OBJFILE for its returned
> function type from OBJFILE of that TYPE (internal in the void case).

Sorry for not commenting earlier -- I had overlooked this change before,
but ran into it now during my per-type architecture work.

It seems to me the two issues you mention above are actually bugs:

- I think the symbol readers should always return per-objfile types.  This
  just makes everything simpler, in particular allocation of derived types
  (as you notice).  I think it would also good to be able to guarantee that
  TYPE_OBJFILE of every symbol type is non-NULL ...

- The Java generated types should *not* go onto the "fake" dynamics objfile
  in the first place.  That objfile is somewhat bogus in that it isn't
  associated with an architecture (thus breaking my per-type architecture
  effort), and it also don't help with type lifetime issues as the fake
  objfile never goes away.  I think those types should be allocated with
  a NULL objfile (in the future are per-gdbarch types) instead.

> This patch fixes only the real-OBJFILE vs. NULL-OBJFILE part which fixes the
> leak even without the types reference counting / garbage collecting patch.
> 
> Whether NULL OBJFILE means permanent or discardable type I consider out of its
> scope.  You still can make it discardable by `type_init_group (function_type)'.

The patch introduces one more instance of an interface where objfile may
be NULL or non-NULL with different semantics; I'm trying to get rid of those.

Also, I think the implementation is broken in the "type smashing" case:
if there is an incoming type allocated in objfile A, but the argument to
make_function_type specifies objfile B, the type gets "smashed" and reused,
and its TYPE_OBJFILE gets redirected to objfile B even though the type
still resides within objfile A's obstack ...

struct type *
make_function_type (struct type *type, struct type **typeptr,
                    struct objfile *objfile)
{
  struct type *ntype;   /* New type */

  if (typeptr == 0 || *typeptr == 0)    /* We'll need to allocate one.  */
    {
      ntype = alloc_type (objfile);
      if (typeptr)
        *typeptr = ntype;
    }
  else                  /* We have storage, but need to reset it.  */
    {
      ntype = *typeptr;
      smash_type (ntype);
      TYPE_OBJFILE (ntype) = objfile;    <--- here
    }

Therefore, I'd prefer to fix the two problems mentioned above, and then
revert your patch.  (I'll be posting patches as a follow-up.)  This should
still solve the leak you originally experienced.  Would this be OK with you?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com