[RFC][PATCH] Sanopt for use-after-scope ASAN_MARK internal functions

Jakub Jelinek jakub@redhat.com
Fri Dec 9 11:56:00 GMT 2016


On Fri, Dec 09, 2016 at 12:39:24PM +0100, Martin Liška wrote:
> +	  if (is_gimple_call (stmt) && gimple_call_internal_p (stmt))
> +	    {
> +	      enum internal_fn ifn = gimple_call_internal_fn (stmt);
> +	      switch (ifn)
> +		{
> +		case IFN_ASAN_MARK:
> +		  {
> +		    HOST_WIDE_INT flags = tree_to_shwi (gimple_call_arg (stmt, 0));
> +		    bool is_clobber = (flags & ASAN_MARK_CLOBBER) != 0;
> +		    if (is_clobber)
> +		      {
> +			bitmap_set_bit (with_poison, bb->index);
> +			finish = true;
> +		      }
> +		    break;
> +		  }
> +		  default:
> +		    break;
> +		}

This looks weird.  Wouldn't it be nicer to just use
	      if (gimple_call_internal_fn (stmt) == IFN_ASAN_MARK)
		{
		  HOST_WIDE_INT flags = tree_to_shwi (gimple_call_arg (stmt, 0));
		  bool is_clobber = (flags & ASAN_MARK_CLOBBER) != 0;
		  if (is_clobber)
		    {
		      bitmap_set_bit (with_poison, bb->index);
		      finish = true;
		    }
		}
(or don't use finish and just break; there)?
> +	      enum internal_fn ifn = gimple_call_internal_fn (stmt);
> +	      switch (ifn)
> +		{
> +		case IFN_ASAN_MARK:
> +		  {

Likewise.

No function comment:

> +static bool
> +maybe_contains_asan_check (gimple *stmt)
> +{
> +  if (is_gimple_call (stmt))
> +    {
> +      if (gimple_call_internal_p (stmt))
> +	{
> +	  enum internal_fn ifn = gimple_call_internal_fn (stmt);
> +	  switch (ifn)
> +	    {
> +	    case IFN_ASAN_CHECK:
> +	      return true;
> +	    default:
> +	      return false;

Are all internal functions really known to be ASAN_CHECK free?

> +	    }
> +	}
> +      else
> +        return true;

What about builtins?  Many will not be fine, but many should be known
to be ASAN_CHECK free.  Consider e.g. most math builtins (except sincos).

> @@ -698,6 +928,9 @@ pass_sanopt::execute (function *fun)
>    bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
>      && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
>  
> +  sanitize_asan_mark_unpoison ();
> +  sanitize_asan_mark_poison ();
> +

What I don't really like here is that you force several IL walks for all
sanitizers, just in case.  Look e.g. how asan_num_accesses is computed,
couldn't you compute similarly has_asan_marks and guard those two functions
on that?

	Jakub



More information about the Gcc-patches mailing list