On Wed, Nov 16, 2016 at 05:01:31PM +0100, Martin Liška wrote:
> +  use_operand_p use_p;
> +  imm_use_iterator imm_iter;
> +  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, poisoned_var)
> +    {
> +      gimple *use = USE_STMT (use_p);
> +      if (is_gimple_debug (use))
> +     continue;
> +
> +      built_in_function b = (recover_p
> +                          ? BUILT_IN_ASAN_REPORT_USE_AFTER_SCOPE_NOABORT
> +                          : BUILT_IN_ASAN_REPORT_USE_AFTER_SCOPE);
> +      tree fun = builtin_decl_implicit (b);
> +      pretty_printer pp;
> +      pp_tree_identifier (&pp, DECL_NAME (var_decl));
> +
> +      gcall *call = gimple_build_call (fun, 2, asan_pp_string (&pp),
> +                                    DECL_SIZE_UNIT (var_decl));
> +      gimple_set_location (call, gimple_location (use));
> +
> +      /* The USE can be a gimple PHI node.  If so, insert the call on
> +      all edges leading to the PHI node.  */
> +      if (is_a <gphi *> (use))
> +     {
> +       gphi * phi = dyn_cast<gphi *> (use);

No space after *.

> +       for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
> +         if (gimple_phi_arg_def (phi, i) == poisoned_var)
> +           {
> +             edge e = gimple_phi_arg_edge (phi, i);
> +             gsi_insert_seq_on_edge (e, call);
> +             *need_commit_edge_insert = true;

You clearly don't have a sufficient testsuite coverage for this,
because this won't really work if you have more than one phi
argument equal to poisoned_var.  Inserting the same gimple stmt
into multiple places can't really work.  I bet you want to set
call to NULL after the gsi_insert_seq_on_edge and before that
call if (call == NULL) { call = gimple_build_call (...); gimple_set_location 
(...); }
Or maybe gimple_copy for the 2nd etc. would work too, dunno.

> +           }
> +     }
> +      else
> +     {
> +       gimple_stmt_iterator gsi = gsi_for_stmt (use);
> +       gsi_insert_before (&gsi, call, GSI_NEW_STMT);
> +     }
> +    }
> +
> +  gimple *nop = gimple_build_nop ();
> +  SSA_NAME_IS_DEFAULT_DEF (poisoned_var) = true;
> +  SSA_NAME_DEF_STMT (poisoned_var) = nop;
> +  gsi_replace (iter, nop, GSI_NEW_STMT);

The last argument of gsi_replace is a bool, not GSI_*.
But not sure how this will work anyway, I think SSA_NAME_IS_DEFAULT_DEF
are supposed to have SSA_NAME_DEF_STMT a GIMPLE_NOP that doesn't
have bb set, while you are putting it into the stmt sequence.
Shouldn't you just gsi_remove iter instead?

Otherwise LGTM, but please post the asan patch to llvm-commits
or through their web review interface.

        Jakub

Reply via email to