diff mbox

Fix two more memory leaks in threader

Message ID 55BF962E.409@redhat.com
State New
Headers show

Commit Message

Jeff Law Aug. 3, 2015, 4:26 p.m. UTC 
On 07/20/2015 08:19 AM, James Greenhalgh wrote:
>
> I think we either want to defer the unordered_remove until we're done
> processing all the vector elements, or make sure to look at element 'i'
> again after we've moved something new in to it.
Correct.  Two loops had this mistake -- while others got it right. 
Sigh.  The easiest fix is to change how we increment "i" in those loops 
so that it's only incremented if we do not delete the path.  That 
happens to also match how other cases are handled.

Thanks for catching these mistakes.

>
> A testcase would need to expose at least two threads which we later want
> to cancel, one of which ends up at the end of the vector of threading
> opportunities. We should then see only the first of those threads
> actually get cancelled, and the second skipped over. Reproducing these
> conditions is quite tough, which has stopped me finding a useful example
> for the list, I'll be sure to follow-up this thread if I do get to one.
Thankfully BZ had two closely related testcases.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu. 
Installed on the trunk.

(Unfortunately it didn't resolve the ppc64 issue I was looking at ;(


Jeff
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index fb9d115..9abde9f 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@ 
+2015-08-03  Jeff Law  <law@redhat.com>
+
+	PR middle-end/66314
+	PR gcov-profile/66899
+	* tree-ssa-threadupdate.c (mark_threaded_blocks): Correctly
+	iterate over the jump threading paths when an element in the
+	jump threading paths array is eliminated.
+
 2015-08-03  Segher Boessenkool  <segher@kernel.crashing.org>
 
 	* Makefile.in (OBJS): Put gimple-match.o and generic-match.o first.
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a403767..0a841b5 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@ 
+2015-08-03  Jeff Law  <law@redhat.com>
+
+	PR middle-end/66314
+	PR gcov-profile/66899
+	* gcc.dg/pr66899.c: New test.
+	* gcc.dg/pr66314.c: New test.
+
 2015-08-03  Marek Polacek  <polacek@redhat.com>
 
 	PR c/67088
diff --git a/gcc/testsuite/gcc.dg/pr66314.c b/gcc/testsuite/gcc.dg/pr66314.c
new file mode 100644
index 0000000..f74ab5b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr66314.c
@@ -0,0 +1,33 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89 -Os -fprofile-arcs -fsanitize=kernel-address" } */
+
+char *a;
+int d;
+
+static int
+fn1 (int b, int c)
+{
+  while (a)
+    if (*a)
+      return -126;
+  if (b)
+    return -12;
+  if (c == -12)
+    return c;
+}
+
+void
+fn2 (int b, int c)
+{
+  for (;;)
+    {
+      d = fn1 (b, c);
+      switch (d)
+        {
+        case -126:
+          continue;
+        default:
+          return;
+        }
+    }
+}
diff --git a/gcc/testsuite/gcc.dg/pr66899.c b/gcc/testsuite/gcc.dg/pr66899.c
new file mode 100644
index 0000000..1fff181
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr66899.c
@@ -0,0 +1,41 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Os -fprofile-arcs" } */
+
+struct
+{
+  int authority;
+} * a, *b, c, d;
+int e, f;
+static int
+fn1 ()
+{
+  if (a)
+    goto verified;
+  if (b)
+    goto matched;
+  return -126;
+matched:
+  e = 0;
+verified:
+  if (b)
+    for (; &c != b; c = d)
+      ;
+  return 0;
+}
+
+int
+fn2 ()
+{
+  for (;;)
+    {
+      f = fn1 ();
+      switch (f)
+        {
+        case -126:
+          continue;
+        default:
+          return 0;
+        }
+    }
+}
+
diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index 31ddf25..5a5f8df 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -2130,7 +2130,7 @@  mark_threaded_blocks (bitmap threaded_blocks)
      cases where the second path starts at a downstream edge on the same
      path).  First record all joiner paths, deleting any in the unexpected
      case where there is already a path for that incoming edge.  */
-  for (i = 0; i < paths.length (); i++)
+  for (i = 0; i < paths.length ();)
     {
       vec<jump_thread_edge *> *path = paths[i];
 
@@ -2140,6 +2140,7 @@  mark_threaded_blocks (bitmap threaded_blocks)
 	  if ((*path)[0]->e->aux == NULL)
 	    {
 	      (*path)[0]->e->aux = path;
+	      i++;
 	    }
 	  else
 	    {
@@ -2149,10 +2150,15 @@  mark_threaded_blocks (bitmap threaded_blocks)
 	      delete_jump_thread_path (path);
 	    }
 	}
+      else
+	{
+	  i++;
+	}
     }
+
   /* Second, look for paths that have any other jump thread attached to
      them, and either finish converting them or cancel them.  */
-  for (i = 0; i < paths.length (); i++)
+  for (i = 0; i < paths.length ();)
     {
       vec<jump_thread_edge *> *path = paths[i];
       edge e = (*path)[0]->e;
@@ -2167,7 +2173,10 @@  mark_threaded_blocks (bitmap threaded_blocks)
 	  /* If we iterated through the entire path without exiting the loop,
 	     then we are good to go, record it.  */
 	  if (j == path->length ())
-	    bitmap_set_bit (tmp, e->dest->index);
+	    {
+	      bitmap_set_bit (tmp, e->dest->index);
+	      i++;
+	    }
 	  else
 	    {
 	      e->aux = NULL;
@@ -2177,6 +2186,10 @@  mark_threaded_blocks (bitmap threaded_blocks)
 	      delete_jump_thread_path (path);
 	    }
 	}
+      else
+	{
+	  i++;
+	}
     }
 
   /* If optimizing for size, only thread through block if we don't have