From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23725 invoked by alias); 16 Jun 2017 07:34:05 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 23698 invoked by uid 89); 16 Jun 2017 07:34:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 16 Jun 2017 07:34:02 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 44F34ADA5; Fri, 16 Jun 2017 07:34:05 +0000 (UTC) Date: Fri, 16 Jun 2017 07:34:00 -0000 From: Richard Biener To: Jakub Jelinek cc: Maxim Ostapenko , GCC Patches , Marek Polacek , Yuri Gribov Subject: =?UTF-8?Q?Re=3A_=5BPING^3=5D=5BRFC=2C_PATCH=5D=5BASAN=5D_Implement_dynamic_allocas=2FVLAs_sanitization=2E=E2=80=8B?= In-Reply-To: <20170614173420.GD2123@tucnak> Message-ID: References: <591C40E9.7030307@samsung.com> <59365A82.2070806@samsung.com> <593FD67C.5040607@samsung.com> <20170613131539.GH2099@tucnak> <5941386C.30306@samsung.com> <20170614173420.GD2123@tucnak> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2017-06/txt/msg01160.txt.bz2 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 SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)