Fix hash checking ICE in temp slot handling (PR middle-end/91190)
diff mbox series

Message ID 20190719080705.GT2125@tucnak
State New
Headers show
Series
  • Fix hash checking ICE in temp slot handling (PR middle-end/91190)
Related show

Commit Message

Jakub Jelinek July 19, 2019, 8:07 a.m. UTC
Hi!

As mentioned in the PR, we ICE on the following testcase.
The problem is that offset_address (in this particular case) does
  update_temp_slot_address (XEXP (memref, 0), new_rtx);
  new_rtx = change_address_1 (memref, VOIDmode, new_rtx, 1, false);
where the first call inserts the address into a hash table and computes a
hash for it, but then change_address_1 will try to legitimize that address
and modifies it in place, so now the hash table contains an entry with an
incorrect hash value (and at incorrect spot in the hash table).
Next we end up trying to insert the legitimized address into the hash table
and the new hash table checking rightfully complains.

The following patch ensures later in-place modifications of the address
don't affect the hash table.  Bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?

2019-07-19  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/91190
	* function.c (insert_temp_slot_address): Store into the hash table
	a copy of address to avoid RTL sharing issues.

	* gcc.c-torture/compile/pr91190.c: New test.


	Jakub

Comments

Richard Biener July 19, 2019, 8:40 a.m. UTC | #1
On Fri, 19 Jul 2019, Jakub Jelinek wrote:

> Hi!
> 
> As mentioned in the PR, we ICE on the following testcase.
> The problem is that offset_address (in this particular case) does
>   update_temp_slot_address (XEXP (memref, 0), new_rtx);
>   new_rtx = change_address_1 (memref, VOIDmode, new_rtx, 1, false);
> where the first call inserts the address into a hash table and computes a
> hash for it, but then change_address_1 will try to legitimize that address
> and modifies it in place, so now the hash table contains an entry with an
> incorrect hash value (and at incorrect spot in the hash table).
> Next we end up trying to insert the legitimized address into the hash table
> and the new hash table checking rightfully complains.
> 
> The following patch ensures later in-place modifications of the address
> don't affect the hash table.  Bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2019-07-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/91190
> 	* function.c (insert_temp_slot_address): Store into the hash table
> 	a copy of address to avoid RTL sharing issues.
> 
> 	* gcc.c-torture/compile/pr91190.c: New test.
> 
> --- gcc/function.c.jj	2019-07-10 15:52:56.785588326 +0200
> +++ gcc/function.c	2019-07-18 09:40:40.172869182 +0200
> @@ -704,7 +704,7 @@ static void
>  insert_temp_slot_address (rtx address, class temp_slot *temp_slot)
>  {
>    struct temp_slot_address_entry *t = ggc_alloc<temp_slot_address_entry> ();
> -  t->address = address;
> +  t->address = copy_rtx (address);
>    t->temp_slot = temp_slot;
>    t->hash = temp_slot_address_compute_hash (t);
>    *temp_slot_address_table->find_slot_with_hash (t, t->hash, INSERT) = t;
> --- gcc/testsuite/gcc.c-torture/compile/pr91190.c.jj	2019-07-18 09:52:07.364298430 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr91190.c	2019-07-18 09:51:17.892058429 +0200
> @@ -0,0 +1,31 @@
> +/* PR middle-end/91190 */
> +
> +unsigned a[1], c;
> +long d, h;
> +int e[2], f, g;
> +char i;
> +
> +int
> +main ()
> +{
> +  char k = 0;
> +  int l;
> +  while (i || d)
> +    {
> +      if (g)
> +	while (1)
> +	  ;
> +      e[1] = 0;
> +      long m[2], n = ~(3 & (5 | (h | 9) * 2237420170));
> +      g = 90 * n;
> +      char b = m[300000000], j = 0;
> +      c = 5 ^ a[c ^ (b & 5)];
> +      int o = d;
> +      k = o ? : j;
> +      if (k)
> +	for (l = 0; l < 3; l++)
> +	  if (m[200000000000000000]) 
> +	    __builtin_printf ("%d", f);
> +    }
> +  return 0; 
> +}
> 
> 	Jakub
>

Patch
diff mbox series

--- gcc/function.c.jj	2019-07-10 15:52:56.785588326 +0200
+++ gcc/function.c	2019-07-18 09:40:40.172869182 +0200
@@ -704,7 +704,7 @@  static void
 insert_temp_slot_address (rtx address, class temp_slot *temp_slot)
 {
   struct temp_slot_address_entry *t = ggc_alloc<temp_slot_address_entry> ();
-  t->address = address;
+  t->address = copy_rtx (address);
   t->temp_slot = temp_slot;
   t->hash = temp_slot_address_compute_hash (t);
   *temp_slot_address_table->find_slot_with_hash (t, t->hash, INSERT) = t;
--- gcc/testsuite/gcc.c-torture/compile/pr91190.c.jj	2019-07-18 09:52:07.364298430 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr91190.c	2019-07-18 09:51:17.892058429 +0200
@@ -0,0 +1,31 @@ 
+/* PR middle-end/91190 */
+
+unsigned a[1], c;
+long d, h;
+int e[2], f, g;
+char i;
+
+int
+main ()
+{
+  char k = 0;
+  int l;
+  while (i || d)
+    {
+      if (g)
+	while (1)
+	  ;
+      e[1] = 0;
+      long m[2], n = ~(3 & (5 | (h | 9) * 2237420170));
+      g = 90 * n;
+      char b = m[300000000], j = 0;
+      c = 5 ^ a[c ^ (b & 5)];
+      int o = d;
+      k = o ? : j;
+      if (k)
+	for (l = 0; l < 3; l++)
+	  if (m[200000000000000000]) 
+	    __builtin_printf ("%d", f);
+    }
+  return 0; 
+}