Improve LTO type checking during symtab merging

[Jan Hubicka <hubicka@ucw.cz>]

Hi,
this patch strengten ODR checking for requiring match on declarations
(if both of them are defined in C++ tranlsation unit).  Currently
ipa-symtab.c will warn only if declarations are incompatible in 
useless_type_conversion sense.  Now we warn more often, i.e. in the following
example:

t.C:
char a;

t2.C:
#include <stdtype.h>
extern int8_t a;
void *b=&a;

Will lead to warning:
t2.C:2:15: warning: type of �a� does not match original declaration
 extern int8_t a;
               ^
<built-in>: note: type name �char� should match type name �signed char�
/usr/include/stdint.h:37:22: note: the incompatible type is defined here
 typedef signed char  int8_t;
                      ^
/aux/hubicka/t.C:1:6: note: previously declared here
 char a;
      ^

Funnilly enough we do not get warning when t2.C is just "signed char"
or "unsigned char" because that is builtin type and thus non-ODR. Something
I need to deal with incrementally.  

I also added call to warn_types_mismatch that outputs extra note about
what is wrong with the type: in C++ the mismatch is often carried over
several levels on declarations and it is hard to work out the reason
without extra info.

Richard, According to comments, it seems that the code was tailored to not
output warning when we can avoid wrong code issues on mismatches based on fact
that linker would be also silent. I am particularly entertained by the
following hunk which I killed:

-      if (!types_compatible_p (TREE_TYPE (prevailing_decl),
-			       TREE_TYPE (decl)))
-	/* If we don't have a merged type yet...sigh.  The linker
-	   wouldn't complain if the types were mismatched, so we
-	   probably shouldn't either.  Just use the type from
-	   whichever decl appears to be associated with the
-	   definition.  If for some odd reason neither decl is, the
-	   older one wins.  */
-	(void) 0;

It is fun we go through the trouble of comparing types and then do nothing ;)
Type merging is also completed at this time, so at least the comment looks
out of date.

I think as QOI issue, we do want to warn in cases the code is inconsistent (it
is pretty much surely a bug), but perhaps we may want to have the warnings with
-Wodr only and use slightly different warning and different -W switch for
warnings that unavoidably leads to wrong code surprises?   This should be easy
to implement by adding two levels into new warn_type_compatibility_p predicate.
I am personaly also OK however with warning always or making all the warnings
-Wodr.

What do you think?

Incrementally I am heading towards proper definition of decl compatibility that
I can plug into symtab merging and avoid merging incompatible decls (so
FORTIFY_SOURCE works).

lto-symtab and ipa-icf both have some knowledge of the problem, I want to get
both right and factor out common logic.

Other improvement is to preserve the ODR type info when non-ODR variant
previals so one can diagnose clash in between C++ units even in mixed
language linktimes.

Bootstrapped/regtested x86_64-linux with no new ODR warnings hit by -Werror.

Honza

	* ipa-devirt.c (register_odr_type): Be ready for non-odr main variant
	of odr type.
	* ipa-utils.h (warn_types_mismatch): New.
	* lto/lto-symtab.c (warn_type_compatibility_p): Break out from ...
	(lto_symtab_merge): ... here.
	(lto_symtab_merge_decls_2): Improve warnings.

Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 222391)
+++ ipa-devirt.c	(working copy)
@@ -2029,10 +2030,14 @@ register_odr_type (tree type)
       if (in_lto_p)
         odr_vtable_hash = new odr_vtable_hash_type (23);
     }
-  /* Arrange things to be nicer and insert main variants first.  */
-  if (odr_type_p (TYPE_MAIN_VARIANT (type)))
+  /* Arrange things to be nicer and insert main variants first.
+     ??? fundamental prerecorded types do not have mangled names; this
+     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))))
     get_odr_type (TYPE_MAIN_VARIANT (type), true);
-  if (TYPE_MAIN_VARIANT (type) != type)
+  if (TYPE_MAIN_VARIANT (type) != type && odr_type_p (main_odr_variant (type)))
     get_odr_type (type, true);
 }
 
Index: ipa-utils.h
===================================================================
--- ipa-utils.h	(revision 222391)
+++ ipa-utils.h	(working copy)
@@ -84,6 +84,7 @@ bool types_must_be_same_for_odr (tree, t
 bool types_odr_comparable (tree, tree, bool strict = false);
 cgraph_node *try_speculative_devirtualization (tree, HOST_WIDE_INT,
 					       ipa_polymorphic_call_context);
+void warn_types_mismatch (tree t1, tree t2);
 
 /* Return vector containing possible targets of polymorphic call E.
    If COMPLETEP is non-NULL, store true if the list is complete. 
Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c	(revision 222391)
+++ lto/lto-symtab.c	(working copy)
@@ -203,45 +203,16 @@ lto_varpool_replace_node (varpool_node *
   vnode->remove ();
 }
 
-/* Merge two variable or function symbol table entries PREVAILING and ENTRY.
-   Return false if the symbols are not fully compatible and a diagnostic
-   should be emitted.  */
+/* Return true if we want to output waring about T1 and T2.  */
 
 static bool
-lto_symtab_merge (symtab_node *prevailing, symtab_node *entry)
+warn_type_compatibility_p (tree prevailing_type, tree type)
 {
-  tree prevailing_decl = prevailing->decl;
-  tree decl = entry->decl;
-  tree prevailing_type, type;
-
-  if (prevailing_decl == decl)
+  /* C++ provide robust way to check for type compatibility via the ODR
+     rule.  */
+  if (odr_type_p (prevailing_type) && odr_type_p (type)
+      && !types_same_for_odr (prevailing_type, type, true))
     return true;
-
-  /* Merge decl state in both directions, we may still end up using
-     the new decl.  */
-  TREE_ADDRESSABLE (prevailing_decl) |= TREE_ADDRESSABLE (decl);
-  TREE_ADDRESSABLE (decl) |= TREE_ADDRESSABLE (prevailing_decl);
-
-  /* The linker may ask us to combine two incompatible symbols.
-     Detect this case and notify the caller of required diagnostics.  */
-
-  if (TREE_CODE (decl) == FUNCTION_DECL)
-    {
-      if (!types_compatible_p (TREE_TYPE (prevailing_decl),
-			       TREE_TYPE (decl)))
-	/* If we don't have a merged type yet...sigh.  The linker
-	   wouldn't complain if the types were mismatched, so we
-	   probably shouldn't either.  Just use the type from
-	   whichever decl appears to be associated with the
-	   definition.  If for some odd reason neither decl is, the
-	   older one wins.  */
-	(void) 0;
-
-      return true;
-    }
-
-  /* Now we exclusively deal with VAR_DECLs.  */
-
   /* Sharing a global symbol is a strong hint that two types are
      compatible.  We could use this information to complete
      incomplete pointed-to types more aggressively here, ignoring
@@ -254,19 +225,22 @@ lto_symtab_merge (symtab_node *prevailin
      ???  In principle we might want to only warn for structurally
      incompatible types here, but unless we have protective measures
      for TBAA in place that would hide useful information.  */
-  prevailing_type = TYPE_MAIN_VARIANT (TREE_TYPE (prevailing_decl));
-  type = TYPE_MAIN_VARIANT (TREE_TYPE (decl));
+  prevailing_type = TYPE_MAIN_VARIANT (prevailing_type);
+  type = TYPE_MAIN_VARIANT (type);
 
   if (!types_compatible_p (prevailing_type, type))
     {
+      if (TREE_CODE (prevailing_type) == FUNCTION_TYPE
+	  || TREE_CODE (type) == METHOD_TYPE)
+	return true;
       if (COMPLETE_TYPE_P (type))
-	return false;
+	return true;
 
       /* If type is incomplete then avoid warnings in the cases
 	 that TBAA handles just fine.  */
 
       if (TREE_CODE (prevailing_type) != TREE_CODE (type))
-	return false;
+	return true;
 
       if (TREE_CODE (prevailing_type) == ARRAY_TYPE)
 	{
@@ -280,10 +254,10 @@ lto_symtab_merge (symtab_node *prevailin
 	    }
 
 	  if (TREE_CODE (tem1) != TREE_CODE (tem2))
-	    return false;
+	    return true;
 
 	  if (!types_compatible_p (tem1, tem2))
-	    return false;
+	    return true;
 	}
 
       /* Fallthru.  Compatible enough.  */
@@ -292,6 +266,43 @@ lto_symtab_merge (symtab_node *prevailin
   /* ???  We might want to emit a warning here if type qualification
      differences were spotted.  Do not do this unconditionally though.  */
 
+  return false;
+}
+
+/* Merge two variable or function symbol table entries PREVAILING and ENTRY.
+   Return false if the symbols are not fully compatible and a diagnostic
+   should be emitted.  */
+
+static bool
+lto_symtab_merge (symtab_node *prevailing, symtab_node *entry)
+{
+  tree prevailing_decl = prevailing->decl;
+  tree decl = entry->decl;
+
+  if (prevailing_decl == decl)
+    return true;
+
+  /* Merge decl state in both directions, we may still end up using
+     the new decl.  */
+  TREE_ADDRESSABLE (prevailing_decl) |= TREE_ADDRESSABLE (decl);
+  TREE_ADDRESSABLE (decl) |= TREE_ADDRESSABLE (prevailing_decl);
+
+  /* The linker may ask us to combine two incompatible symbols.
+     Detect this case and notify the caller of required diagnostics.  */
+
+  if (TREE_CODE (decl) == FUNCTION_DECL)
+    {
+      if (warn_type_compatibility_p (TREE_TYPE (prevailing_decl),
+			             TREE_TYPE (decl)))
+	return false;
+
+      return true;
+    }
+
+  if (warn_type_compatibility_p (TREE_TYPE (prevailing_decl),
+				 TREE_TYPE (decl)))
+    return false;
+
   /* There is no point in comparing too many details of the decls here.
      The type compatibility checks or the completing of types has properly
      dealt with most issues.  */
@@ -483,12 +494,18 @@ lto_symtab_merge_decls_2 (symtab_node *f
   /* Diagnose all mismatched re-declarations.  */
   FOR_EACH_VEC_ELT (mismatches, i, decl)
     {
-      if (!types_compatible_p (TREE_TYPE (prevailing->decl),
-			       TREE_TYPE (decl)))
-	diagnosed_p |= warning_at (DECL_SOURCE_LOCATION (decl), 0,
-				   "type of %qD does not match original "
-				   "declaration", decl);
-
+      if (warn_type_compatibility_p (TREE_TYPE (prevailing->decl),
+				     TREE_TYPE (decl)))
+	{
+	  bool diag;
+	  diag = warning_at (DECL_SOURCE_LOCATION (decl), 0,
+			     "type of %qD does not match original "
+			     "declaration", decl);
+	  diagnosed_p |= diag;
+	  if (diag)
+	    warn_types_mismatch (TREE_TYPE (prevailing->decl),
+				 TREE_TYPE (decl));
+	}
       else if ((DECL_USER_ALIGN (prevailing->decl)
 	        && DECL_USER_ALIGN (decl))
 	       && DECL_ALIGN (prevailing->decl) < DECL_ALIGN (decl))