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]

ODR merging and implicit typedefs


Jason,
I just noticed that there are bogus ODR violation warnings during LTO-bootstrap
(that breaks -Werror builds).  It was caused by my work-around for type_in_anonymous_namespace
for the issue discussed in:
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01245.html
(i.e. the TYPE_STUB_DECL disucssion).

I simply added a loop that for type that looks anonymous by
if (TYPE_STUB_DECL (t) && !TREE_PUBLIC (TYPE_STUB_DECL (t)))
it walks up the context and looks for actual anonymous NAMESPACE_DECL.

Now I however run into type merging issues as follows:

for following type:
../../gcc/postreload-gcse.c:107:1: error: type ïstruct <anon>ï violates one definition rule [-Werror=odr]
 {
  ^
../../gcc/postreload.c:721:3: note: a different type is defined in another translation unit
   {
    ^
../../gcc/postreload-gcse.c:108:7: note: the first difference of corresponding definitions is field ïmoves_insertedï
   int moves_inserted;
        ^
../../gcc/postreload.c:722:51: note: a field with different name is defined in another translation unit
     struct reg_use reg_use[RELOAD_COMBINE_MAX_USES];
                                                    ^
(gdb) p debug_tree (t1->type_common.name->decl_with_vis.assembler_name)
 <identifier_node 0x7ffff5ecac08 5._134>

So the problem is that at compile time we think the type is ODR type but it does not really have name:

 <record_type 0x7ffff233e348 BLK
    size <integer_cst 0x7ffff6cce138 type <integer_type 0x7ffff6adb150 bitsizetype> constant 96>
    unit size <integer_cst 0x7ffff6987a08 type <integer_type 0x7ffff6adb0a8 sizetype> constant 12>
    align 32 symtab 0 alias set -1 canonical type 0x7ffff46d2bd0
    fields <field_decl 0x7ffff27178e8 moves_inserted
        type <integer_type 0x7ffff6adb690 int public SI
            size <integer_cst 0x7ffff6ad7df8 constant 32>
            unit size <integer_cst 0x7ffff6ad7e10 constant 4>
            align 32 symtab 0 alias set -1 canonical type 0x7ffff6adb690 precision 32 min <integer_cst 0x7ffff6ad7db0 -2147483648> max <integer_cst 0x7ffff6ad7dc8 2147483647>
            pointer_to_this <pointer_type 0x7ffff6af27e0> reference_to_this <reference_type 0x7ffff6148dc8>>
        nonlocal SI file ../../gcc/postreload-gcse.c line 108 col 7 size <integer_cst 0x7ffff6ad7df8 32> unit size <integer_cst 0x7ffff6ad7e10 4>
        align 32 offset_align 128
        offset <integer_cst 0x7ffff6ad7be8 constant 0>
        bit offset <integer_cst 0x7ffff6ad7c30 constant 0> context <record_type 0x7ffff233e150>
        chain <field_decl 0x7ffff2717850 copies_inserted type <integer_type 0x7ffff6adb690 int>
            nonlocal SI file ../../gcc/postreload-gcse.c line 109 col 7 size <integer_cst 0x7ffff6ad7df8 32> unit size <integer_cst 0x7ffff6ad7e10 4>
            align 32 offset_align 128 offset <integer_cst 0x7ffff6ad7be8 0> bit offset <integer_cst 0x7ffff6ad7df8 32> context <record_type 0x7ffff233e150> chain <field_decl 0x7ffff27177b8 insns_deleted>>> context <translation_unit_decl 0x7ffff25c2f78 D.1019967>
    chain <type_decl 0x7ffff2717688 D.1023523>>

 <type_decl 0x7ffff2717688 D.1023523
    type <record_type 0x7ffff233e150 BLK
        size <integer_cst 0x7ffff6cce138 constant 96>
        unit size <integer_cst 0x7ffff6987a08 constant 12>
        align 32 symtab 0 alias set -1 canonical type 0x7ffff46d2bd0
        fields <field_decl 0x7ffff27178e8 moves_inserted type <integer_type 0x7ffff6adb690 int>
            nonlocal SI file ../../gcc/postreload-gcse.c line 108 col 7
            size <integer_cst 0x7ffff6ad7df8 constant 32>
            unit size <integer_cst 0x7ffff6ad7e10 constant 4>
            align 32 offset_align 128
            offset <integer_cst 0x7ffff6ad7be8 constant 0>
            bit offset <integer_cst 0x7ffff6ad7c30 constant 0> context <record_type 0x7ffff233e150> chain <field_decl 0x7ffff2717850 copies_inserted>> context <translation_unit_decl 0x7ffff25c2f78 D.1019967>
        pointer_to_this <pointer_type 0x7ffff233e3f0> chain <type_decl 0x7ffff2717688 D.1023523>>
    VOID file ../../gcc/postreload-gcse.c line 107 col 1
    align 8 context <translation_unit_decl 0x7ffff25c2f78 D.1019967>>

I tracked down that those are implicit typedef created by create_implicit_typedef.
My patch made them no longer anonymous that in turn triggers the bogus diagnostics.
I do not think it is fully correct though - those types are not anonymous.
(I also wonder we we need to introdce a type name "._134") and pass it all the way down
to LTO.

I tried to make type_with_linkage_p to return false on those, but that causes problem
witht fact that polymorphic call analysis expects all class types to have linkage
and working ODR equivalency on these. 

Is there a way to associate them with the real named type they correspond to and
arrange them to have same name mangling? (and perhaps the mangler to ICE on attempt
to try to use local name like this?)

I bootstrapped/regtested on x86_64-linux the patch bellow. If it will work for Firefox
and Chrome I will go ahead with it at least temporarily.

Honza

	* ipa-devirt.c (type_in_anonymous_namespace_p): Return true
	or implicit declarations.
	(odr_type_p): Check that TYPE_NAME is TYPE_DECL before looking
	into it.
	(get_odr_type): Check type has linkage before adding bases.
	(register_odr_type): Check that type has linkage before adding it.
	(type_known_to_have_no_deriavations_p): Rename to ..
	(type_known_to_have_no_derivations_p): This one.
	* ipa-utils.h (type_known_to_have_no_deriavations_p): Rename to ..
	(type_known_to_have_no_derivations_p): This one.
	* ipa-polymorphic-call.c
	(ipa_polymorphic_call_context::restrict_to_inner_type): Check that
	type has linkage.
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 223390)
+++ ipa-devirt.c	(working copy)
@@ -269,6 +269,8 @@ type_in_anonymous_namespace_p (const_tre
 
   if (TYPE_STUB_DECL (t) && !TREE_PUBLIC (TYPE_STUB_DECL (t)))
     {
+      if (DECL_ARTIFICIAL (TYPE_NAME (t)))
+	return true;
       tree ctx = DECL_CONTEXT (TYPE_NAME (t));
       while (ctx)
 	{
@@ -296,7 +298,7 @@ odr_type_p (const_tree t)
      to care, since it is used only for type merging.  */
   gcc_checking_assert (in_lto_p || flag_lto);
 
-  return (TYPE_NAME (t)
+  return (TYPE_NAME (t) && TREE_CODE (TYPE_NAME (t)) == TYPE_DECL
           && (DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t))));
 }
 
@@ -2124,6 +2126,7 @@ get_odr_type (tree type, bool insert)
     }
 
   if (build_bases && TREE_CODE (type) == RECORD_TYPE && TYPE_BINFO (type)
+      && type_with_linkage_p (type)
       && type == TYPE_MAIN_VARIANT (type))
     {
       tree binfo = TYPE_BINFO (type);
@@ -2183,7 +2186,8 @@ register_odr_type (tree type)
      makes it possible that non-ODR type is main_odr_variant of ODR type.
      Things may get smoother if LTO FE set mangled name of those types same
      way as C++ FE does.  */
-  if (odr_type_p (main_odr_variant (TYPE_MAIN_VARIANT (type))))
+  if (odr_type_p (main_odr_variant (TYPE_MAIN_VARIANT (type)))
+      && odr_type_p (TYPE_MAIN_VARIANT (type)))
     get_odr_type (TYPE_MAIN_VARIANT (type), true);
   if (TYPE_MAIN_VARIANT (type) != type && odr_type_p (main_odr_variant (type)))
     get_odr_type (type, true);
@@ -2192,7 +2196,7 @@ register_odr_type (tree type)
 /* Return true if type is known to have no derivations.  */
 
 bool
-type_known_to_have_no_deriavations_p (tree t)
+type_known_to_have_no_derivations_p (tree t)
 {
   return (type_all_derivations_known_p (t)
 	  && (TYPE_FINAL_P (t)
Index: ipa-utils.h
===================================================================
--- ipa-utils.h	(revision 223390)
+++ ipa-utils.h	(working copy)
@@ -80,7 +80,7 @@ bool vtable_pointer_value_to_vtable (con
 tree subbinfo_with_vtable_at_offset (tree, unsigned HOST_WIDE_INT, tree);
 void compare_virtual_tables (varpool_node *, varpool_node *);
 bool type_all_derivations_known_p (const_tree);
-bool type_known_to_have_no_deriavations_p (tree);
+bool type_known_to_have_no_derivations_p (tree);
 bool contains_polymorphic_type_p (const_tree);
 void register_odr_type (tree);
 bool types_must_be_same_for_odr (tree, tree);
Index: ipa-polymorphic-call.c
===================================================================
--- ipa-polymorphic-call.c	(revision 223390)
+++ ipa-polymorphic-call.c	(working copy)
@@ -269,7 +269,8 @@ ipa_polymorphic_call_context::restrict_t
 		 types.  Testing it here may help us to avoid speculation.  */
 	      if (otr_type && TREE_CODE (outer_type) == RECORD_TYPE
 		  && (!in_lto_p || odr_type_p (outer_type))
-		  && type_known_to_have_no_deriavations_p (outer_type))
+		  && type_with_linkage_p (outer_type)
+		  && type_known_to_have_no_derivations_p (outer_type))
 		maybe_derived_type = false;
 
 	      /* Type can not contain itself on an non-zero offset.  In that case
@@ -393,7 +394,7 @@ ipa_polymorphic_call_context::restrict_t
 	    goto no_useful_type_info;
 
 	  cur_offset = new_offset;
-	  type = subtype;
+	  type = TYPE_MAIN_VARIANT (subtype);
 	  if (!speculative)
 	    {
 	      outer_type = type;


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