This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix -fcompare-debug sanopt bug (PR sanitizer/78832)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>,Martin Liška <mliska at suse dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Sat, 17 Dec 2016 20:06:01 +0100
- Subject: Re: [PATCH] Fix -fcompare-debug sanopt bug (PR sanitizer/78832)
- Authentication-results: sourceware.org; auth=none
- References: <20161216205610.GH21933@tucnak>
On December 16, 2016 9:56:10 PM GMT+01:00, Jakub Jelinek <jakub@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 <jakub@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