On Wed, 14 Jun 2017, Jakub Jelinek wrote:
> 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.
The gimple_build API at the moment mirrors the behavior of building
a large GENERIC expr which means it will only match-and-simplify
stmts currently building (actually not yet associated with any BB).
So if you build _2 & 31 you get that expr folded with match.pd rules
(not sure if there is any yet doing the desired simplification to _2
using get_nonzero_bits).
If you are building a stmt at a time folding built stmts would
get you the same result (but then using the gimple_build helpers
is more powerful)
> 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
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
