On December 16, 2016 9:56:10 PM GMT+01:00, Jakub Jelinek <ja...@redhat.com> 
wrote:
>Hi!
>
>The following testcase fails with -fcompare-debug, because we have a bb
>containing 2 ASAN_MARK (POISON, ...) calls immediately after each
>other,
>followed with -g only by debug stmts till end of basic block.
>sanitize_asan_mark_poison walks stmts in a bb backwards and assumes
>(incorrectly) that gsi_remove will move the iterator backwards too, but
>it moves it forward (and if removing stmt at the end of bb turns it
>into
>gsi end).  The effect is that with -g, we remove both ASAN_MARK calls
>(desirable), because after the first removal gsi is moved to the
>following
>debug stmt which we do nothing about once again and get to the first
>ASAN_MARK, but with -g0 gsi becomes end and we don't remove anything
>further
>from the bb after removing the last stmt.
>
>Fixed by copying the iterator to a copy, gsi_prev and then gsi_remove
>on the
>copy.  The rest is just cleanup, I think we don't need the bool vars
>when we
>can easily continue; instead.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

>2016-12-16  Jakub Jelinek  <ja...@redhat.com>
>
>       PR sanitizer/78832
>       * sanopt.c (sanitize_asan_mark_unpoison): Remove next variable, use
>       continue if gsi_next should be skipped.
>       (sanitize_asan_mark_poison): Remove prev variable, use continue if
>       gsi_prev should be skipped.  When removing ASAN_MARK, do gsi_prev
>       first and gsi_remove on a previously made copy of the iterator.
>
>       * gcc.dg/asan/pr78832.c: New test.
>
>--- gcc/sanopt.c.jj    2016-12-14 20:28:14.000000000 +0100
>+++ gcc/sanopt.c       2016-12-16 17:08:03.016432304 +0100
>@@ -740,7 +740,6 @@ sanitize_asan_mark_unpoison (void)
>       gimple_stmt_iterator gsi;
>       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
>       {
>-        bool next = true;
>         gimple *stmt = gsi_stmt (gsi);
>         if (gimple_call_internal_p (stmt, IFN_ASAN_MARK))
>           {
>@@ -753,12 +752,11 @@ sanitize_asan_mark_unpoison (void)
>                 unlink_stmt_vdef (stmt);
>                 release_defs (stmt);
>                 gsi_remove (&gsi, true);
>-                next = false;
>+                continue;
>               }
>           }
> 
>-        if (next)
>-          gsi_next (&gsi);
>+        gsi_next (&gsi);
>       }
>     }
> }
>@@ -840,7 +838,6 @@ sanitize_asan_mark_poison (void)
>       gimple_stmt_iterator gsi;
>       for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi);)
>       {
>-        bool prev = true;
>         gimple *stmt = gsi_stmt (gsi);
>         if (maybe_contains_asan_check (stmt))
>           break;
>@@ -850,12 +847,13 @@ sanitize_asan_mark_poison (void)
>               fprintf (dump_file, "Removing ASAN_MARK poison\n");
>             unlink_stmt_vdef (stmt);
>             release_defs (stmt);
>-            gsi_remove (&gsi, true);
>-            prev = false;
>+            gimple_stmt_iterator gsi2 = gsi;
>+            gsi_prev (&gsi);
>+            gsi_remove (&gsi2, true);
>+            continue;
>           }
> 
>-        if (prev)
>-          gsi_prev (&gsi);
>+        gsi_prev (&gsi);
>       }
>     }
> }
>--- gcc/testsuite/gcc.dg/asan/pr78832.c.jj     2016-12-16
>17:22:27.446280214 +0100
>+++ gcc/testsuite/gcc.dg/asan/pr78832.c        2016-12-16 17:23:17.117638783
>+0100
>@@ -0,0 +1,22 @@
>+/* PR sanitizer/78832 */
>+/* { dg-do compile } */
>+/* { dg-additional-options "-fcompare-debug" } */
>+
>+void bar (int *);
>+
>+int
>+foo (int x)
>+{
>+  int *f = 0;
>+  if (x)
>+    goto lab;
>+  {
>+    int y, z;
>+    bar (&y);
>+    int *d = &y;
>+    bar (&z);
>+    int *e = &z;
>+  }
>+  f = &x;
>+  lab: return 6;
>+}
>
>       Jakub


Reply via email to