On Tue, Dec 13, 2016 at 09:56:00AM +0100, Martin Liška wrote:
> @@ -671,18 +678,203 @@ public:
>
> }; // class pass_sanopt
>
Please add a short function comment here...
> +static void
> +sanitize_asan_mark_unpoison (void)
> +{
> + /* 1) Find all BBs that contain an ASAN_MARK poison call. */
> + auto_sbitmap with_poison (last_basic_block_for_fn (cfun) + 1);
> + bitmap_clear (with_poison);
> + basic_block bb;
... and here.
> +static void
> +sanitize_asan_mark_poison (void)
> +{
> + /* 1) Find all BBs that possibly contain an ASAN_CHECK. */
> + auto_sbitmap with_check (last_basic_block_for_fn (cfun) + 1);
> + bitmap_clear (with_check);
> + basic_block bb;
> + FOR_EACH_BB_FN (bb, cfun)
> + {
> + if (bitmap_bit_p (with_check, bb->index))
> + continue;
When would this happen? It seems you only set it for the current bb,
it doesn't seem that you'd try to process the same bb multiple times.
> + gimple_stmt_iterator gsi;
> + bool finish = false;
> + for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi))
> + {
> + gimple *stmt = gsi_stmt (gsi);
> + if (maybe_contains_asan_check (stmt))
> + {
> + bitmap_set_bit (with_check, bb->index);
> + finish = true;
Why the finish var and separate if (finish) break; ? Perhaps from the first
iteration when you used a switch? Just doing break; should be enough.
> + else if (asan_mark_p (stmt, ASAN_MARK_POISON))
> + {
> + if (dump_file)
> + fprintf (dump_file, "Removing ASAN_MARK poison\n");
> + unlink_stmt_vdef (stmt);
> + gsi_remove (&gsi, true);
Don't you need also release_defs (stmt); here (and in the other gsi_remove
spot)?
Jakub
