Silence false positive on LTO type merging waring

Message ID 20180125210937.GA85544@kam.mff.cuni.cz
State New
Headers show
Series
  • Silence false positive on LTO type merging waring
Related show

Commit Message

Jan Hubicka Jan. 25, 2018, 9:09 p.m.
Hi,
the testcase triggers invalid warning on type mismatch because array
of pointers to complete type has different alias set from array of pointers
to incomplete type.  This is valid, because incoplete pointer has alias set
of void_ptr which alias all pointers and arrays alias with their members.

I already silenced the wanring for pointers but I forgot about arrays.

Bootstrapped/regtested x86_64-linux, OK?
	
	* gcc.dg/lto/pr83954.h: New testcase.
	* gcc.dg/lto/pr83954_0.c: New testcase.
	* gcc.dg/lto/pr83954_1.c: New testcase.
	* lto-symtab.c (warn_type_compatibility_p): Silence false positive
	for type match warning on arrays of pointers.

Comments

Richard Biener Jan. 26, 2018, 11:15 a.m. | #1
On Thu, 25 Jan 2018, Jan Hubicka wrote:

> Hi,

> the testcase triggers invalid warning on type mismatch because array

> of pointers to complete type has different alias set from array of pointers

> to incomplete type.  This is valid, because incoplete pointer has alias set

> of void_ptr which alias all pointers and arrays alias with their members.


But isn't that a problem?  Not if the pointer to incomlete type have
alias set zero but IIRC they don't, right?

Richard.

> I already silenced the wanring for pointers but I forgot about arrays.

> 

> Bootstrapped/regtested x86_64-linux, OK?

> 	

> 	* gcc.dg/lto/pr83954.h: New testcase.

> 	* gcc.dg/lto/pr83954_0.c: New testcase.

> 	* gcc.dg/lto/pr83954_1.c: New testcase.

> 	* lto-symtab.c (warn_type_compatibility_p): Silence false positive

> 	for type match warning on arrays of pointers.

> 

> Index: testsuite/gcc.dg/lto/pr83954.h

> ===================================================================

> --- testsuite/gcc.dg/lto/pr83954.h	(revision 0)

> +++ testsuite/gcc.dg/lto/pr83954.h	(working copy)

> @@ -0,0 +1,3 @@

> +struct foo;

> +extern struct foo *FOO_PTR_ARR[1];

> +

> Index: testsuite/gcc.dg/lto/pr83954_0.c

> ===================================================================

> --- testsuite/gcc.dg/lto/pr83954_0.c	(revision 0)

> +++ testsuite/gcc.dg/lto/pr83954_0.c	(working copy)

> @@ -0,0 +1,8 @@

> +/* { dg-lto-do link } */

> +#include "pr83954.h"

> +

> +int main() { 

> +  // just to prevent symbol removal

> +  FOO_PTR_ARR[1] = 0;

> +  return 0;

> +}

> Index: testsuite/gcc.dg/lto/pr83954_1.c

> ===================================================================

> --- testsuite/gcc.dg/lto/pr83954_1.c	(revision 0)

> +++ testsuite/gcc.dg/lto/pr83954_1.c	(working copy)

> @@ -0,0 +1,7 @@

> +#include "pr83954.h"

> +

> +struct foo {

> + int x;

> +};

> +struct foo *FOO_PTR_ARR[1] = { 0 };

> +

> Index: lto/lto-symtab.c

> ===================================================================

> --- lto/lto-symtab.c	(revision 257009)

> +++ lto/lto-symtab.c	(working copy)

> @@ -284,11 +284,22 @@ warn_type_compatibility_p (tree prevaili

>        alias_set_type set1 = get_alias_set (type);

>        alias_set_type set2 = get_alias_set (prevailing_type);

>  

> -      if (set1 && set2 && set1 != set2 

> -          && (!POINTER_TYPE_P (type) || !POINTER_TYPE_P (prevailing_type)

> +      if (set1 && set2 && set1 != set2)

> +	{

> +          tree t1 = type, t2 = prevailing_type;

> +

> +	  /* Alias sets of arrays are the same as alias sets of the inner

> +	     types.  */

> +	  while (TREE_CODE (t1) == ARRAY_TYPE && TREE_CODE (t2) == ARRAY_TYPE)

> +	    {

> +	      t1 = TREE_TYPE (t1);

> +	      t2 = TREE_TYPE (t2);

> +	    }

> +          if ((!POINTER_TYPE_P (t1) || !POINTER_TYPE_P (t2))

>  	      || (set1 != TYPE_ALIAS_SET (ptr_type_node)

> -		  && set2 != TYPE_ALIAS_SET (ptr_type_node))))

> -        lev |= 5;

> +		  && set2 != TYPE_ALIAS_SET (ptr_type_node)))

> +             lev |= 5;

> +	}

>      }

>  

>    return lev;

> 

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Jan Hubicka Jan. 26, 2018, 1:09 p.m. | #2
> On Thu, 25 Jan 2018, Jan Hubicka wrote:

> 

> > Hi,

> > the testcase triggers invalid warning on type mismatch because array

> > of pointers to complete type has different alias set from array of pointers

> > to incomplete type.  This is valid, because incoplete pointer has alias set

> > of void_ptr which alias all pointers and arrays alias with their members.

> 

> But isn't that a problem?  Not if the pointer to incomlete type have

> alias set zero but IIRC they don't, right?


pointers to incomplete type are same as alias set of ptr_type_node (1 in this case)
and pointer to complete types have different alias set but that set has
is_pointer flag.  When checking for conflict we make 1 to alias with everything that
has is_pointer flag in it, so it will return true for these two array types.

Honza
> 

> Richard.

> 

> > I already silenced the wanring for pointers but I forgot about arrays.

> > 

> > Bootstrapped/regtested x86_64-linux, OK?

> > 	

> > 	* gcc.dg/lto/pr83954.h: New testcase.

> > 	* gcc.dg/lto/pr83954_0.c: New testcase.

> > 	* gcc.dg/lto/pr83954_1.c: New testcase.

> > 	* lto-symtab.c (warn_type_compatibility_p): Silence false positive

> > 	for type match warning on arrays of pointers.

> > 

> > Index: testsuite/gcc.dg/lto/pr83954.h

> > ===================================================================

> > --- testsuite/gcc.dg/lto/pr83954.h	(revision 0)

> > +++ testsuite/gcc.dg/lto/pr83954.h	(working copy)

> > @@ -0,0 +1,3 @@

> > +struct foo;

> > +extern struct foo *FOO_PTR_ARR[1];

> > +

> > Index: testsuite/gcc.dg/lto/pr83954_0.c

> > ===================================================================

> > --- testsuite/gcc.dg/lto/pr83954_0.c	(revision 0)

> > +++ testsuite/gcc.dg/lto/pr83954_0.c	(working copy)

> > @@ -0,0 +1,8 @@

> > +/* { dg-lto-do link } */

> > +#include "pr83954.h"

> > +

> > +int main() { 

> > +  // just to prevent symbol removal

> > +  FOO_PTR_ARR[1] = 0;

> > +  return 0;

> > +}

> > Index: testsuite/gcc.dg/lto/pr83954_1.c

> > ===================================================================

> > --- testsuite/gcc.dg/lto/pr83954_1.c	(revision 0)

> > +++ testsuite/gcc.dg/lto/pr83954_1.c	(working copy)

> > @@ -0,0 +1,7 @@

> > +#include "pr83954.h"

> > +

> > +struct foo {

> > + int x;

> > +};

> > +struct foo *FOO_PTR_ARR[1] = { 0 };

> > +

> > Index: lto/lto-symtab.c

> > ===================================================================

> > --- lto/lto-symtab.c	(revision 257009)

> > +++ lto/lto-symtab.c	(working copy)

> > @@ -284,11 +284,22 @@ warn_type_compatibility_p (tree prevaili

> >        alias_set_type set1 = get_alias_set (type);

> >        alias_set_type set2 = get_alias_set (prevailing_type);

> >  

> > -      if (set1 && set2 && set1 != set2 

> > -          && (!POINTER_TYPE_P (type) || !POINTER_TYPE_P (prevailing_type)

> > +      if (set1 && set2 && set1 != set2)

> > +	{

> > +          tree t1 = type, t2 = prevailing_type;

> > +

> > +	  /* Alias sets of arrays are the same as alias sets of the inner

> > +	     types.  */

> > +	  while (TREE_CODE (t1) == ARRAY_TYPE && TREE_CODE (t2) == ARRAY_TYPE)

> > +	    {

> > +	      t1 = TREE_TYPE (t1);

> > +	      t2 = TREE_TYPE (t2);

> > +	    }

> > +          if ((!POINTER_TYPE_P (t1) || !POINTER_TYPE_P (t2))

> >  	      || (set1 != TYPE_ALIAS_SET (ptr_type_node)

> > -		  && set2 != TYPE_ALIAS_SET (ptr_type_node))))

> > -        lev |= 5;

> > +		  && set2 != TYPE_ALIAS_SET (ptr_type_node)))

> > +             lev |= 5;

> > +	}

> >      }

> >  

> >    return lev;

> > 

> > 

> 

> -- 

> Richard Biener <rguenther@suse.de>

> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Richard Biener Jan. 26, 2018, 1:39 p.m. | #3
On Fri, 26 Jan 2018, Jan Hubicka wrote:

> > On Thu, 25 Jan 2018, Jan Hubicka wrote:

> > 

> > > Hi,

> > > the testcase triggers invalid warning on type mismatch because array

> > > of pointers to complete type has different alias set from array of pointers

> > > to incomplete type.  This is valid, because incoplete pointer has alias set

> > > of void_ptr which alias all pointers and arrays alias with their members.

> > 

> > But isn't that a problem?  Not if the pointer to incomlete type have

> > alias set zero but IIRC they don't, right?

> 

> pointers to incomplete type are same as alias set of ptr_type_node (1 in this case)

> and pointer to complete types have different alias set but that set has

> is_pointer flag.  When checking for conflict we make 1 to alias with everything that

> has is_pointer flag in it, so it will return true for these two array types.


Ah, ok.  The patch is ok then.  I suppose in theory complex pointers
or vector pointers would have the same issue if we'd ever generate
those (GIMPLE at least doens't like them ;)).

Richard.

> Honza

> > 

> > Richard.

> > 

> > > I already silenced the wanring for pointers but I forgot about arrays.

> > > 

> > > Bootstrapped/regtested x86_64-linux, OK?

> > > 	

> > > 	* gcc.dg/lto/pr83954.h: New testcase.

> > > 	* gcc.dg/lto/pr83954_0.c: New testcase.

> > > 	* gcc.dg/lto/pr83954_1.c: New testcase.

> > > 	* lto-symtab.c (warn_type_compatibility_p): Silence false positive

> > > 	for type match warning on arrays of pointers.

> > > 

> > > Index: testsuite/gcc.dg/lto/pr83954.h

> > > ===================================================================

> > > --- testsuite/gcc.dg/lto/pr83954.h	(revision 0)

> > > +++ testsuite/gcc.dg/lto/pr83954.h	(working copy)

> > > @@ -0,0 +1,3 @@

> > > +struct foo;

> > > +extern struct foo *FOO_PTR_ARR[1];

> > > +

> > > Index: testsuite/gcc.dg/lto/pr83954_0.c

> > > ===================================================================

> > > --- testsuite/gcc.dg/lto/pr83954_0.c	(revision 0)

> > > +++ testsuite/gcc.dg/lto/pr83954_0.c	(working copy)

> > > @@ -0,0 +1,8 @@

> > > +/* { dg-lto-do link } */

> > > +#include "pr83954.h"

> > > +

> > > +int main() { 

> > > +  // just to prevent symbol removal

> > > +  FOO_PTR_ARR[1] = 0;

> > > +  return 0;

> > > +}

> > > Index: testsuite/gcc.dg/lto/pr83954_1.c

> > > ===================================================================

> > > --- testsuite/gcc.dg/lto/pr83954_1.c	(revision 0)

> > > +++ testsuite/gcc.dg/lto/pr83954_1.c	(working copy)

> > > @@ -0,0 +1,7 @@

> > > +#include "pr83954.h"

> > > +

> > > +struct foo {

> > > + int x;

> > > +};

> > > +struct foo *FOO_PTR_ARR[1] = { 0 };

> > > +

> > > Index: lto/lto-symtab.c

> > > ===================================================================

> > > --- lto/lto-symtab.c	(revision 257009)

> > > +++ lto/lto-symtab.c	(working copy)

> > > @@ -284,11 +284,22 @@ warn_type_compatibility_p (tree prevaili

> > >        alias_set_type set1 = get_alias_set (type);

> > >        alias_set_type set2 = get_alias_set (prevailing_type);

> > >  

> > > -      if (set1 && set2 && set1 != set2 

> > > -          && (!POINTER_TYPE_P (type) || !POINTER_TYPE_P (prevailing_type)

> > > +      if (set1 && set2 && set1 != set2)

> > > +	{

> > > +          tree t1 = type, t2 = prevailing_type;

> > > +

> > > +	  /* Alias sets of arrays are the same as alias sets of the inner

> > > +	     types.  */

> > > +	  while (TREE_CODE (t1) == ARRAY_TYPE && TREE_CODE (t2) == ARRAY_TYPE)

> > > +	    {

> > > +	      t1 = TREE_TYPE (t1);

> > > +	      t2 = TREE_TYPE (t2);

> > > +	    }

> > > +          if ((!POINTER_TYPE_P (t1) || !POINTER_TYPE_P (t2))

> > >  	      || (set1 != TYPE_ALIAS_SET (ptr_type_node)

> > > -		  && set2 != TYPE_ALIAS_SET (ptr_type_node))))

> > > -        lev |= 5;

> > > +		  && set2 != TYPE_ALIAS_SET (ptr_type_node)))

> > > +             lev |= 5;

> > > +	}

> > >      }

> > >  

> > >    return lev;

> > > 

> > > 

> > 

> > -- 

> > Richard Biener <rguenther@suse.de>

> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

> 

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Jan Hubicka Jan. 26, 2018, 1:52 p.m. | #4
> On Fri, 26 Jan 2018, Jan Hubicka wrote:

> 

> > > On Thu, 25 Jan 2018, Jan Hubicka wrote:

> > > 

> > > > Hi,

> > > > the testcase triggers invalid warning on type mismatch because array

> > > > of pointers to complete type has different alias set from array of pointers

> > > > to incomplete type.  This is valid, because incoplete pointer has alias set

> > > > of void_ptr which alias all pointers and arrays alias with their members.

> > > 

> > > But isn't that a problem?  Not if the pointer to incomlete type have

> > > alias set zero but IIRC they don't, right?

> > 

> > pointers to incomplete type are same as alias set of ptr_type_node (1 in this case)

> > and pointer to complete types have different alias set but that set has

> > is_pointer flag.  When checking for conflict we make 1 to alias with everything that

> > has is_pointer flag in it, so it will return true for these two array types.

> 

> Ah, ok.  The patch is ok then.  I suppose in theory complex pointers

> or vector pointers would have the same issue if we'd ever generate

> those (GIMPLE at least doens't like them ;)).


Yep, VECTOR_TYPE is only remaining case where get_alias_set recurse, but I would
wait for those to be actually used.  We will not only generate them but produce
static variables of them for waring to triger.

Honza

Patch

Index: testsuite/gcc.dg/lto/pr83954.h
===================================================================
--- testsuite/gcc.dg/lto/pr83954.h	(revision 0)
+++ testsuite/gcc.dg/lto/pr83954.h	(working copy)
@@ -0,0 +1,3 @@ 
+struct foo;
+extern struct foo *FOO_PTR_ARR[1];
+
Index: testsuite/gcc.dg/lto/pr83954_0.c
===================================================================
--- testsuite/gcc.dg/lto/pr83954_0.c	(revision 0)
+++ testsuite/gcc.dg/lto/pr83954_0.c	(working copy)
@@ -0,0 +1,8 @@ 
+/* { dg-lto-do link } */
+#include "pr83954.h"
+
+int main() { 
+  // just to prevent symbol removal
+  FOO_PTR_ARR[1] = 0;
+  return 0;
+}
Index: testsuite/gcc.dg/lto/pr83954_1.c
===================================================================
--- testsuite/gcc.dg/lto/pr83954_1.c	(revision 0)
+++ testsuite/gcc.dg/lto/pr83954_1.c	(working copy)
@@ -0,0 +1,7 @@ 
+#include "pr83954.h"
+
+struct foo {
+ int x;
+};
+struct foo *FOO_PTR_ARR[1] = { 0 };
+
Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c	(revision 257009)
+++ lto/lto-symtab.c	(working copy)
@@ -284,11 +284,22 @@  warn_type_compatibility_p (tree prevaili
       alias_set_type set1 = get_alias_set (type);
       alias_set_type set2 = get_alias_set (prevailing_type);
 
-      if (set1 && set2 && set1 != set2 
-          && (!POINTER_TYPE_P (type) || !POINTER_TYPE_P (prevailing_type)
+      if (set1 && set2 && set1 != set2)
+	{
+          tree t1 = type, t2 = prevailing_type;
+
+	  /* Alias sets of arrays are the same as alias sets of the inner
+	     types.  */
+	  while (TREE_CODE (t1) == ARRAY_TYPE && TREE_CODE (t2) == ARRAY_TYPE)
+	    {
+	      t1 = TREE_TYPE (t1);
+	      t2 = TREE_TYPE (t2);
+	    }
+          if ((!POINTER_TYPE_P (t1) || !POINTER_TYPE_P (t2))
 	      || (set1 != TYPE_ALIAS_SET (ptr_type_node)
-		  && set2 != TYPE_ALIAS_SET (ptr_type_node))))
-        lev |= 5;
+		  && set2 != TYPE_ALIAS_SET (ptr_type_node)))
+             lev |= 5;
+	}
     }
 
   return lev;