This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix verify_type ICE on TYPE_METHOS


Jason,
this patch is trying to fix the following verify_type_ice that check
that TYPE_METHODS is same for main variant and all its complete variants.

-  /* FIXME: this check triggers during libstdc++ build that is a bug.
-     It affects non-LTO debug output only, because free_lang_data clears
-     this anyway.  */
-  if (RECORD_OR_UNION_TYPE_P (t) && COMPLETE_TYPE_P (t) && 0
-      && TYPE_METHODS (t) != TYPE_METHODS (tv))

I tried to go in a way of making TYPE_METHODS to be defined only on main vairants
as it seems impractical to update variants each time we change TYPE_METHODS.
While doing so, I noticed that lazily_declare_fn sometimes adds new method
to variant only (that is probably source of ICE as fixup_type_variants copies the
TYPE_METHODS).

The attach patch however removes the copying and verifies that TYPE_MEHTODS is
NULL at all variants. It also updates middle-end uses to always check for main variant.
Bootstrapped/regtested ppc64-linux, does this seem to make sense?

I also noticed that TYPE_METHODS != NULL matters in some cases: it makes function.c
to drop register keyword and it makes us to speak of class instead of struct in
diagnostics, so I changed free_lang_data to zap the list to error_mark_node instead
of NULL (in past I tried to stream it for ODR warnings and I remember it added
measurable extra overhead that I considered too large for mainline).

Honza

	* class.c (fixup_type_variants): Do not copy TYPE_METHODS
	(one_inheriting_sig): Assert tat we always set TYPE_METHODS of main variant.
	* semantics.c (finish_member_declaration): Likewise.
	* method.c (lazily_declare_fn): Allways add method to main variant list.

	* dwarf2out.c (gen_member_die): Sanity check that we access TYPE_MAIN_VARIANT
	for TYPE_METHODS.
	* function.c (use_register_for_decl): Look for TYPE_MAIN_VARIANT when checking
	TYPE_METHODS.
	* tree.c (free_lang_data_in_type): See TYPE_METHODS to error_mark_node
	if non-null.
	(build_distinct_type_copy): Clear TYPE_METHODS.
	(verify_type_variant): Verify that TYPE_METHODS is NULL for variants.
	(verify_type): Allow TYPE_METHODS to be error_mark_node.
	* tree.def: Update docs of YTPE_STUB_DECL and TYPE_METHODS.
Index: cp/class.c
===================================================================
--- cp/class.c	(revision 222991)
+++ cp/class.c	(working copy)
@@ -1972,7 +1972,6 @@ fixup_type_variants (tree t)
 
       /* Copy whatever these are holding today.  */
       TYPE_VFIELD (variants) = TYPE_VFIELD (t);
-      TYPE_METHODS (variants) = TYPE_METHODS (t);
       TYPE_FIELDS (variants) = TYPE_FIELDS (t);
     }
 }
@@ -3238,6 +3237,7 @@ one_inheriting_sig (tree t, tree ctor, t
     parmlist = tree_cons (NULL_TREE, parms[i], parmlist);
   tree fn = implicitly_declare_fn (sfk_inheriting_constructor,
 				   t, false, ctor, parmlist);
+  gcc_assert (TYPE_MAIN_VARIANT (t) == t);
   if (add_method (t, fn, NULL_TREE))
     {
       DECL_CHAIN (fn) = TYPE_METHODS (t);
Index: cp/method.c
===================================================================
--- cp/method.c	(revision 222991)
+++ cp/method.c	(working copy)
@@ -2189,11 +2189,11 @@ lazily_declare_fn (special_function_kind
       && DECL_VIRTUAL_P (fn))
     /* The ABI requires that a virtual destructor go at the end of the
        vtable.  */
-    TYPE_METHODS (type) = chainon (TYPE_METHODS (type), fn);
+    TYPE_METHODS (type) = chainon (TYPE_METHODS (TYPE_MAIN_VARIANT (type)), fn);
   else
     {
-      DECL_CHAIN (fn) = TYPE_METHODS (type);
-      TYPE_METHODS (type) = fn;
+      DECL_CHAIN (fn) = TYPE_METHODS (TYPE_MAIN_VARIANT (type));
+      TYPE_METHODS (TYPE_MAIN_VARIANT (type)) = fn;
     }
   maybe_add_class_template_decl_list (type, fn, /*friend_p=*/0);
   if (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (fn)
Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 222991)
+++ cp/semantics.c	(working copy)
@@ -2913,6 +2913,7 @@ finish_member_declaration (tree decl)
 	 CLASSTYPE_METHOD_VEC.  */
       if (add_method (current_class_type, decl, NULL_TREE))
 	{
+	  gcc_assert (TYPE_MAIN_VARIANT (current_class_type) == current_class_type);
 	  DECL_CHAIN (decl) = TYPE_METHODS (current_class_type);
 	  TYPE_METHODS (current_class_type) = decl;
 
Index: dwarf2out.c
===================================================================
--- dwarf2out.c	(revision 222991)
+++ dwarf2out.c	(working copy)
@@ -19945,6 +19945,8 @@ gen_member_die (tree type, dw_die_ref co
 	gen_decl_die (member, NULL, context_die);
     }
 
+  /* We do not keep type methods in type variants.  */
+  gcc_assert (TYPE_MAIN_VARIANT (type) == type);
   /* Now output info about the function members (if any).  */
   for (member = TYPE_METHODS (type); member; member = DECL_CHAIN (member))
     {
Index: function.c
===================================================================
--- function.c	(revision 222991)
+++ function.c	(working copy)
@@ -2170,7 +2170,7 @@ use_register_for_decl (const_tree decl)
       /* When not optimizing, disregard register keyword for variables with
 	 types containing methods, otherwise the methods won't be callable
 	 from the debugger.  */
-      if (TYPE_METHODS (TREE_TYPE (decl)))
+      if (TYPE_METHODS (TYPE_MAIN_VARIANT (TREE_TYPE (decl))))
 	return false;
       break;
     default:
Index: tree.c
===================================================================
--- tree.c	(revision 222991)
+++ tree.c	(working copy)
@@ -5100,7 +5100,13 @@ free_lang_data_in_type (tree type)
       if (TYPE_VFIELD (type) && TREE_CODE (TYPE_VFIELD (type)) != FIELD_DECL)
         TYPE_VFIELD (type) = NULL_TREE;
 
-      TYPE_METHODS (type) = NULL_TREE;
+      /* Remove TYPE_METHODS list.  While it would be nice to keep it
+ 	 to enable ODR warnings about different method lists, doing so
+	 seems to impractically increase size of LTO data streamed.
+	 Keep the information if TYPE_METHODS was non-NULL. This is used
+	 by function.c and pretty printers.  */
+      if (TYPE_METHODS (type))
+        TYPE_METHODS (type) = error_mark_node;
       if (TYPE_BINFO (type))
 	{
 	  free_lang_data_in_binfo (TYPE_BINFO (type));
@@ -6574,6 +6580,12 @@ build_distinct_type_copy (tree type)
   TYPE_MAIN_VARIANT (t) = t;
   TYPE_NEXT_VARIANT (t) = 0;
 
+  /* We do not record methods in type copies nor variants
+     so we do not need to keep them up to date when new method
+     is inserted.  */
+  if (RECORD_OR_UNION_TYPE_P (t))
+    TYPE_METHODS (t) = NULL_TREE;
+
   /* Note that it is now possible for TYPE_MIN_VALUE to be a value
      whose TREE_TYPE is not t.  This can also happen in the Ada
      frontend when using subtypes.  */
@@ -12528,13 +12540,9 @@ verify_type_variant (const_tree t, tree
       debug_tree (tv);
       return false;
     }
-  /* FIXME: this check triggers during libstdc++ build that is a bug.
-     It affects non-LTO debug output only, because free_lang_data clears
-     this anyway.  */
-  if (RECORD_OR_UNION_TYPE_P (t) && COMPLETE_TYPE_P (t) && 0
-      && TYPE_METHODS (t) != TYPE_METHODS (tv))
+  if (RECORD_OR_UNION_TYPE_P (t) && TYPE_METHODS (t))
     {
-      error ("type variant has different TYPE_METHODS");
+      error ("type variant has TYPE_METHODS");
       debug_tree (tv);
       return false;
     }
@@ -12749,9 +12757,10 @@ verify_type (const_tree t)
   if (RECORD_OR_UNION_TYPE_P (t))
     {
       if (TYPE_METHODS (t) && TREE_CODE (TYPE_METHODS (t)) != FUNCTION_DECL
-	  && TREE_CODE (TYPE_METHODS (t)) != TEMPLATE_DECL)
+	  && TREE_CODE (TYPE_METHODS (t)) != TEMPLATE_DECL
+	  && TYPE_METHODS (t) != error_mark_node)
 	{
-	  error ("TYPE_METHODS is not FUNCTION_DECL nor TEMPLATE_DECL");
+	  error ("TYPE_METHODS is not FUNCTION_DECL, TEMPLATE_DECL nor error_mark_node");
 	  debug_tree (TYPE_METHODS (t));
 	  error_found = true;
 	}
Index: tree.def
===================================================================
--- tree.def	(revision 222991)
+++ tree.def	(working copy)
@@ -110,9 +110,12 @@ DEFTREECODE (BLOCK, "block", tcc_excepti
     particular, since any type which is of some type category  (e.g.
     an array type or a function type) which cannot either have a name
     itself or have named members doesn't really have a "scope" per se.
-  The TREE_CHAIN field is used as a forward-references to names for
+  The TYPE_STUB_DECL field is used as a forward-references to names for
     ENUMERAL_TYPE, RECORD_TYPE, UNION_TYPE, and QUAL_UNION_TYPE nodes;
-    see below.  */
+    see below.
+  The TYPE_METHODS points to list of all methods associated with the type.
+    It is non-NULL only at main variant of the type and after free_lang_data
+    it may be set to error_mark_node instead of actual list to save memory. */
 
 /* The ordering of the following codes is optimized for the checking
    macros in tree.h.  Changing the order will degrade the speed of the


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]