Fix -Wrestrict SSA_NAME handling (PR c/83989)

Message ID 20180125215820.GS2063@tucnak
State New
Headers show
Series
  • Fix -Wrestrict SSA_NAME handling (PR c/83989)
Related show

Commit Message

Jakub Jelinek Jan. 25, 2018, 9:58 p.m.
Hi!

builtin_memref ctor for a SSA_NAME with non-NULL SSA_NAME_VAR returns
the underlying variable, rather than just the SSA_NAME.
Later on the code checks if the bases are equal and then compares
corresponding offsets.

The fact that two different SSA_NAMEs have the same underlying variable
says nothing at all whether they have the same value, as the testcase shows,
either the SSA_NAMEs can be completely unrelated, or related, but with
different offsets.  The code already has code to handle POINTER_PLUS_EXPR
with a constant offset, to look through that.  Perhaps in the future there
can be other cases we'd special case, but generally we should compare as
bases the SSA_NAMEs, if they have the same or different base says nothing
about them.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-01-25  Jakub Jelinek  <jakub@redhat.com>

	PR c/83989
	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Don't
	use SSA_NAME_VAR as base for SSA_NAMEs with non-NULL SSA_NAME_VAR.

	* c-c++-common/Wrestrict-3.c: New test.


	Jakub

Comments

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

> Hi!

> 

> builtin_memref ctor for a SSA_NAME with non-NULL SSA_NAME_VAR returns

> the underlying variable, rather than just the SSA_NAME.

> Later on the code checks if the bases are equal and then compares

> corresponding offsets.

> 

> The fact that two different SSA_NAMEs have the same underlying variable

> says nothing at all whether they have the same value, as the testcase shows,

> either the SSA_NAMEs can be completely unrelated, or related, but with

> different offsets.  The code already has code to handle POINTER_PLUS_EXPR

> with a constant offset, to look through that.  Perhaps in the future there

> can be other cases we'd special case, but generally we should compare as

> bases the SSA_NAMEs, if they have the same or different base says nothing

> about them.

> 

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?


OK.

Richard.

> 2018-01-25  Jakub Jelinek  <jakub@redhat.com>

> 

> 	PR c/83989

> 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Don't

> 	use SSA_NAME_VAR as base for SSA_NAMEs with non-NULL SSA_NAME_VAR.

> 

> 	* c-c++-common/Wrestrict-3.c: New test.

> 

> --- gcc/gimple-ssa-warn-restrict.c.jj	2018-01-17 11:54:17.000000000 +0100

> +++ gcc/gimple-ssa-warn-restrict.c	2018-01-25 14:10:26.182498552 +0100

> @@ -373,9 +373,6 @@ builtin_memref::builtin_memref (tree exp

>  		  offrange[1] += off;

>  		}

>  	    }

> -

> -	if (TREE_CODE (base) == SSA_NAME && SSA_NAME_VAR (base))

> -	  base = SSA_NAME_VAR (base);

>        }

>  

>    if (size)

> --- gcc/testsuite/c-c++-common/Wrestrict-3.c.jj	2018-01-25 14:16:01.574563425 +0100

> +++ gcc/testsuite/c-c++-common/Wrestrict-3.c	2018-01-25 14:14:39.273547506 +0100

> @@ -0,0 +1,48 @@

> +/* PR c/83989 */

> +/* { dg-do compile } */

> +/* { dg-options "-O2 -Wrestrict" } */

> +

> +__attribute__((__malloc__)) extern void *my_malloc (__SIZE_TYPE__);

> +void baz (void *);

> +

> +#define SIZE 32

> +

> +void

> +foo (void)

> +{

> +  void *recmem = __builtin_malloc (SIZE);

> +  baz (recmem);

> +  while (1)

> +    {

> +      void *oldrecmem = recmem;

> +      recmem = __builtin_malloc (SIZE);

> +      if (!recmem)

> +	{

> +	  __builtin_free (oldrecmem);

> +	  return;

> +	}

> +      __builtin_memcpy (recmem, oldrecmem, SIZE);	/* { dg-bogus "accessing" } */

> +      baz (recmem);

> +      __builtin_free (oldrecmem);

> +    }

> +}

> +

> +void

> +bar (void)

> +{

> +  void *recmem = my_malloc (SIZE);

> +  baz (recmem);

> +  while (1)

> +    {

> +      void *oldrecmem = recmem;

> +      recmem = my_malloc (SIZE);

> +      if (!recmem)

> +	{

> +	  __builtin_free (oldrecmem);

> +	  return;

> +	}

> +      __builtin_memcpy (recmem, oldrecmem, SIZE);	/* { dg-bogus "accessing" } */

> +      baz (recmem);

> +      __builtin_free (oldrecmem);

> +    }

> +}

> 

> 	Jakub

> 

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Patch

--- gcc/gimple-ssa-warn-restrict.c.jj	2018-01-17 11:54:17.000000000 +0100
+++ gcc/gimple-ssa-warn-restrict.c	2018-01-25 14:10:26.182498552 +0100
@@ -373,9 +373,6 @@  builtin_memref::builtin_memref (tree exp
 		  offrange[1] += off;
 		}
 	    }
-
-	if (TREE_CODE (base) == SSA_NAME && SSA_NAME_VAR (base))
-	  base = SSA_NAME_VAR (base);
       }
 
   if (size)
--- gcc/testsuite/c-c++-common/Wrestrict-3.c.jj	2018-01-25 14:16:01.574563425 +0100
+++ gcc/testsuite/c-c++-common/Wrestrict-3.c	2018-01-25 14:14:39.273547506 +0100
@@ -0,0 +1,48 @@ 
+/* PR c/83989 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wrestrict" } */
+
+__attribute__((__malloc__)) extern void *my_malloc (__SIZE_TYPE__);
+void baz (void *);
+
+#define SIZE 32
+
+void
+foo (void)
+{
+  void *recmem = __builtin_malloc (SIZE);
+  baz (recmem);
+  while (1)
+    {
+      void *oldrecmem = recmem;
+      recmem = __builtin_malloc (SIZE);
+      if (!recmem)
+	{
+	  __builtin_free (oldrecmem);
+	  return;
+	}
+      __builtin_memcpy (recmem, oldrecmem, SIZE);	/* { dg-bogus "accessing" } */
+      baz (recmem);
+      __builtin_free (oldrecmem);
+    }
+}
+
+void
+bar (void)
+{
+  void *recmem = my_malloc (SIZE);
+  baz (recmem);
+  while (1)
+    {
+      void *oldrecmem = recmem;
+      recmem = my_malloc (SIZE);
+      if (!recmem)
+	{
+	  __builtin_free (oldrecmem);
+	  return;
+	}
+      __builtin_memcpy (recmem, oldrecmem, SIZE);	/* { dg-bogus "accessing" } */
+      baz (recmem);
+      __builtin_free (oldrecmem);
+    }
+}