[PATCH] Enhance ASAN_CHECK optimization

[Jakub Jelinek]

On Tue, Nov 25, 2014 at 08:06:00PM +0300, Yury Gribov wrote:
> +/* Traits class for tree hash maps below.  */
> +
> +struct tree_map_traits : default_hashmap_traits
> +{
> +  static inline hashval_t hash (const_tree ref)
> +    {
> +      return iterative_hash_expr (ref, 0);
> +    }
> +
> +  static inline bool equal_keys (const_tree ref1, const_tree ref2)
> +    {
> +      return operand_equal_p (ref1, ref2, 0);
> +    }

Formatting.  The {} should be indented like static and return 2 columns to
the right of that.

> @@ -281,37 +316,46 @@ maybe_optimize_asan_check_ifn (struct sanopt_ctx *ctx, gimple stmt)
>  
>    gimple_set_uid (stmt, info->freeing_call_events);
>  
> -  auto_vec<gimple> &v = ctx->asan_check_map.get_or_insert (ptr);
> -  if (v.is_empty ())
> +  auto_vec<gimple> *ptr_checks = &ctx->asan_check_map.get_or_insert (ptr);
> +  gimple g = maybe_get_dominating_check (*ptr_checks);
> +
> +  tree base_addr = maybe_get_single_definition (ptr);
> +  auto_vec<gimple> *base_checks = NULL;
> +  if (base_addr)
>      {
> -      /* For this PTR we don't have any ASAN_CHECK stmts recorded, so there's
> -	 nothing to optimize yet.  */
> -      v.safe_push (stmt);
> -      return false;
> +      base_checks = &ctx->asan_check_map.get_or_insert (base_addr);
> +      /* Original pointer might have been invalidated.  */
> +      ptr_checks = ctx->asan_check_map.get (ptr);
>      }

For base_addr computation, you don't really need g or ptr_checks,
do you?  So why not move the:
  auto_vec<gimple> *ptr_checks = &ctx->asan_check_map.get_or_insert (ptr);
  gimple g = maybe_get_dominating_check (*ptr_checks);
lines below the if?

> @@ -404,10 +445,7 @@ sanopt_optimize_walker (basic_block bb, struct sanopt_ctx *ctx)
>    basic_block son;
>    gimple_stmt_iterator gsi;
>    sanopt_info *info = (sanopt_info *) bb->aux;
> -  bool asan_check_optimize
> -    = (flag_sanitize & SANITIZE_ADDRESS)
> -      && ((flag_sanitize & flag_sanitize_recover
> -	   & SANITIZE_KERNEL_ADDRESS) == 0);
> +  bool asan_check_optimize = (flag_sanitize & SANITIZE_ADDRESS) != 0;
>  
>    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
>      {

I'm afraid I'm not convinced about this hunk.  If asan (kernel-address) is
recovering, I don't see a difference from not reporting two different
invalid accesses to the same function and not reporting two integer
overflows in the same function, at least if they have different location_t.

	Jakub