On Tue, Dec 13, 2016 at 01:12:34PM +0100, Martin Liška wrote:
> >> +      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.
> 
> Well, I verified that even the first iteration (of patch) needed if(finish) 
> break;
> as I need to jump to next iteration of FOR_EACH_BB_FN (bb, cfun).

I fail to see functional difference between
  for (...)
    {
      ...
      bool finish = false;
      for (...)
        {
          ...
          if (...)
            {
              ...
              finish = true;
            }
          if (finish)
            break;
        }
    }
and
  for (...)
    {
      ...
      for (...)
        {
          ...
          if (...)
            {
              ...
              break;
            }
        }
    }
just the latter is not obfuscated.  The break is in both cases in the same
for loop.

> > Don't you need also release_defs (stmt); here (and in the other gsi_remove
> > spot)?
> 
> As I remove only internal function calls that do not define a SSA name then 
> not:
>   ASAN_MARK (UNPOISON, &my_char, 1);
>   ASAN_MARK (POISON, &my_char, 1);

If they have a vdef, then I believe they do define a SSA name (the vdef).
I think unlink_stmt_vdef does not release the vdef SSA_NAME if any.

        Jakub

Reply via email to