Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization.​

[Jakub Jelinek <jakub@redhat.com>]

Hi!

So, I've tried to look at
struct __attribute__((aligned (N))) S { char s[N]; };

void bar (struct S *, struct S *);

void
foo (int x)
{
  struct S a;
  {
    struct S b[x];
    bar (&a, &b[0]);
  }
  {
    struct S b[x + 4];
    bar (&a, &b[0]);
  }
}

void
baz (int x)
{
  struct S a;
  struct S b[x];
  bar (&a, &b[0]);
}
testcase at -O2 -fsanitize=address -DN=64 (and -DN=8) on x86_64.
Even in *.optimized dump I'm seeing:
  _1 = (sizetype) x_4(D);
  # RANGE [0, 18446744073709551552] NONZERO 18446744073709551552
  _2 = _1 * 64;
  # RANGE [0, 31] NONZERO 31
  _24 = _2 & 31;
  # RANGE ~[65, 127]
  _19 = _2 + 128;
  # RANGE ~[65, 96]
  _27 = _19 - _24;
  _28 = __builtin_alloca_with_align (_27, 512);
  _29 = _28 + 64;
  __builtin___asan_alloca_poison (_29, _2);
which seems to be unnecessary complicated, as _2 has nonzero
mask of 0xffffffffffffffc0 trying to and it with 0x1f should
yield certainly _24 = 0 and thus there is no need to subtract anything.

I wonder if this is just because the asan1 pass is fairly late and say
ccp isn't scheduled after it.  The question is if trying to use
gimple_build APIs instead of gimple_build_assign would help here
(and whether we'd need some new match.pd rules to figure out
that if you have SSA_NAME & constant and get_nonzero_bits on the
SSA_NAME & constant is 0, then the result is 0) or not.
Or you could just try to check get_nonzero_bits yourself and if
all the bits you want to mask are clear, avoid the subtraction.

Also, isn't the size used for the adjusted __builtin_alloca_with_align
too large?  If you need _2 initially, and alignment is 64 bytes,
then you certainly need 64 bytes before (unless we want to go into too
low-level backend details and say that we want to allocate ret + 32
as 64-byte aligned), but 64 bytes after it is too much, 32 bytes would be
enough (there is no partial right zone in this case)?

On Wed, Jun 14, 2017 at 04:21:48PM +0300, Maxim Ostapenko wrote:
> +static void
> +handle_builtin_alloca (gcall *call, gimple_stmt_iterator *iter)
> +{
> +  if (!iter)
> +    return;
> +
> +  gimple_seq seq = NULL;
> +  gassign *g;
> +  gcall *gg;
> +  gimple_stmt_iterator gsi = *iter;
> +  const HOST_WIDE_INT redzone_mask = ASAN_RED_ZONE_SIZE - 1;
> +
> +  tree last_alloca_addr = get_last_alloca_addr ();
> +  tree callee = gimple_call_fndecl (call);
> +  tree old_size = gimple_call_arg (call, 0);
> +  tree ptr_type = gimple_call_lhs (call) ? TREE_TYPE (gimple_call_lhs (call))
> +					 : ptr_type_node;
> +  bool alloca_with_align
> +    = DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA_WITH_ALIGN;
> +  unsigned int align
> +    = alloca_with_align ? tree_to_uhwi (gimple_call_arg (call, 1)) : 0;
> +
> +  /* If ALIGN > ASAN_RED_ZONE_SIZE, we embed left redzone into first ALIGN
> +     bytes of allocated space.  */
> +  align = MAX (align, ASAN_RED_ZONE_SIZE * BITS_PER_UNIT);
> +
> +  tree alloca_rz_mask = build_int_cst (size_type_node, redzone_mask);
> +  tree redzone_size = build_int_cst (size_type_node, ASAN_RED_ZONE_SIZE);
> +
> +  /* misalign = size & (ASAN_RED_ZONE_SIZE - 1)
> +     partial_size = ASAN_RED_ZONE_SIZE - misalign.  */
> +  g = gimple_build_assign (make_ssa_name (size_type_node, NULL), BIT_AND_EXPR,
> +			   old_size, alloca_rz_mask);
> +  gimple_seq_add_stmt_without_update (&seq, g);
> +  tree misalign = gimple_assign_lhs (g);
> +  g = gimple_build_assign (make_ssa_name (size_type_node, NULL), MINUS_EXPR,
> +			   redzone_size, misalign);
> +  gimple_seq_add_stmt_without_update (&seq, g);
> +  tree partial_size = gimple_assign_lhs (g);
> +
> +  /* padding = align + ASAN_RED_ZONE_SIZE;
> +     additional_size = padding + partial_size.  */
> +  tree padding = build_int_cst (size_type_node,
> +				align / BITS_PER_UNIT + ASAN_RED_ZONE_SIZE);
> +  g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR,
> +			   partial_size, padding);
> +  gimple_seq_add_stmt_without_update (&seq, g);
> +  tree additional_size = gimple_assign_lhs (g);
> +
> +  /* new_size = old_size + additional_size.  */
> +  g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, old_size,
> +			   additional_size);
> +  gimple_seq_add_stmt_without_update (&seq, g);
> +  tree new_size = gimple_assign_lhs (g);
> +
> +  /* Build new __builtin_alloca call:
> +       new_alloca_with_rz = __builtin_alloca (new_size, align).  */
> +  tree fn = builtin_decl_implicit (BUILT_IN_ALLOCA_WITH_ALIGN);
> +  gg = gimple_build_call (fn, 2, new_size,
> +			  build_int_cst (size_type_node, align));
> +  tree new_alloca_with_rz = make_ssa_name (ptr_type, gg);
> +  gimple_call_set_lhs (gg, new_alloca_with_rz);
> +  gimple_seq_add_stmt_without_update (&seq, gg);
> +
> +  /* new_alloca = new_alloca_with_rz + align.  */
> +  g = gimple_build_assign (make_ssa_name (ptr_type), POINTER_PLUS_EXPR,
> +			   new_alloca_with_rz,
> +			   build_int_cst (size_type_node,
> +					  align / BITS_PER_UNIT));
> +  gimple_seq_add_stmt_without_update (&seq, g);
> +  tree new_alloca = gimple_assign_lhs (g);
> +
> +  /* Replace old alloca ptr with NEW_ALLOCA.  */
> +  replace_call_with_value (&gsi, new_alloca);
> +
> +  /* Poison newly created alloca redzones:
> +      __asan_alloca_poison (new_alloca, old_size).  */
> +  fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCA_POISON);
> +  gg = gimple_build_call (fn, 2, new_alloca, old_size);
> +  gimple_seq_add_stmt_without_update (&seq, gg);
> +
> +  /* Save new_alloca_with_rz value into last_alloca_addr to use it during
> +     allocas unpoisoning.  */
> +  g = gimple_build_assign (last_alloca_addr, NOP_EXPR, new_alloca_with_rz);
> +  gimple_seq_add_stmt_without_update (&seq, g);
> +  gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
> +}
> +

	Jakub