diff mbox

Refine lto-symtab warnings

Message ID 20150610032625.GA16800@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka June 10, 2015, 3:26 a.m. UTC
Hi,
this patch makes warn_type_compatibility_p to use TYPE_CANONICAL rather than
types_compatible_p to decide about warnings on type mismatches.
types_compatible_p is way too precise. It insist on oprations among the types
to be compatible. This is not necessary here as accesses are wrapped in MEM_REF
that does the proper cast - we only want types to be compatible by their
representaiton and by TBAA rules.

Comparing TYPE_CANONICAL is still not completely fitting the bill, as it won't
work for incomplete types (we can hit these in array declarations and return
values of structures) and if we get TYPE_CANONICAl for pointers to not be fully
globbed (I hope so), we will need to add exception these same way as get_alias_set
does some exception.

An option would be to simply compare alias set values. This however is bit too
coarse as it won't get size mismatches nor complete/incomplete type
incompatibilities, so I think it makes sense to simply build from
TYPE_CANONICAL and implement whatever globbing is needed here.  Even if we may
need to implement similar globbing in get_alias_set and here, it is not
necessarily the same: the definition of interoperability is not precisely
following TBAA compatibility.

Bootstrapped/regtested ppc64le-linux, OK?  The testcase here is based on mixing
signedness between C and Fortran units. It won't work unless the earlier patch is
applied. But even if we get more discussion in the TYPE_UNSIGNED globbing, I think
this patch makes sense by itself, possibly w/o a testcase.

Honza

	* lto-symtab.c (warn_type_compatibility_p): Use TYPE_CANONICAL
	to compare memory representation equality.

	* gfortran.dg/lto/bind_c-6_0.f90: New testcase.
	* gfortran.dg/lto/bind_c-6_1.c: New testcase.
diff mbox

Patch

Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c	(revision 224250)
+++ lto/lto-symtab.c	(working copy)
@@ -199,13 +199,19 @@ 
 
 /* Return non-zero if we want to output waring about T1 and T2.
    Return value is a bitmask of reasons of violation:
-   Bit 0 indicates that types are not compatible of memory layout.
-   Bot 1 indicates that types are not compatible because of C++ ODR rule.  */
+   Bit 0 indicates that types are not compatible in memory representation
+   or by TBAA.
+   Bit 1 indicates that types are not compatible because of C++ ODR rule.
 
+   We may have false positives for bit 0.  We may also output more strict
+   warnings in mismatches between types originating from same compilation
+   units.  For example, we may spot qualification differences for C.  */
+
 static int
 warn_type_compatibility_p (tree prevailing_type, tree type)
 {
   int lev = 0;
+
   /* C++ provide a robust way to check for type compatibility via the ODR
      rule.  */
   if (odr_or_derived_type_p (prevailing_type) && odr_or_derived_type_p (type)
@@ -245,54 +251,48 @@ 
      incomplete pointed-to types more aggressively here, ignoring
      mismatches in both field and tag names.  It's difficult though
      to guarantee that this does not have side-effects on merging
-     more compatible types from other translation units though.  */
+     more compatible types from other translation units though.
+     For C programs doing so is however valid by WG14
+     as response to DR#314  */
 
-  /* We can tolerate differences in type qualification, the
-     qualification of the prevailing definition will prevail.
-     ???  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 (prevailing_type);
   type = TYPE_MAIN_VARIANT (type);
 
-  if (!types_compatible_p (prevailing_type, type))
+  /* Check type compatibility by the canonical type machinery.  This only works
+     for complete types.  */
+  if (COMPLETE_TYPE_P (type) && COMPLETE_TYPE_P (prevailing_type))
     {
-      if (TREE_CODE (prevailing_type) == FUNCTION_TYPE
-	  || TREE_CODE (type) == METHOD_TYPE)
+      /* For mixed C/non-C programs we only can warn if the types are incompatible
+	 in their representation.
+	 For example it is possible to declare the same variable as C_PTR in
+	 Fortran unit and as float * in C unit, because C_PTR (represented as
+	 void *) is defined to be interoperable.
+	 It is however definitely possible to check more here than what we
+	 do currently.  */
+      gcc_assert (TYPE_CANONICAL (prevailing_type) && TYPE_CANONICAL (type));
+      if (TYPE_CANONICAL (prevailing_type) != TYPE_CANONICAL (type))
 	return 1 | lev;
-      if (COMPLETE_TYPE_P (type) && COMPLETE_TYPE_P (prevailing_type))
-	return 1 | lev;
+      return lev;
+    }
 
-      /* If type is incomplete then avoid warnings in the cases
-	 that TBAA handles just fine.  */
+  /* Incomplete structure is compatible with every other structure.
+     Similarly for unions.  For arrays we can do better by recursing
+     on array type and checking the same flags as canonical type machinery
+     does.  */
 
-      if (TREE_CODE (prevailing_type) != TREE_CODE (type))
-	return 1 | lev;
+  if (tree_code_for_canonical_type_merging (TREE_CODE (prevailing_type))
+      != tree_code_for_canonical_type_merging (TREE_CODE (type)))
+    return 1 | lev;
 
-      if (TREE_CODE (prevailing_type) == ARRAY_TYPE)
-	{
-	  tree tem1 = TREE_TYPE (prevailing_type);
-	  tree tem2 = TREE_TYPE (type);
-	  while (TREE_CODE (tem1) == ARRAY_TYPE
-		 && TREE_CODE (tem2) == ARRAY_TYPE)
-	    {
-	      tem1 = TREE_TYPE (tem1);
-	      tem2 = TREE_TYPE (tem2);
-	    }
-
-	  if (TREE_CODE (tem1) != TREE_CODE (tem2))
-	    return 1 | lev;
-
-	  if (!types_compatible_p (tem1, tem2))
-	    return 1 | lev;
-	}
-
-      /* Fallthru.  Compatible enough.  */
+  if (TREE_CODE (prevailing_type) == ARRAY_TYPE)
+    {
+      if (TYPE_STRING_FLAG (prevailing_type) != TYPE_STRING_FLAG (type)
+	  || TYPE_NONALIASED_COMPONENT (prevailing_type)
+	     != TYPE_NONALIASED_COMPONENT (type))
+      return warn_type_compatibility_p (TREE_TYPE (prevailing_type),
+				        TREE_TYPE (type)) | lev;
     }
 
-  /* ???  We might want to emit a warning here if type qualification
-     differences were spotted.  Do not do this unconditionally though.  */
-
   return lev;
 }
 
@@ -325,6 +325,10 @@ 
 
       return true;
     }
+  else if (DECL_SIZE (decl) && DECL_SIZE (prevailing_decl)
+	   && !operand_equal_p (DECL_SIZE (decl),
+			        DECL_SIZE (prevailing_decl), 0))
+    return false;
 
   if (warn_type_compatibility_p (TREE_TYPE (prevailing_decl),
 				 TREE_TYPE (decl)))
Index: testsuite/gfortran.dg/lto/bind_c-6_0.f90
===================================================================
--- testsuite/gfortran.dg/lto/bind_c-6_0.f90	(revision 0)
+++ testsuite/gfortran.dg/lto/bind_c-6_0.f90	(working copy)
@@ -0,0 +1,17 @@ 
+! { dg-lto-do run }
+! { dg-lto-options {{ -O3 -flto }} }
+! This testcase will abort if C_FUNPTR is not interoperable with both int *
+! and float *
+module lto_type_merge_test
+  use, intrinsic :: iso_c_binding
+  implicit none
+
+  integer(c_size_t), bind(c, name="myVar") :: myVar
+  integer(c_size_t), bind(c, name="myVar2") :: myVar2
+
+contains
+  subroutine types_test() bind(c)
+    myVar = myVar2
+  end subroutine types_test
+end module lto_type_merge_test
+
Index: testsuite/gfortran.dg/lto/bind_c-6_1.c
===================================================================
--- testsuite/gfortran.dg/lto/bind_c-6_1.c	(revision 0)
+++ testsuite/gfortran.dg/lto/bind_c-6_1.c	(working copy)
@@ -0,0 +1,29 @@ 
+#include <stdlib.h>
+/* declared in the fortran module */
+extern size_t myVar, myVar2;
+void types_test(void);
+
+
+extern void abort(void);
+
+int main(int argc, char **argv)
+{
+   size_t *myptr, *myptr2;
+   asm("":"=r"(myptr):"0"(&myVar));
+   asm("":"=r"(myptr2):"0"(&myVar2));
+   *myptr = 1;
+   *myptr2 = 2;
+   types_test();
+   if (*myptr != 2)
+	abort ();
+   if (*myptr2 != 2)
+	abort ();
+   *myptr2 = 3;
+   types_test();
+   if (*myptr != 3)
+	abort ();
+   if (*myptr2 != 3)
+	abort ();
+   return 0;
+}
+