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

Ulrich Weigand
2009-06-26T13:23:31Z

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,