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

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

[Jan Kratochvil]

Hi,

make_function_type was sometimes being called with permanent target type which
would now also create a permanent function type while it should be
objfile-associated function type instead.

This patch only fixes a small leak.


Thanks,
Jan


gdb/
2009-04-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Make specifiable the make_function_type type memory ownership.
	* gdbtypes.c (make_function_type): New parameter `objfile', use it
	explicitely instead of TYPE-initialized removed local variable
	`objfile'.  Describe `objfile' it in the function comment.
	(lookup_function_type): Update make_function_type callers.
	* gdbtypes.h (make_function_type): Update the prototype.
	* jv-lang.c (java_link_class_type): Update make_function_type callers.
	* dwarf2read.c (read_subroutine_type): Likewise.
	* stabsread.c (read_type): Likewise.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index c3f3838..bcf7bb8 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -5011,7 +5011,7 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
   struct attribute *attr;
 
   type = die_type (die, cu);
-  ftype = make_function_type (type, (struct type **) 0);
+  ftype = make_function_type (type, (struct type **) 0, cu->objfile);
 
   /* All functions in C++, Pascal and Java have prototypes.  */
   attr = dwarf2_attr (die, DW_AT_prototyped, cu);
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index fb52e1a..f415773 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -378,24 +378,24 @@ lookup_reference_type (struct type *type)
 /* Lookup a function type that returns type TYPE.  TYPEPTR, if
    nonzero, points to a pointer to memory where the function type
    should be stored.  If *TYPEPTR is zero, update it to point to the
-   function type we return.  We allocate new memory if needed.  */
+   function type we return.  We allocate new memory from OBJFILE if needed; use
+   NULL for permanent types.  */
 
 struct type *
-make_function_type (struct type *type, struct type **typeptr)
+make_function_type (struct type *type, struct type **typeptr,
+		    struct objfile *objfile)
 {
   struct type *ntype;	/* New type */
-  struct objfile *objfile;
 
   if (typeptr == 0 || *typeptr == 0)	/* We'll need to allocate one.  */
     {
-      ntype = alloc_type (TYPE_OBJFILE (type));
+      ntype = alloc_type (objfile);
       if (typeptr)
 	*typeptr = ntype;
     }
   else			/* We have storage, but need to reset it.  */
     {
       ntype = *typeptr;
-      objfile = TYPE_OBJFILE (ntype);
       smash_type (ntype);
       TYPE_OBJFILE (ntype) = objfile;
     }
@@ -415,7 +415,7 @@ make_function_type (struct type *type, struct type **typeptr)
 struct type *
 lookup_function_type (struct type *type)
 {
-  return make_function_type (type, (struct type **) 0);
+  return make_function_type (type, (struct type **) 0, TYPE_OBJFILE (type));
 }
 
 /* Identify address space identifier by name --
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 3c4e948..fc54acc 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1165,7 +1165,8 @@ extern struct type *make_pointer_type (struct type *, struct type **);
 
 extern struct type *lookup_pointer_type (struct type *);
 
-extern struct type *make_function_type (struct type *, struct type **);
+extern struct type *make_function_type (struct type *, struct type **,
+					struct objfile *);
 
 extern struct type *lookup_function_type (struct type *);
 
diff --git a/gdb/jv-lang.c b/gdb/jv-lang.c
index 0d0f4bc..ce17a28 100644
--- a/gdb/jv-lang.c
+++ b/gdb/jv-lang.c
@@ -557,7 +557,8 @@ java_link_class_type (struct type *type, struct value *clas)
 	}
       fn_fields[k].physname = "";
       fn_fields[k].is_stub = 1;
-      fn_fields[k].type = make_function_type (java_void_type, NULL);	/* FIXME */
+      /* FIXME */
+      fn_fields[k].type = make_function_type (java_void_type, NULL, objfile);
       TYPE_CODE (fn_fields[k].type) = TYPE_CODE_METHOD;
     }
 
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 5ce53e3..20bc4f5 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -1685,7 +1685,7 @@ again:
 
     case 'f':			/* Function returning another type */
       type1 = read_type (pp, objfile);
-      type = make_function_type (type1, dbx_lookup_type (typenums));
+      type = make_function_type (type1, dbx_lookup_type (typenums), objfile);
       break;
 
     case 'g':                   /* Prototyped function.  (Sun)  */
@@ -1708,7 +1708,8 @@ again:
         const char *type_start = (*pp) - 1;
         struct type *return_type = read_type (pp, objfile);
         struct type *func_type
-          = make_function_type (return_type, dbx_lookup_type (typenums));
+          = make_function_type (return_type, dbx_lookup_type (typenums),
+				objfile);
         struct type_list {
           struct type *type;
           struct type_list *next;

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

[Tom Tromey]

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

Jan> make_function_type was sometimes being called with permanent
Jan> target type which would now also create a permanent function type
Jan> while it should be objfile-associated function type instead.

The only one is in jv-lang.c, right?

This patch is ok.  I'd like to understand why function types are
always permanent, though.

Tom

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

[Jan Kratochvil]

On Thu, 16 Apr 2009 23:43:11 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> make_function_type was sometimes being called with permanent
> Jan> target type which would now also create a permanent function type
> Jan> while it should be objfile-associated function type instead.
> 
> The only one is in jv-lang.c, right?

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


> I'd like to understand why function types are always permanent, though.

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


> This patch is ok.

Checked it in in its original form:
	http://sourceware.org/ml/gdb-cvs/2009-05/msg00008.html


Thanks,
Jan

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

[Ulrich Weigand]

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

[rfc] Always use per-objfile types in symbol readers

[Ulrich Weigand]

Hello,

the symbol readers currently occasionally fall back to global or
architecture-provided builtin types to use as symbol types.  This
causes problems because those types are not associated with the
objfile in question.  In particular, this becomes problematic if
further derived types are based on such global types.

This patch changes the symbol readers to *always* provide per-objfile
types (i.e. those with TYPE_OBJFILE (type) != NULL) for any objfile-
related purposes, e.g. as symbol types.

This means in particular:

- Symbol readers can no longer access builtin_type ().  To simplify
  use of "default" types, I've added a new struct objfile_type that
  provides per-objfile copies of the default types.  Some elements
  of builtin_type can be moved completely to objfile_type, some are
  not needed in objfile_type, some are available in both.

- The routine stabsread.c:rs6000_builtin_type_data used to allocate
  global types; the patch changes that routine to keep those types
  per-objfile as well.

- The global builtin_type_error is removed; a builtin_error type is
  added to struct objfile_type instead.

Bye,
Ulrich


ChangeLog:

	* gdbtypes.h (struct builtin_type): Remove builtin_core_addr,
	nodebug_text_symbol, nodebug_data_symbol, nodebug_unknown_symbol,
	and nodebug_tls_symbol members.
	(struct objfile_type): New data structure.
	(objfile_type): Add prototype.
	* gdbtypes.c (gdbtypes_post_init): Remove initialization code
	for types no longer in struct builtin_type.
	(objfile_type_data): New static variable.
	(_initialize_gdbtypes): Initialize it.
	(objfile_type): New function.

	* gdbtypes.h (builtin_type_error): Remove.
	* gdbtypes.c (build_complex): Do not use builtin_type_error.
	* symtab.c (builtin_type_error): Remove.
	(_initialize_symtab): Remove initialization.

	* stabsread.c (dbx_lookup_type, define_symbol, error_type,
	rs6000_builtin_type, read_range_type): Use per-objfile types
	instead of global or per-architecture builtin types.
	* coffread.c (decode_type): Likewise.
	* dwarf2read.c (read_array_type, read_tag_string_type,
	new_symbol, die_type): Likewise.
	* mdebugread.c (parse_symbol, basic_type, upgrade_type,
	parse_procedure, psymtab_to_symtab_1): Likewise.
	* xcoffread.c (process_xcoff_symbol): Likewise.
	* parse.c (write_exp_msymbol): Likewise.

	* stabsread.c (rs6000_builtin_type_data): New static variable.
	(_initialize_stabsread): Initialize it.
	(rs6000_builtin_type): Add OBJFILE argument.  Allocate builtin
	types per-objfile instead of globally.

	* stabsread.c (dbx_lookup_type): Add OBJFILE argument.  Use it
	instead of current_objfile; pass it to rs6000_builtin_type.
	(dbx_alloc_type, read_type, read_range_type): Update calls.
	(cleanup_undefined_types_noname): Add OBJFILE argument and
	pass it to dbx_lookup_type.
	(cleanup_undefined_types): Add OBJFILE argument and pass it
	to cleanup_undefined_types_noname.
	* stabsread.h (cleanup_undefined_types): Add OBJFILE argument.
	* buildsym.c (end_symtab): Update call.


Index: gdb-head/gdb/stabsread.c
===================================================================
--- gdb-head.orig/gdb/stabsread.c
+++ gdb-head/gdb/stabsread.c
@@ -114,7 +114,7 @@ static struct type *read_sun_floating_ty
 
 static struct type *read_enum_type (char **, struct type *, struct objfile *);
 
-static struct type *rs6000_builtin_type (int);
+static struct type *rs6000_builtin_type (int, struct objfile *);
 
 static int
 read_member_functions (struct field_info *, char **, struct type *,
@@ -218,7 +218,7 @@ static int noname_undefs_length;
    or for associating a new type with the pair.  */
 
 static struct type **
-dbx_lookup_type (int typenums[2])
+dbx_lookup_type (int typenums[2], struct objfile *objfile)
 {
   int filenum = typenums[0];
   int index = typenums[1];
@@ -249,7 +249,7 @@ dbx_lookup_type (int typenums[2])
 	     this will do the right thing.  */
 	  static struct type *temp_type;
 
-	  temp_type = rs6000_builtin_type (index);
+	  temp_type = rs6000_builtin_type (index, objfile);
 	  return &temp_type;
 	}
 
@@ -280,18 +280,18 @@ dbx_lookup_type (int typenums[2])
     {
       real_filenum = this_object_header_files[filenum];
 
-      if (real_filenum >= N_HEADER_FILES (current_objfile))
+      if (real_filenum >= N_HEADER_FILES (objfile))
 	{
-	  static struct type **temp_type_p;
+	  static struct type *temp_type;
 
 	  warning (_("GDB internal error: bad real_filenum"));
 
 	error_return:
-	  temp_type_p = &builtin_type_error;
-	  return temp_type_p;
+	  temp_type = objfile_type (objfile)->builtin_error;
+	  return &temp_type;
 	}
 
-      f = HEADER_FILES (current_objfile) + real_filenum;
+      f = HEADER_FILES (objfile) + real_filenum;
 
       f_orig_length = f->length;
       if (index >= f_orig_length)
@@ -325,7 +325,7 @@ dbx_alloc_type (int typenums[2], struct 
       return (alloc_type (objfile));
     }
 
-  type_addr = dbx_lookup_type (typenums);
+  type_addr = dbx_lookup_type (typenums, objfile);
 
   /* If we are referring to a type not known at all yet,
      allocate an empty type for it.
@@ -772,7 +772,7 @@ define_symbol (CORE_ADDR valu, char *str
 	       target arithmetic to get the value.  real.c in GCC
 	       probably has the necessary code.  */
 
-	    dbl_type = builtin_type (gdbarch)->builtin_double;
+	    dbl_type = objfile_type (objfile)->builtin_double;
 	    dbl_valu =
 	      obstack_alloc (&objfile->objfile_obstack,
 			     TYPE_LENGTH (dbl_type));
@@ -792,7 +792,7 @@ define_symbol (CORE_ADDR valu, char *str
 	       types; other languages probably should have at least
 	       unsigned as well as signed constants.  */
 
-	    SYMBOL_TYPE (sym) = builtin_type (gdbarch)->builtin_long;
+	    SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_long;
 	    SYMBOL_VALUE (sym) = atoi (p);
 	    SYMBOL_CLASS (sym) = LOC_CONST;
 	  }
@@ -903,7 +903,7 @@ define_symbol (CORE_ADDR valu, char *str
 	         it back into builtin_int here.
 	         FIXME: Do we need a new builtin_promoted_int_arg ?  */
 	      if (TYPE_CODE (ptype) == TYPE_CODE_VOID)
-		ptype = builtin_type (gdbarch)->builtin_int;
+		ptype = objfile_type (objfile)->builtin_int;
 	      TYPE_FIELD_TYPE (ftype, nparams) = ptype;
 	      TYPE_FIELD_ARTIFICIAL (ftype, nparams++) = 0;
 	    }
@@ -995,8 +995,8 @@ define_symbol (CORE_ADDR valu, char *str
 	    {
 	      SYMBOL_TYPE (sym) =
 		TYPE_UNSIGNED (SYMBOL_TYPE (sym))
-		? builtin_type (gdbarch)->builtin_unsigned_int
-		: builtin_type (gdbarch)->builtin_int;
+		? objfile_type (objfile)->builtin_unsigned_int
+		: objfile_type (objfile)->builtin_int;
 	    }
 	  break;
 	}
@@ -1401,7 +1401,7 @@ error_type (char **pp, struct objfile *o
 	  break;
 	}
     }
-  return (builtin_type_error);
+  return objfile_type (objfile)->builtin_error;
 }
 \f
 
@@ -1577,7 +1577,7 @@ again:
 		  obstack_free (&objfile->objfile_obstack, type_name);
 		  type = SYMBOL_TYPE (sym);
 	          if (typenums[0] != -1)
-	            *dbx_lookup_type (typenums) = type;
+	            *dbx_lookup_type (typenums, objfile) = type;
 		  return type;
 		}
 	    }
@@ -1672,17 +1672,18 @@ again:
 
     case '*':			/* Pointer to another type */
       type1 = read_type (pp, objfile);
-      type = make_pointer_type (type1, dbx_lookup_type (typenums));
+      type = make_pointer_type (type1, dbx_lookup_type (typenums, objfile));
       break;
 
     case '&':			/* Reference to another type */
       type1 = read_type (pp, objfile);
-      type = make_reference_type (type1, dbx_lookup_type (typenums));
+      type = make_reference_type (type1, dbx_lookup_type (typenums, objfile));
       break;
 
     case 'f':			/* Function returning another type */
       type1 = read_type (pp, objfile);
-      type = make_function_type (type1, dbx_lookup_type (typenums), objfile);
+      type = make_function_type (type1, dbx_lookup_type (typenums, objfile),
+				 objfile);
       break;
 
     case 'g':                   /* Prototyped function.  (Sun)  */
@@ -1705,8 +1706,8 @@ again:
         const char *type_start = (*pp) - 1;
         struct type *return_type = read_type (pp, objfile);
         struct type *func_type
-          = make_function_type (return_type, dbx_lookup_type (typenums),
-				objfile);
+          = make_function_type (return_type,
+				dbx_lookup_type (typenums, objfile), objfile);
         struct type_list {
           struct type *type;
           struct type_list *next;
@@ -1762,13 +1763,13 @@ again:
     case 'k':			/* Const qualifier on some type (Sun) */
       type = read_type (pp, objfile);
       type = make_cv_type (1, TYPE_VOLATILE (type), type,
-			   dbx_lookup_type (typenums));
+			   dbx_lookup_type (typenums, objfile));
       break;
 
     case 'B':			/* Volatile qual on some type (Sun) */
       type = read_type (pp, objfile);
       type = make_cv_type (TYPE_CONST (type), 1, type,
-			   dbx_lookup_type (typenums));
+			   dbx_lookup_type (typenums, objfile));
       break;
 
     case '@':
@@ -1842,7 +1843,7 @@ again:
 		       symnum);
 	  type = allocate_stub_method (return_type);
 	  if (typenums[0] != -1)
-	    *dbx_lookup_type (typenums) = type;
+	    *dbx_lookup_type (typenums, objfile) = type;
 	}
       else
 	{
@@ -1870,7 +1871,7 @@ again:
     case 'r':			/* Range type */
       type = read_range_type (pp, typenums, type_size, objfile);
       if (typenums[0] != -1)
-	*dbx_lookup_type (typenums) = type;
+	*dbx_lookup_type (typenums, objfile) = type;
       break;
 
     case 'b':
@@ -1878,21 +1879,21 @@ again:
 	  /* Sun ACC builtin int type */
 	  type = read_sun_builtin_type (pp, typenums, objfile);
 	  if (typenums[0] != -1)
-	    *dbx_lookup_type (typenums) = type;
+	    *dbx_lookup_type (typenums, objfile) = type;
 	}
       break;
 
     case 'R':			/* Sun ACC builtin float type */
       type = read_sun_floating_type (pp, typenums, objfile);
       if (typenums[0] != -1)
-	*dbx_lookup_type (typenums) = type;
+	*dbx_lookup_type (typenums, objfile) = type;
       break;
 
     case 'e':			/* Enumeration type */
       type = dbx_alloc_type (typenums, objfile);
       type = read_enum_type (pp, type, objfile);
       if (typenums[0] != -1)
-	*dbx_lookup_type (typenums) = type;
+	*dbx_lookup_type (typenums, objfile) = type;
       break;
 
     case 's':			/* Struct type */
@@ -1932,7 +1933,7 @@ again:
       if (is_string)
 	TYPE_CODE (type) = TYPE_CODE_BITSTRING;
       if (typenums[0] != -1)
-	*dbx_lookup_type (typenums) = type;
+	*dbx_lookup_type (typenums, objfile) = type;
       break;
 
     default:
@@ -1957,20 +1958,31 @@ again:
 /* RS/6000 xlc/dbx combination uses a set of builtin types, starting from -1.
    Return the proper type node for a given builtin type number. */
 
+static const struct objfile_data *rs6000_builtin_type_data;
+
 static struct type *
-rs6000_builtin_type (int typenum)
+rs6000_builtin_type (int typenum, struct objfile *objfile)
 {
+  struct type **negative_types = objfile_data (objfile, rs6000_builtin_type_data);
+
   /* We recognize types numbered from -NUMBER_RECOGNIZED to -1.  */
 #define NUMBER_RECOGNIZED 34
-  /* This includes an empty slot for type number -0.  */
-  static struct type *negative_types[NUMBER_RECOGNIZED + 1];
   struct type *rettype = NULL;
 
   if (typenum >= 0 || typenum < -NUMBER_RECOGNIZED)
     {
       complaint (&symfile_complaints, _("Unknown builtin type %d"), typenum);
-      return builtin_type_error;
+      return objfile_type (objfile)->builtin_error;
     }
+
+  if (!negative_types)
+    {
+      /* This includes an empty slot for type number -0.  */
+      negative_types = OBSTACK_CALLOC (&objfile->objfile_obstack,
+				       NUMBER_RECOGNIZED + 1, struct type *);
+      set_objfile_data (objfile, rs6000_builtin_type_data, negative_types);
+    }
+
   if (negative_types[-typenum] != NULL)
     return negative_types[-typenum];
 
@@ -1990,129 +2002,129 @@ rs6000_builtin_type (int typenum)
          is other than 32 bits, then it should use a new negative type
          number (or avoid negative type numbers for that case).
          See stabs.texinfo.  */
-      rettype = init_type (TYPE_CODE_INT, 4, 0, "int", NULL);
+      rettype = init_type (TYPE_CODE_INT, 4, 0, "int", objfile);
       break;
     case 2:
-      rettype = init_type (TYPE_CODE_INT, 1, 0, "char", NULL);
+      rettype = init_type (TYPE_CODE_INT, 1, 0, "char", objfile);
       break;
     case 3:
-      rettype = init_type (TYPE_CODE_INT, 2, 0, "short", NULL);
+      rettype = init_type (TYPE_CODE_INT, 2, 0, "short", objfile);
       break;
     case 4:
-      rettype = init_type (TYPE_CODE_INT, 4, 0, "long", NULL);
+      rettype = init_type (TYPE_CODE_INT, 4, 0, "long", objfile);
       break;
     case 5:
       rettype = init_type (TYPE_CODE_INT, 1, TYPE_FLAG_UNSIGNED,
-			   "unsigned char", NULL);
+			   "unsigned char", objfile);
       break;
     case 6:
-      rettype = init_type (TYPE_CODE_INT, 1, 0, "signed char", NULL);
+      rettype = init_type (TYPE_CODE_INT, 1, 0, "signed char", objfile);
       break;
     case 7:
       rettype = init_type (TYPE_CODE_INT, 2, TYPE_FLAG_UNSIGNED,
-			   "unsigned short", NULL);
+			   "unsigned short", objfile);
       break;
     case 8:
       rettype = init_type (TYPE_CODE_INT, 4, TYPE_FLAG_UNSIGNED,
-			   "unsigned int", NULL);
+			   "unsigned int", objfile);
       break;
     case 9:
       rettype = init_type (TYPE_CODE_INT, 4, TYPE_FLAG_UNSIGNED,
-			   "unsigned", NULL);
+			   "unsigned", objfile);
     case 10:
       rettype = init_type (TYPE_CODE_INT, 4, TYPE_FLAG_UNSIGNED,
-			   "unsigned long", NULL);
+			   "unsigned long", objfile);
       break;
     case 11:
-      rettype = init_type (TYPE_CODE_VOID, 1, 0, "void", NULL);
+      rettype = init_type (TYPE_CODE_VOID, 1, 0, "void", objfile);
       break;
     case 12:
       /* IEEE single precision (32 bit).  */
-      rettype = init_type (TYPE_CODE_FLT, 4, 0, "float", NULL);
+      rettype = init_type (TYPE_CODE_FLT, 4, 0, "float", objfile);
       break;
     case 13:
       /* IEEE double precision (64 bit).  */
-      rettype = init_type (TYPE_CODE_FLT, 8, 0, "double", NULL);
+      rettype = init_type (TYPE_CODE_FLT, 8, 0, "double", objfile);
       break;
     case 14:
       /* This is an IEEE double on the RS/6000, and different machines with
          different sizes for "long double" should use different negative
          type numbers.  See stabs.texinfo.  */
-      rettype = init_type (TYPE_CODE_FLT, 8, 0, "long double", NULL);
+      rettype = init_type (TYPE_CODE_FLT, 8, 0, "long double", objfile);
       break;
     case 15:
-      rettype = init_type (TYPE_CODE_INT, 4, 0, "integer", NULL);
+      rettype = init_type (TYPE_CODE_INT, 4, 0, "integer", objfile);
       break;
     case 16:
       rettype = init_type (TYPE_CODE_BOOL, 4, TYPE_FLAG_UNSIGNED,
-			   "boolean", NULL);
+			   "boolean", objfile);
       break;
     case 17:
-      rettype = init_type (TYPE_CODE_FLT, 4, 0, "short real", NULL);
+      rettype = init_type (TYPE_CODE_FLT, 4, 0, "short real", objfile);
       break;
     case 18:
-      rettype = init_type (TYPE_CODE_FLT, 8, 0, "real", NULL);
+      rettype = init_type (TYPE_CODE_FLT, 8, 0, "real", objfile);
       break;
     case 19:
-      rettype = init_type (TYPE_CODE_ERROR, 0, 0, "stringptr", NULL);
+      rettype = init_type (TYPE_CODE_ERROR, 0, 0, "stringptr", objfile);
       break;
     case 20:
       rettype = init_type (TYPE_CODE_CHAR, 1, TYPE_FLAG_UNSIGNED,
-			   "character", NULL);
+			   "character", objfile);
       break;
     case 21:
       rettype = init_type (TYPE_CODE_BOOL, 1, TYPE_FLAG_UNSIGNED,
-			   "logical*1", NULL);
+			   "logical*1", objfile);
       break;
     case 22:
       rettype = init_type (TYPE_CODE_BOOL, 2, TYPE_FLAG_UNSIGNED,
-			   "logical*2", NULL);
+			   "logical*2", objfile);
       break;
     case 23:
       rettype = init_type (TYPE_CODE_BOOL, 4, TYPE_FLAG_UNSIGNED,
-			   "logical*4", NULL);
+			   "logical*4", objfile);
       break;
     case 24:
       rettype = init_type (TYPE_CODE_BOOL, 4, TYPE_FLAG_UNSIGNED,
-			   "logical", NULL);
+			   "logical", objfile);
       break;
     case 25:
       /* Complex type consisting of two IEEE single precision values.  */
-      rettype = init_type (TYPE_CODE_COMPLEX, 8, 0, "complex", NULL);
+      rettype = init_type (TYPE_CODE_COMPLEX, 8, 0, "complex", objfile);
       TYPE_TARGET_TYPE (rettype) = init_type (TYPE_CODE_FLT, 4, 0, "float",
-					      NULL);
+					      objfile);
       break;
     case 26:
       /* Complex type consisting of two IEEE double precision values.  */
       rettype = init_type (TYPE_CODE_COMPLEX, 16, 0, "double complex", NULL);
       TYPE_TARGET_TYPE (rettype) = init_type (TYPE_CODE_FLT, 8, 0, "double",
-					      NULL);
+					      objfile);
       break;
     case 27:
-      rettype = init_type (TYPE_CODE_INT, 1, 0, "integer*1", NULL);
+      rettype = init_type (TYPE_CODE_INT, 1, 0, "integer*1", objfile);
       break;
     case 28:
-      rettype = init_type (TYPE_CODE_INT, 2, 0, "integer*2", NULL);
+      rettype = init_type (TYPE_CODE_INT, 2, 0, "integer*2", objfile);
       break;
     case 29:
-      rettype = init_type (TYPE_CODE_INT, 4, 0, "integer*4", NULL);
+      rettype = init_type (TYPE_CODE_INT, 4, 0, "integer*4", objfile);
       break;
     case 30:
-      rettype = init_type (TYPE_CODE_CHAR, 2, 0, "wchar", NULL);
+      rettype = init_type (TYPE_CODE_CHAR, 2, 0, "wchar", objfile);
       break;
     case 31:
-      rettype = init_type (TYPE_CODE_INT, 8, 0, "long long", NULL);
+      rettype = init_type (TYPE_CODE_INT, 8, 0, "long long", objfile);
       break;
     case 32:
       rettype = init_type (TYPE_CODE_INT, 8, TYPE_FLAG_UNSIGNED,
-			   "unsigned long long", NULL);
+			   "unsigned long long", objfile);
       break;
     case 33:
       rettype = init_type (TYPE_CODE_INT, 8, TYPE_FLAG_UNSIGNED,
-			   "logical*8", NULL);
+			   "logical*8", objfile);
       break;
     case 34:
-      rettype = init_type (TYPE_CODE_INT, 8, 0, "integer*8", NULL);
+      rettype = init_type (TYPE_CODE_INT, 8, 0, "integer*8", objfile);
       break;
     }
   negative_types[-typenum] = rettype;
@@ -4062,9 +4074,9 @@ read_range_type (char **pp, int typenums
 handle_true_range:
 
   if (self_subrange)
-    index_type = builtin_type (gdbarch)->builtin_int;
+    index_type = objfile_type (objfile)->builtin_int;
   else
-    index_type = *dbx_lookup_type (rangenums);
+    index_type = *dbx_lookup_type (rangenums, objfile);
   if (index_type == NULL)
     {
       /* Does this actually ever happen?  Is that why we are worrying
@@ -4073,7 +4085,7 @@ handle_true_range:
       complaint (&symfile_complaints,
 		 _("base type %d of range type is not defined"), rangenums[1]);
 
-      index_type = builtin_type (gdbarch)->builtin_int;
+      index_type = objfile_type (objfile)->builtin_int;
     }
 
   result_type = create_range_type ((struct type *) NULL, index_type, n2, n3);
@@ -4293,7 +4305,7 @@ add_undefined_type (struct type *type, i
 /* Try to fix all undefined types pushed on the UNDEF_TYPES vector.  */
 
 static void
-cleanup_undefined_types_noname (void)
+cleanup_undefined_types_noname (struct objfile *objfile)
 {
   int i;
 
@@ -4302,7 +4314,7 @@ cleanup_undefined_types_noname (void)
       struct nat nat = noname_undefs[i];
       struct type **type;
 
-      type = dbx_lookup_type (nat.typenums);
+      type = dbx_lookup_type (nat.typenums, objfile);
       if (nat.type != *type && TYPE_CODE (*type) != TYPE_CODE_UNDEF)
         {
           /* The instance flags of the undefined type are still unset,
@@ -4415,10 +4427,10 @@ cleanup_undefined_types_1 (void)
    this unit.  */
 
 void
-cleanup_undefined_types (void)
+cleanup_undefined_types (struct objfile *objfile)
 {
   cleanup_undefined_types_1 ();
-  cleanup_undefined_types_noname ();
+  cleanup_undefined_types_noname (objfile);
 }
 
 /* Scan through all of the global symbols defined in the object file,
@@ -4650,6 +4662,8 @@ find_name_end (char *name)
 void
 _initialize_stabsread (void)
 {
+  rs6000_builtin_type_data = register_objfile_data ();
+
   undef_types_allocated = 20;
   undef_types_length = 0;
   undef_types = (struct type **)
Index: gdb-head/gdb/gdbtypes.c
===================================================================
--- gdb-head.orig/gdb/gdbtypes.c
+++ gdb-head/gdb/gdbtypes.c
@@ -3070,11 +3070,6 @@ static struct type *
 build_complex (int bit, char *name, struct type *target_type)
 {
   struct type *t;
-  if (bit <= 0 || target_type == builtin_type_error)
-    {
-      gdb_assert (builtin_type_error != NULL);
-      return builtin_type_error;
-    }
   t = init_type (TYPE_CODE_COMPLEX, 2 * bit / TARGET_CHAR_BIT,
 		 0, name, (struct objfile *) NULL);
   TYPE_TARGET_TYPE (t) = target_type;
@@ -3087,6 +3082,7 @@ gdbtypes_post_init (struct gdbarch *gdba
   struct builtin_type *builtin_type
     = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct builtin_type);
 
+  /* Basic types.  */
   builtin_type->builtin_void =
     init_type (TYPE_CODE_VOID, 1,
 	       0,
@@ -3179,7 +3175,130 @@ gdbtypes_post_init (struct gdbarch *gdba
 	       0,
 	       "_Decimal128", (struct objfile *) NULL);
 
-  /* Pointer/Address types.  */
+  /* Default data/code pointer types.  */
+  builtin_type->builtin_data_ptr =
+    make_pointer_type (builtin_type->builtin_void, NULL);
+  builtin_type->builtin_func_ptr =
+    lookup_pointer_type (lookup_function_type (builtin_type->builtin_void));
+
+  return builtin_type;
+}
+
+
+/* This set of objfile-based types is intended to be used by symbol
+   readers as basic types.  */
+
+static const struct objfile_data *objfile_type_data;
+
+const struct objfile_type *
+objfile_type (struct objfile *objfile)
+{
+  struct gdbarch *gdbarch;
+  struct objfile_type *objfile_type
+    = objfile_data (objfile, objfile_type_data);
+
+  if (objfile_type)
+    return objfile_type;
+
+  objfile_type = OBSTACK_CALLOC (&objfile->objfile_obstack,
+				 1, struct objfile_type);
+
+  /* Use the objfile architecture to determine basic type properties.  */
+  gdbarch = get_objfile_arch (objfile);
+
+  /* Basic types.  */
+  objfile_type->builtin_void
+    = init_type (TYPE_CODE_VOID, 1,
+		 0,
+		 "void", objfile);
+
+  objfile_type->builtin_char
+    = init_type (TYPE_CODE_INT, TARGET_CHAR_BIT / TARGET_CHAR_BIT,
+		 (TYPE_FLAG_NOSIGN
+		  | (gdbarch_char_signed (gdbarch) ? 0 : TYPE_FLAG_UNSIGNED)),
+		 "char", objfile);
+  objfile_type->builtin_signed_char
+    = init_type (TYPE_CODE_INT, TARGET_CHAR_BIT / TARGET_CHAR_BIT,
+		 0,
+		 "signed char", objfile);
+  objfile_type->builtin_unsigned_char
+    = init_type (TYPE_CODE_INT, TARGET_CHAR_BIT / TARGET_CHAR_BIT,
+		 TYPE_FLAG_UNSIGNED,
+		 "unsigned char", objfile);
+  objfile_type->builtin_short
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_short_bit (gdbarch) / TARGET_CHAR_BIT,
+		 0, "short", objfile);
+  objfile_type->builtin_unsigned_short
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_short_bit (gdbarch) / TARGET_CHAR_BIT,
+		 TYPE_FLAG_UNSIGNED, "unsigned short", objfile);
+  objfile_type->builtin_int
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_int_bit (gdbarch) / TARGET_CHAR_BIT,
+		 0, "int", objfile);
+  objfile_type->builtin_unsigned_int
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_int_bit (gdbarch) / TARGET_CHAR_BIT,
+		 TYPE_FLAG_UNSIGNED, "unsigned int", objfile);
+  objfile_type->builtin_long
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT,
+		 0, "long", objfile);
+  objfile_type->builtin_unsigned_long
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT,
+		 TYPE_FLAG_UNSIGNED, "unsigned long", objfile);
+  objfile_type->builtin_long_long
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_long_long_bit (gdbarch) / TARGET_CHAR_BIT,
+		 0, "long long", objfile);
+  objfile_type->builtin_unsigned_long_long
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_long_long_bit (gdbarch) / TARGET_CHAR_BIT,
+		 TYPE_FLAG_UNSIGNED, "unsigned long long", objfile);
+
+  objfile_type->builtin_float
+    = init_type (TYPE_CODE_FLT,
+		 gdbarch_float_bit (gdbarch) / TARGET_CHAR_BIT,
+		 0, "float", objfile);
+  TYPE_FLOATFORMAT (objfile_type->builtin_float)
+    = gdbarch_float_format (gdbarch);
+  objfile_type->builtin_double
+    = init_type (TYPE_CODE_FLT,
+		 gdbarch_double_bit (gdbarch) / TARGET_CHAR_BIT,
+		 0, "double", objfile);
+  TYPE_FLOATFORMAT (objfile_type->builtin_double)
+    = gdbarch_double_format (gdbarch);
+  objfile_type->builtin_long_double
+    = init_type (TYPE_CODE_FLT,
+		 gdbarch_long_double_bit (gdbarch) / TARGET_CHAR_BIT,
+		 0, "long double", objfile);
+  TYPE_FLOATFORMAT (objfile_type->builtin_long_double)
+    = gdbarch_long_double_format (gdbarch);
+
+  /* This type represents a type that was unrecognized in symbol read-in.  */
+  objfile_type->builtin_error
+    = init_type (TYPE_CODE_ERROR, 0, 0, "<unknown type>", objfile);
+
+  /* The following set of types is used for symbols with no
+     debug information.  */
+  objfile_type->nodebug_text_symbol
+    = init_type (TYPE_CODE_FUNC, 1, 0,
+		 "<text variable, no debug info>", objfile);
+  TYPE_TARGET_TYPE (objfile_type->nodebug_text_symbol)
+    = objfile_type->builtin_int;
+  objfile_type->nodebug_data_symbol
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_int_bit (gdbarch) / HOST_CHAR_BIT, 0,
+		 "<data variable, no debug info>", objfile);
+  objfile_type->nodebug_unknown_symbol
+    = init_type (TYPE_CODE_INT, 1, 0,
+		 "<variable (not text or data), no debug info>", objfile);
+  objfile_type->nodebug_tls_symbol
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_int_bit (gdbarch) / HOST_CHAR_BIT, 0,
+		 "<thread local variable, no debug info>", objfile);
 
   /* NOTE: on some targets, addresses and pointers are not necessarily
      the same --- for example, on the D10V, pointers are 16 bits long,
@@ -3203,49 +3322,27 @@ gdbtypes_post_init (struct gdbarch *gdba
      - If p is a D10V pointer type, TYPE_LENGTH (p) == 2, just as
        sizeof (void *) == 2 on the target.
 
-     In this context, builtin_type->CORE_ADDR is a bit odd: it's a
-     target type for a value the target will never see.  It's only
-     used to hold the values of (typeless) linker symbols, which are
-     indeed in the unified virtual address space.  */
+     In this context, objfile_type->builtin_core_addr is a bit odd:
+     it's a target type for a value the target will never see.  It's
+     only used to hold the values of (typeless) linker symbols, which
+     are indeed in the unified virtual address space.  */
+
+  objfile_type->builtin_core_addr
+    = init_type (TYPE_CODE_INT,
+		 gdbarch_addr_bit (gdbarch) / 8,
+		 TYPE_FLAG_UNSIGNED, "__CORE_ADDR", objfile);
 
-  builtin_type->builtin_data_ptr =
-    make_pointer_type (builtin_type->builtin_void, NULL);
-  builtin_type->builtin_func_ptr =
-    lookup_pointer_type (lookup_function_type (builtin_type->builtin_void));
-  builtin_type->builtin_core_addr =
-    init_type (TYPE_CODE_INT, 
-	       gdbarch_addr_bit (gdbarch) / 8,
-	       TYPE_FLAG_UNSIGNED,
-	       "__CORE_ADDR", (struct objfile *) NULL);
-
-
-  /* The following set of types is used for symbols with no
-     debug information.  */
-  builtin_type->nodebug_text_symbol =
-    init_type (TYPE_CODE_FUNC, 1, 0, 
-	       "<text variable, no debug info>", NULL);
-  TYPE_TARGET_TYPE (builtin_type->nodebug_text_symbol) =
-    builtin_type->builtin_int;
-  builtin_type->nodebug_data_symbol =
-    init_type (TYPE_CODE_INT, 
-	       gdbarch_int_bit (gdbarch) / HOST_CHAR_BIT, 0,
-	       "<data variable, no debug info>", NULL);
-  builtin_type->nodebug_unknown_symbol =
-    init_type (TYPE_CODE_INT, 1, 0,
-	       "<variable (not text or data), no debug info>", NULL);
-  builtin_type->nodebug_tls_symbol =
-    init_type (TYPE_CODE_INT, 
-	       gdbarch_int_bit (gdbarch) / HOST_CHAR_BIT, 0,
-	       "<thread local variable, no debug info>", NULL);
-
-  return builtin_type;
+  set_objfile_data (objfile, objfile_type_data, objfile_type);
+  return objfile_type;
 }
 
+
 extern void _initialize_gdbtypes (void);
 void
 _initialize_gdbtypes (void)
 {
   gdbtypes_data = gdbarch_data_register_post_init (gdbtypes_post_init);
+  objfile_type_data = register_objfile_data ();
 
   /* FIXME: The following types are architecture-neutral.  However,
      they contain pointer_type and reference_type fields potentially
Index: gdb-head/gdb/gdbtypes.h
===================================================================
--- gdb-head.orig/gdb/gdbtypes.h
+++ gdb-head/gdb/gdbtypes.h
@@ -957,7 +957,34 @@ extern void allocate_cplus_struct_type (
 
 struct builtin_type
 {
-  /* Address/pointer types.  */
+  /* Integral types.  */
+
+  /* Implicit size/sign (based on the the architecture's ABI).  */
+  struct type *builtin_void;
+  struct type *builtin_char;
+  struct type *builtin_short;
+  struct type *builtin_int;
+  struct type *builtin_long;
+  struct type *builtin_signed_char;
+  struct type *builtin_unsigned_char;
+  struct type *builtin_unsigned_short;
+  struct type *builtin_unsigned_int;
+  struct type *builtin_unsigned_long;
+  struct type *builtin_float;
+  struct type *builtin_double;
+  struct type *builtin_long_double;
+  struct type *builtin_complex;
+  struct type *builtin_double_complex;
+  struct type *builtin_string;
+  struct type *builtin_bool;
+  struct type *builtin_long_long;
+  struct type *builtin_unsigned_long_long;
+  struct type *builtin_decfloat;
+  struct type *builtin_decdouble;
+  struct type *builtin_declong;
+
+
+  /* Pointer types.  */
 
   /* `pointer to data' type.  Some target platforms use an implicitly
      {sign,zero} -extended 32-bit ABI pointer on a 64-bit ISA.  */
@@ -972,47 +999,48 @@ struct builtin_type
      However, all function pointer types are interconvertible, so void
      (*) () can server as a generic function pointer.  */
   struct type *builtin_func_ptr;
+};
 
-  /* The target CPU's address type.  This is the ISA address size.  */
-  struct type *builtin_core_addr;
-
-
-  /* Types used for symbols with no debug information.  */
-  struct type *nodebug_text_symbol;
-  struct type *nodebug_data_symbol;
-  struct type *nodebug_unknown_symbol;
-  struct type *nodebug_tls_symbol;
+/* Return the type table for the specified architecture.  */
+extern const struct builtin_type *builtin_type (struct gdbarch *gdbarch);
 
 
-  /* Integral types.  */
+/* Per-objfile types used by symbol readers.  */
 
-  /* Implicit size/sign (based on the the architecture's ABI).  */
+struct objfile_type
+{
+  /* Basic types based on the objfile architecture.  */
   struct type *builtin_void;
   struct type *builtin_char;
   struct type *builtin_short;
   struct type *builtin_int;
   struct type *builtin_long;
+  struct type *builtin_long_long;
   struct type *builtin_signed_char;
   struct type *builtin_unsigned_char;
   struct type *builtin_unsigned_short;
   struct type *builtin_unsigned_int;
   struct type *builtin_unsigned_long;
+  struct type *builtin_unsigned_long_long;
   struct type *builtin_float;
   struct type *builtin_double;
   struct type *builtin_long_double;
-  struct type *builtin_complex;
-  struct type *builtin_double_complex;
-  struct type *builtin_string;
-  struct type *builtin_bool;
-  struct type *builtin_long_long;
-  struct type *builtin_unsigned_long_long;
-  struct type *builtin_decfloat;
-  struct type *builtin_decdouble;
-  struct type *builtin_declong;
+
+  /* This type is used to represent symbol addresses.  */
+  struct type *builtin_core_addr;
+
+  /* This type represents a type that was unrecognized in symbol read-in.  */
+  struct type *builtin_error;
+
+  /* Types used for symbols with no debug information.  */
+  struct type *nodebug_text_symbol;
+  struct type *nodebug_data_symbol;
+  struct type *nodebug_unknown_symbol;
+  struct type *nodebug_tls_symbol;
 };
 
-/* Return the type table for the specified architecture.  */
-extern const struct builtin_type *builtin_type (struct gdbarch *gdbarch);
+/* Return the type table for the specified objfile.  */
+extern const struct objfile_type *objfile_type (struct objfile *objfile);
 
  
 /* Explicit sizes - see C9X <intypes.h> for naming scheme.  The "int0"
@@ -1064,14 +1092,6 @@ extern struct type *builtin_type_true_ch
 extern struct type *builtin_type_true_unsigned_char;
 
 
-/* This type represents a type that was unrecognized in symbol
-   read-in.  */
-extern struct type *builtin_type_error;
-
-
-/* RTTI for C++ */
-/* extern struct type *builtin_type_cxx_typeinfo; */
-
 /* Maximum and minimum values of built-in types */
 
 #define	MAX_OF_TYPE(t)	\
Index: gdb-head/gdb/symtab.c
===================================================================
--- gdb-head.orig/gdb/symtab.c
+++ gdb-head/gdb/symtab.c
@@ -146,9 +146,6 @@ multiple_symbols_select_mode (void)
   return multiple_symbols_mode;
 }
 
-/* The single non-language-specific builtin type */
-struct type *builtin_type_error;
-
 /* Block in which the most recently searched-for symbol was found.
    Might be better to make this a parameter to lookup_symbol and
    value_of_this. */
@@ -4720,9 +4717,5 @@ Show how the debugger handles ambiguitie
 Valid values are \"ask\", \"all\", \"cancel\", and the default is \"all\"."),
                         NULL, NULL, &setlist, &showlist);
 
-  /* Initialize the one built-in type that isn't language dependent... */
-  builtin_type_error = init_type (TYPE_CODE_ERROR, 0, 0,
-				  "<unknown type>", (struct objfile *) NULL);
-
   observer_attach_executable_changed (symtab_observer_executable_changed);
 }
Index: gdb-head/gdb/buildsym.c
===================================================================
--- gdb-head.orig/gdb/buildsym.c
+++ gdb-head/gdb/buildsym.c
@@ -997,7 +997,7 @@ end_symtab (CORE_ADDR end_addr, struct o
      are no-ops.  FIXME: Is this handled right in case of QUIT?  Can
      we make this cleaner?  */
 
-  cleanup_undefined_types ();
+  cleanup_undefined_types (objfile);
   finish_global_stabs (objfile);
 
   if (pending_blocks == NULL
Index: gdb-head/gdb/stabsread.h
===================================================================
--- gdb-head.orig/gdb/stabsread.h
+++ gdb-head/gdb/stabsread.h
@@ -129,7 +129,7 @@ EXTERN int n_this_object_header_files;
 
 EXTERN int n_allocated_this_object_header_files;
 
-extern void cleanup_undefined_types (void);
+extern void cleanup_undefined_types (struct objfile *);
 
 extern long read_number (char **, int);
 
Index: gdb-head/gdb/coffread.c
===================================================================
--- gdb-head.orig/gdb/coffread.c
+++ gdb-head/gdb/coffread.c
@@ -1713,7 +1713,7 @@ decode_type (struct coff_symbol *cs, uns
 	  *dim = 0;
 
 	  base_type = decode_type (cs, new_c_type, aux, objfile);
-	  index_type = builtin_type_int32;
+	  index_type = objfile_type (objfile)->builtin_int;
 	  range_type =
 	    create_range_type ((struct type *) NULL, index_type, 0, n - 1);
 	  type =
@@ -1777,39 +1777,39 @@ decode_base_type (struct coff_symbol *cs
     {
     case T_NULL:
       /* shows up with "void (*foo)();" structure members */
-      return builtin_type (gdbarch)->builtin_void;
+      return objfile_type (objfile)->builtin_void;
 
 #ifdef T_VOID
     case T_VOID:
       /* Intel 960 COFF has this symbol and meaning.  */
-      return builtin_type (gdbarch)->builtin_void;
+      return objfile_type (objfile)->builtin_void;
 #endif
 
     case T_CHAR:
-      return builtin_type (gdbarch)->builtin_char;
+      return objfile_type (objfile)->builtin_char;
 
     case T_SHORT:
-      return builtin_type (gdbarch)->builtin_short;
+      return objfile_type (objfile)->builtin_short;
 
     case T_INT:
-      return builtin_type (gdbarch)->builtin_int;
+      return objfile_type (objfile)->builtin_int;
 
     case T_LONG:
       if (cs->c_sclass == C_FIELD
 	  && aux->x_sym.x_misc.x_lnsz.x_size
 	     > gdbarch_long_bit (gdbarch))
-	return builtin_type (gdbarch)->builtin_long_long;
+	return objfile_type (objfile)->builtin_long_long;
       else
-	return builtin_type (gdbarch)->builtin_long;
+	return objfile_type (objfile)->builtin_long;
 
     case T_FLOAT:
-      return builtin_type (gdbarch)->builtin_float;
+      return objfile_type (objfile)->builtin_float;
 
     case T_DOUBLE:
-      return builtin_type (gdbarch)->builtin_double;
+      return objfile_type (objfile)->builtin_double;
 
     case T_LNGDBL:
-      return builtin_type (gdbarch)->builtin_long_double;
+      return objfile_type (objfile)->builtin_long_double;
 
     case T_STRUCT:
       if (cs->c_naux != 1)
@@ -1890,24 +1890,24 @@ decode_base_type (struct coff_symbol *cs
       break;
 
     case T_UCHAR:
-      return builtin_type (gdbarch)->builtin_unsigned_char;
+      return objfile_type (objfile)->builtin_unsigned_char;
 
     case T_USHORT:
-      return builtin_type (gdbarch)->builtin_unsigned_short;
+      return objfile_type (objfile)->builtin_unsigned_short;
 
     case T_UINT:
-      return builtin_type (gdbarch)->builtin_unsigned_int;
+      return objfile_type (objfile)->builtin_unsigned_int;
 
     case T_ULONG:
       if (cs->c_sclass == C_FIELD
 	  && aux->x_sym.x_misc.x_lnsz.x_size
 	     > gdbarch_long_bit (gdbarch))
-	return builtin_type (gdbarch)->builtin_unsigned_long_long;
+	return objfile_type (objfile)->builtin_unsigned_long_long;
       else
-	return builtin_type (gdbarch)->builtin_unsigned_long;
+	return objfile_type (objfile)->builtin_unsigned_long;
     }
   complaint (&symfile_complaints, _("Unexpected type for symbol %s"), cs->c_name);
-  return builtin_type (gdbarch)->builtin_void;
+  return objfile_type (objfile)->builtin_void;
 }
 \f
 /* This page contains subroutines of read_type.  */
Index: gdb-head/gdb/dwarf2read.c
===================================================================
--- gdb-head.orig/gdb/dwarf2read.c
+++ gdb-head/gdb/dwarf2read.c
@@ -4814,7 +4814,7 @@ read_array_type (struct die_info *die, s
      arrays with unspecified length.  */
   if (die->child == NULL)
     {
-      index_type = builtin_type_int32;
+      index_type = objfile_type (objfile)->builtin_int;
       range_type = create_range_type (NULL, index_type, 0, -1);
       type = create_array_type (NULL, element_type, range_type);
       return set_die_type (die, type, cu);
@@ -5254,7 +5254,7 @@ read_tag_string_type (struct die_info *d
         }
     }
 
-  index_type = builtin_type_int32;
+  index_type = objfile_type (objfile)->builtin_int;
   range_type = create_range_type (NULL, index_type, 1, length);
   char_type = language_string_char_type (cu->language_defn, gdbarch);
   type = create_string_type (NULL, char_type, range_type);
@@ -7714,7 +7714,6 @@ static struct symbol *
 new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 {
   struct objfile *objfile = cu->objfile;
-  struct gdbarch *gdbarch = get_objfile_arch (objfile);
   struct symbol *sym = NULL;
   char *name;
   struct attribute *attr = NULL;
@@ -7806,7 +7805,7 @@ new_symbol (struct die_info *die, struct
 	     to something sensible.  */
 	  if (TYPE_CODE (SYMBOL_TYPE (sym)) == TYPE_CODE_VOID)
 	    SYMBOL_TYPE (sym)
-	      = builtin_type (gdbarch)->nodebug_data_symbol;
+	      = objfile_type (objfile)->nodebug_data_symbol;
 
 	  attr = dwarf2_attr (die, DW_AT_const_value, cu);
 	  if (attr)
@@ -8107,7 +8106,6 @@ dwarf2_const_value_data (struct attribut
 static struct type *
 die_type (struct die_info *die, struct dwarf2_cu *cu)
 {
-  struct gdbarch *gdbarch = get_objfile_arch (cu->objfile);
   struct type *type;
   struct attribute *type_attr;
   struct die_info *type_die;
@@ -8116,7 +8114,7 @@ die_type (struct die_info *die, struct d
   if (!type_attr)
     {
       /* A missing DW_AT_type represents a void type.  */
-      return builtin_type (gdbarch)->builtin_void;
+      return objfile_type (cu->objfile)->builtin_void;
     }
   else
     type_die = follow_die_ref (die, type_attr, &cu);
Index: gdb-head/gdb/mdebugread.c
===================================================================
--- gdb-head.orig/gdb/mdebugread.c
+++ gdb-head/gdb/mdebugread.c
@@ -636,7 +636,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
       /* Type could be missing if file is compiled without debugging info.  */
       if (SC_IS_UNDEF (sh->sc)
 	  || sh->sc == scNil || sh->index == indexNil)
-	SYMBOL_TYPE (s) = builtin_type (gdbarch)->nodebug_data_symbol;
+	SYMBOL_TYPE (s) = objfile_type (objfile)->nodebug_data_symbol;
       else
 	SYMBOL_TYPE (s) = parse_type (cur_fd, ax, sh->index, 0, bigend, name);
       /* Value of a data symbol is its memory address */
@@ -685,7 +685,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
       SYMBOL_DOMAIN (s) = VAR_DOMAIN;	/* so that it can be used */
       SYMBOL_CLASS (s) = LOC_LABEL;	/* but not misused */
       SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
-      SYMBOL_TYPE (s) = builtin_type (gdbarch)->builtin_int;
+      SYMBOL_TYPE (s) = objfile_type (objfile)->builtin_int;
       add_symbol (s, top_stack->cur_st, top_stack->cur_block);
       break;
 
@@ -728,7 +728,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
       SYMBOL_CLASS (s) = LOC_BLOCK;
       /* Type of the return value */
       if (SC_IS_UNDEF (sh->sc) || sh->sc == scNil)
-	t = builtin_type (gdbarch)->builtin_int;
+	t = objfile_type (objfile)->builtin_int;
       else
 	{
 	  t = parse_type (cur_fd, ax, sh->index + 1, 0, bigend, name);
@@ -1138,7 +1138,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
 	  s = new_symbol (MDEBUG_EFI_SYMBOL_NAME);
 	  SYMBOL_DOMAIN (s) = LABEL_DOMAIN;
 	  SYMBOL_CLASS (s) = LOC_CONST;
-	  SYMBOL_TYPE (s) = builtin_type_void;
+	  SYMBOL_TYPE (s) = objfile_type (current_objfile)->builtin_void;
 	  e = ((struct mdebug_extra_func_info *)
 	       obstack_alloc (&current_objfile->objfile_obstack,
 			      sizeof (struct mdebug_extra_func_info)));
@@ -1359,13 +1359,13 @@ basic_type (int bt, struct objfile *objf
   switch (bt)
     {
     case btNil:
-      tp = builtin_type_void;
+      tp = objfile_type (objfile)->builtin_void;
       break;
 
     case btAdr:
       tp = init_type (TYPE_CODE_PTR, 4, TYPE_FLAG_UNSIGNED,
 		      "adr_32", objfile);
-      TYPE_TARGET_TYPE (tp) = builtin_type_void;
+      TYPE_TARGET_TYPE (tp) = objfile_type (objfile)->builtin_void;
       break;
 
     case btChar:
@@ -1457,7 +1457,7 @@ basic_type (int bt, struct objfile *objf
       break;
 
     case btVoid:
-      tp = builtin_type_void;
+      tp = objfile_type (objfile)->builtin_void;
       break;
 
     case btLong64:
@@ -1483,7 +1483,7 @@ basic_type (int bt, struct objfile *objf
     case btAdr64:
       tp = init_type (TYPE_CODE_PTR, 8, TYPE_FLAG_UNSIGNED,
 		      "adr_64", objfile);
-      TYPE_TARGET_TYPE (tp) = builtin_type_void;
+      TYPE_TARGET_TYPE (tp) = objfile_type (objfile)->builtin_void;
       break;
 
     case btInt64:
@@ -1849,7 +1849,7 @@ upgrade_type (int fd, struct type **tpp,
 	{
 	  complaint (&symfile_complaints,
 		     _("illegal array index type for %s, assuming int"), sym_name);
-	  indx = builtin_type_int32;
+	  indx = objfile_type (current_objfile)->builtin_int;
 	}
 
       /* Get the bounds, and create the array type.  */
@@ -1918,7 +1918,6 @@ static void
 parse_procedure (PDR *pr, struct symtab *search_symtab,
 		 struct partial_symtab *pst)
 {
-  struct gdbarch *gdbarch = get_objfile_arch (pst->objfile);
   struct symbol *s, *i;
   struct block *b;
   char *sh_name;
@@ -2002,7 +2001,7 @@ parse_procedure (PDR *pr, struct symtab 
       SYMBOL_CLASS (s) = LOC_BLOCK;
       /* Donno its type, hope int is ok */
       SYMBOL_TYPE (s)
-	= lookup_function_type (builtin_type (gdbarch)->builtin_int);
+	= lookup_function_type (objfile_type (pst->objfile)->builtin_int);
       add_symbol (s, top_stack->cur_st, top_stack->cur_block);
       /* Wont have symbols for this one */
       b = new_block (2);
@@ -2057,7 +2056,7 @@ parse_procedure (PDR *pr, struct symtab 
   if (processing_gcc_compilation == 0
       && found_ecoff_debugging_info == 0
       && TYPE_CODE (TYPE_TARGET_TYPE (SYMBOL_TYPE (s))) == TYPE_CODE_VOID)
-    SYMBOL_TYPE (s) = builtin_type (gdbarch)->nodebug_text_symbol;
+    SYMBOL_TYPE (s) = objfile_type (pst->objfile)->nodebug_text_symbol;
 }
 
 /* Parse the external symbol ES. Just call parse_symbol() after
@@ -4043,7 +4042,7 @@ psymtab_to_symtab_1 (struct partial_symt
 		  memset (e, 0, sizeof (struct mdebug_extra_func_info));
 		  SYMBOL_DOMAIN (s) = LABEL_DOMAIN;
 		  SYMBOL_CLASS (s) = LOC_CONST;
-		  SYMBOL_TYPE (s) = builtin_type_void;
+		  SYMBOL_TYPE (s) = objfile_type (pst->objfile)->builtin_void;
 		  SYMBOL_VALUE_BYTES (s) = (gdb_byte *) e;
 		  e->pdr.framereg = -1;
 		  add_symbol_to_list (s, &local_symbols);
Index: gdb-head/gdb/parse.c
===================================================================
--- gdb-head.orig/gdb/parse.c
+++ gdb-head/gdb/parse.c
@@ -489,7 +489,7 @@ write_exp_msymbol (struct minimal_symbol
 
   write_exp_elt_opcode (OP_LONG);
   /* Let's make the type big enough to hold a 64-bit address.  */
-  write_exp_elt_type (builtin_type (gdbarch)->builtin_core_addr);
+  write_exp_elt_type (objfile_type (objfile)->builtin_core_addr);
   write_exp_elt_longcst ((LONGEST) addr);
   write_exp_elt_opcode (OP_LONG);
 
@@ -497,7 +497,7 @@ write_exp_msymbol (struct minimal_symbol
     {
       write_exp_elt_opcode (UNOP_MEMVAL_TLS);
       write_exp_elt_objfile (objfile);
-      write_exp_elt_type (builtin_type (gdbarch)->nodebug_tls_symbol);
+      write_exp_elt_type (objfile_type (objfile)->nodebug_tls_symbol);
       write_exp_elt_opcode (UNOP_MEMVAL_TLS);
       return;
     }
@@ -508,18 +508,18 @@ write_exp_msymbol (struct minimal_symbol
     case mst_text:
     case mst_file_text:
     case mst_solib_trampoline:
-      write_exp_elt_type (builtin_type (gdbarch)->nodebug_text_symbol);
+      write_exp_elt_type (objfile_type (objfile)->nodebug_text_symbol);
       break;
 
     case mst_data:
     case mst_file_data:
     case mst_bss:
     case mst_file_bss:
-      write_exp_elt_type (builtin_type (gdbarch)->nodebug_data_symbol);
+      write_exp_elt_type (objfile_type (objfile)->nodebug_data_symbol);
       break;
 
     default:
-      write_exp_elt_type (builtin_type (gdbarch)->nodebug_unknown_symbol);
+      write_exp_elt_type (objfile_type (objfile)->nodebug_unknown_symbol);
       break;
     }
   write_exp_elt_opcode (UNOP_MEMVAL);
Index: gdb-head/gdb/xcoffread.c
===================================================================
--- gdb-head.orig/gdb/xcoffread.c
+++ gdb-head/gdb/xcoffread.c
@@ -1454,7 +1454,6 @@ read_xcoff_symtab (struct partial_symtab
 static struct symbol *
 process_xcoff_symbol (struct coff_symbol *cs, struct objfile *objfile)
 {
-  struct gdbarch *gdbarch = get_objfile_arch (objfile);
   struct symbol onesymbol;
   struct symbol *sym = &onesymbol;
   struct symbol *sym2 = NULL;
@@ -1494,7 +1493,7 @@ process_xcoff_symbol (struct coff_symbol
          patch_block_stabs (), unless the file was compiled without -g.  */
 
       SYMBOL_SET_LINKAGE_NAME (sym, SYMNAME_ALLOC (name, symname_alloced));
-      SYMBOL_TYPE (sym) = builtin_type (gdbarch)->nodebug_text_symbol;
+      SYMBOL_TYPE (sym) = objfile_type (objfile)->nodebug_text_symbol;
 
       SYMBOL_CLASS (sym) = LOC_BLOCK;
       SYMBOL_DUP (sym, sym2);
@@ -1507,7 +1506,7 @@ process_xcoff_symbol (struct coff_symbol
   else
     {
       /* In case we can't figure out the type, provide default. */
-      SYMBOL_TYPE (sym) = builtin_type (gdbarch)->nodebug_data_symbol;
+      SYMBOL_TYPE (sym) = objfile_type (objfile)->nodebug_data_symbol;
 
       switch (cs->c_sclass)
 	{

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

[rfc] Fix Java type allocation and revert make_function_type change

[Ulrich Weigand]

Hello,

as mentioned in
http://sourceware.org/ml/gdb-patches/2009-06/msg00706.html
this patch changes jv-lang.c to not allocate temporary class types
on the "fake" dynamics objfile, and then reverts the change to add
an objfile parameter to make_function_type.

Tested together with the previous patch on amd64-linux and
powerpc64-linux (also with stabs).

Bye,
Ulrich


ChangeLog:

	* gdbtypes.h (make_function_type): Remove OBJFILE parameter.
	* gdbtypes.c (make_function_type): Remove OBJFILE parameter.
	(lookup_function_type): Update call.
	* stabsread.c (read_type): Likewise.
	* dwarf2read.c (read_subroutine_type): Use lookup_function_type
	instead of make_function_type.
	* jv-lang.c (type_from_class): Likewise.  Do not allocate types
	on the fake "dynamics" objfile.


Index: gdb-head/gdb/dwarf2read.c
===================================================================
--- gdb-head.orig/gdb/dwarf2read.c
+++ gdb-head/gdb/dwarf2read.c
@@ -5281,7 +5281,7 @@ read_subroutine_type (struct die_info *d
   struct attribute *attr;
 
   type = die_type (die, cu);
-  ftype = make_function_type (type, (struct type **) 0, cu->objfile);
+  ftype = lookup_function_type (type);
 
   /* All functions in C++, Pascal and Java have prototypes.  */
   attr = dwarf2_attr (die, DW_AT_prototyped, cu);
Index: gdb-head/gdb/gdbtypes.c
===================================================================
--- gdb-head.orig/gdb/gdbtypes.c
+++ gdb-head/gdb/gdbtypes.c
@@ -378,24 +378,24 @@ lookup_reference_type (struct type *type
 /* Lookup a function type that returns type TYPE.  TYPEPTR, if
    nonzero, points to a pointer to memory where the function type
    should be stored.  If *TYPEPTR is zero, update it to point to the
-   function type we return.  We allocate new memory from OBJFILE if needed; use
-   NULL for permanent types.  */
+   function type we return.  We allocate new memory if needed.  */
 
 struct type *
-make_function_type (struct type *type, struct type **typeptr,
-		    struct objfile *objfile)
+make_function_type (struct type *type, struct type **typeptr)
 {
   struct type *ntype;	/* New type */
+  struct objfile *objfile;
 
   if (typeptr == 0 || *typeptr == 0)	/* We'll need to allocate one.  */
     {
-      ntype = alloc_type (objfile);
+      ntype = alloc_type (TYPE_OBJFILE (type));
       if (typeptr)
 	*typeptr = ntype;
     }
   else			/* We have storage, but need to reset it.  */
     {
       ntype = *typeptr;
+      objfile = TYPE_OBJFILE (ntype);
       smash_type (ntype);
       TYPE_OBJFILE (ntype) = objfile;
     }
@@ -415,7 +415,7 @@ make_function_type (struct type *type, s
 struct type *
 lookup_function_type (struct type *type)
 {
-  return make_function_type (type, (struct type **) 0, TYPE_OBJFILE (type));
+  return make_function_type (type, (struct type **) 0);
 }
 
 /* Identify address space identifier by name --
Index: gdb-head/gdb/gdbtypes.h
===================================================================
--- gdb-head.orig/gdb/gdbtypes.h
+++ gdb-head/gdb/gdbtypes.h
@@ -1186,8 +1186,7 @@ extern struct type *make_pointer_type (s
 
 extern struct type *lookup_pointer_type (struct type *);
 
-extern struct type *make_function_type (struct type *, struct type **,
-					struct objfile *);
+extern struct type *make_function_type (struct type *, struct type **);
 
 extern struct type *lookup_function_type (struct type *);
 
Index: gdb-head/gdb/stabsread.c
===================================================================
--- gdb-head.orig/gdb/stabsread.c
+++ gdb-head/gdb/stabsread.c
@@ -1682,8 +1682,7 @@ again:
 
     case 'f':			/* Function returning another type */
       type1 = read_type (pp, objfile);
-      type = make_function_type (type1, dbx_lookup_type (typenums, objfile),
-				 objfile);
+      type = make_function_type (type1, dbx_lookup_type (typenums, objfile));
       break;
 
     case 'g':                   /* Prototyped function.  (Sun)  */
@@ -1707,7 +1706,7 @@ again:
         struct type *return_type = read_type (pp, objfile);
         struct type *func_type
           = make_function_type (return_type,
-				dbx_lookup_type (typenums, objfile), objfile);
+				dbx_lookup_type (typenums, objfile));
         struct type_list {
           struct type *type;
           struct type_list *next;
Index: gdb-head/gdb/jv-lang.c
===================================================================
--- gdb-head.orig/gdb/jv-lang.c
+++ gdb-head/gdb/jv-lang.c
@@ -302,7 +302,10 @@ type_from_class (struct value *clas)
   if (type != NULL)
     return type;
 
-  type = alloc_type (objfile);
+  /* Do not use the "fake" dynamics objfile to own dynamically generated
+     types, as it does not provide an architecture, and it would not help
+     manage the lifetime of these types anyway.  */
+  type = alloc_type (NULL);
   TYPE_CODE (type) = TYPE_CODE_STRUCT;
   INIT_CPLUS_SPECIFIC (type);
 
@@ -560,7 +563,7 @@ java_link_class_type (struct type *type,
       fn_fields[k].physname = "";
       fn_fields[k].is_stub = 1;
       /* FIXME */
-      fn_fields[k].type = make_function_type (java_void_type, NULL, objfile);
+      fn_fields[k].type = lookup_function_type (java_void_type);
       TYPE_CODE (fn_fields[k].type) = TYPE_CODE_METHOD;
     }
 

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

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

[Tom Tromey]

>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:

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

My recollection is that we have a few places where an objfile-attached
type can be derived from a NULL-objfile type.  Index types at least,
but ISTR also something from Ada.

I'm fine with changing this and adding an invariant like "a type with
an objfile may only reference other types from the same objfile".  I
think I must have just assumed that this would be too hard.

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

This phony objfile is also not multi-inferior-safe.

It seems to me that this java stuff is like a rough prototype of how
better debugging for JITs might work -- make a dynamic objfile based
on information from the running inferior.  So, I suppose it would be
nice to fix it up rather than nuke it :)

For the lifetime issue, it seems like this objfile should be treated
however a .so objfile is treated.

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

Is this really valid?  It seems to me that it ought to be an internal
error.  Otherwise aren't we at risk for memory corruption, if objfile
A goes away?

Tom

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

[Jan Kratochvil]

On Fri, 26 Jun 2009 15:23:00 +0200, Ulrich Weigand wrote:
> - 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 ...

This IMO means "any two types somehow interconnected (by TYPE_TARGET_TYPE,
TYPE_INDEX_TYPE etc.) should have the same TYPE_OBJFILE".


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

I see now dynamics_objfile should be replaced by NULL TYPE_OBJFILE and being
a discardable type to be processed by the types garbage collector (which is
still not checked-in).


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

I do not think it can happen.  So far always type A referencing -> type
B either had the same TYPE_OBJFILE or TYPE_OBJFILE(B) was NULL.  Using such
assertion-checks without facing problems.

As there cannot be a DWARF reference across objfiles (there can be one just
across CUs) cross-file type references use TYPE_IS_OPAQUE and they get
resolved each time on dereferencing such reference by calling check_typedef.


> Therefore, I'd prefer to fix the two problems mentioned above, and then
> revert your patch.

If the current rule:
# So far always type A referencing -> type B had either the same TYPE_OBJFILE
# or TYPE_OBJFILE(B) was NULL.

will be changed to:
# Type A referencing -> type B had the same TYPE_OBJFILE.

I only welcome it and sure I agree with reverting my patch afterwards.


Thanks,
Jan

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

[Ulrich Weigand]

Tom Tromey wrote:
> >>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:
> 
> Ulrich> - I think the symbol readers should always return per-objfile
> Ulrich> types.  This just makes everything simpler, in particular
> Ulrich> allocation of derived types (as you notice).  I think it would
> Ulrich> also good to be able to guarantee that TYPE_OBJFILE of every
> Ulrich> symbol type is non-NULL ...
> 
> My recollection is that we have a few places where an objfile-attached
> type can be derived from a NULL-objfile type.  Index types at least,
> but ISTR also something from Ada.

Well, for temporary GDB-created index types we typically have
  index_type:  NULL objfile  (usually builtin_type_int32 today)
  range_type:  inherits NULL objfile from index_type
  array_type:  inherits NULL objfile from range_type

Ada sometimes creates "temporary" objfile-attached types, but those
always are based on other objfile-attached types.  (These probably
should really not be objfile-attached in the first place, in particular
once we have type garbage collection in place.)

> I'm fine with changing this and adding an invariant like "a type with
> an objfile may only reference other types from the same objfile".  I
> think I must have just assumed that this would be too hard.

Yes, this is the invariant I'd like to impose.

> Ulrich> - The Java generated types should *not* go onto the "fake"
> Ulrich> dynamics objfile in the first place.  That objfile is somewhat
> Ulrich> bogus in that it isn't associated with an architecture (thus
> Ulrich> breaking my per-type architecture effort), and it also don't
> Ulrich> help with type lifetime issues as the fake objfile never goes
> Ulrich> away.  I think those types should be allocated with a NULL
> Ulrich> objfile (in the future are per-gdbarch types) instead.
> 
> This phony objfile is also not multi-inferior-safe.
> 
> It seems to me that this java stuff is like a rough prototype of how
> better debugging for JITs might work -- make a dynamic objfile based
> on information from the running inferior.  So, I suppose it would be
> nice to fix it up rather than nuke it :)
> 
> For the lifetime issue, it seems like this objfile should be treated
> however a .so objfile is treated.

Well, yes, but we know when an .so is removed.  How should we know when
the "JIT objfile" can be removed?

> Ulrich> Also, I think the implementation is broken in the "type
> Ulrich> smashing" case: if there is an incoming type allocated in
> Ulrich> objfile A, but the argument to make_function_type specifies
> Ulrich> objfile B, the type gets "smashed" and reused, and its
> Ulrich> TYPE_OBJFILE gets redirected to objfile B even though the type
> Ulrich> still resides within objfile A's obstack ...
> 
> Is this really valid?  It seems to me that it ought to be an internal
> error.  Otherwise aren't we at risk for memory corruption, if objfile
> A goes away?

Yes, this would be invalid usage in any case.  I'd be fine with
adding an internal error.

Bye,
Ulrich

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

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

[Ulrich Weigand]

Jan Kratochvil wrote:
> > 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,
> 
> I do not think it can happen.  So far always type A referencing -> type
> B either had the same TYPE_OBJFILE or TYPE_OBJFILE(B) was NULL.  Using such
> assertion-checks without facing problems.
> 
> As there cannot be a DWARF reference across objfiles (there can be one just
> across CUs) cross-file type references use TYPE_IS_OPAQUE and they get
> resolved each time on dereferencing such reference by calling check_typedef.

Yes, there is no current usage of the make_function_type interface that
does this, and it would indeed be broken.  I guess was I was trying to
say is that the new interface that allows to specify an objfile independently
of the target type may appear to invite such incorrect usage ...

But in either case, I agree this should be prevented by assertion checks.

> > Therefore, I'd prefer to fix the two problems mentioned above, and then
> > revert your patch.
> 
> If the current rule:
> # So far always type A referencing -> type B had either the same TYPE_OBJFILE
> # or TYPE_OBJFILE(B) was NULL.
> 
> will be changed to:
> # Type A referencing -> type B had the same TYPE_OBJFILE.

Mostly.  We still can have the case were TYPE_OBJFILE(A) is NULL but
TYPE_OBJFILE(B) is non-NULL, for example where A is a temporary array
and B the element type.

The way I see the invariant is
  If Type A references Type B *and* A is objfile-associated
  *then* B must be associated to the same objfile

> I only welcome it and sure I agree with reverting my patch afterwards.

Thanks,
Ulrich

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

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

[Tom Tromey]

>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:

Ulrich> Mostly.  We still can have the case were TYPE_OBJFILE(A) is NULL but
Ulrich> TYPE_OBJFILE(B) is non-NULL, for example where A is a temporary array
Ulrich> and B the element type.

Ulrich> The way I see the invariant is
Ulrich>   If Type A references Type B *and* A is objfile-associated
Ulrich>   *then* B must be associated to the same objfile

Suppose we have A as an array type with a NULL objfile, and with
element type B, which has an objfile.

If we have a convenience variable of type A, and the objfile is
deleted, I don't think preserve_values will copy type B.  That is
because we only check the value's immediate objfile to decide whether
we need to do a type copy.

Therefore it seems to me that the invariant must be TYPE_OBJFILE(A) ==
TYPE_OBJFILE(B), for all related types A and B.

Tom

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

[Ulrich Weigand]

Tom Tromey wrote:
> >>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:
> 
> Ulrich> Mostly.  We still can have the case were TYPE_OBJFILE(A) is NULL but
> Ulrich> TYPE_OBJFILE(B) is non-NULL, for example where A is a temporary array
> Ulrich> and B the element type.
> 
> Ulrich> The way I see the invariant is
> Ulrich>   If Type A references Type B *and* A is objfile-associated
> Ulrich>   *then* B must be associated to the same objfile
> 
> Suppose we have A as an array type with a NULL objfile, and with
> element type B, which has an objfile.
> 
> If we have a convenience variable of type A, and the objfile is
> deleted, I don't think preserve_values will copy type B.  That is
> because we only check the value's immediate objfile to decide whether
> we need to do a type copy.

Hmm, good point.

> Therefore it seems to me that the invariant must be TYPE_OBJFILE(A) ==
> TYPE_OBJFILE(B), for all related types A and B.

I'm not sure how to best implement this invariant -- we *need* to support
the case of a temporary array of pre-existing element types (e.g. for
value_slice etc.) ...

We don't want to allocate the temporary array types per-objfile, because
then they will not be subject to garbage collection (and if the user does
something that causes a lot of those arrays to be generated before the
objfile is unloaded, this could cause a significant leak).

I guess we could force the element type to be copied at the time the
array type is allocated.  In the alternative, we could change 
preserve_values to do the recursive check even if the main type is
not objfile-associated.

Bye,
Ulrich

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

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

[Tom Tromey]

>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:

Ulrich> I guess we could force the element type to be copied at the time the
Ulrich> array type is allocated.  In the alternative, we could change 
Ulrich> preserve_values to do the recursive check even if the main type is
Ulrich> not objfile-associated.

I think either of these choices would be ok.

Tom

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

[Ulrich Weigand]

> On Fri, 26 Jun 2009 15:23:00 +0200, Ulrich Weigand wrote:
> > - 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.
> 
> > Therefore, I'd prefer to fix the two problems mentioned above, and then
> > revert your patch.
> 
> If the current rule:
> # So far always type A referencing -> type B had either the same TYPE_OBJFILE
> # or TYPE_OBJFILE(B) was NULL.
> 
> will be changed to:
> # Type A referencing -> type B had the same TYPE_OBJFILE.
> 
> I only welcome it and sure I agree with reverting my patch afterwards.

I've now checked the two patches in.

Bye,
Ulrich

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