From patchwork Tue Nov 1 14:53:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Martin_Li=C5=A1ka?= X-Patchwork-Id: 80371 Delivered-To: patch@linaro.org Received: by 10.80.142.83 with SMTP id 19csp689601edx; Tue, 1 Nov 2016 07:54:13 -0700 (PDT) X-Received: by 10.98.184.25 with SMTP id p25mr59911259pfe.56.1478012053375; Tue, 01 Nov 2016 07:54:13 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id 70si30943668pga.188.2016.11.01.07.54.12 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 01 Nov 2016 07:54:13 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-440043-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-440043-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-440043-patch=linaro.org@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=EIfOvoMs2vv5TTVSG zeydDtqLaW0SBugpt21295GXslGePaGWI6w15AtBRvhFdc8jryW0/0mBN+FdoCgL bQ58rmBYO1MN4HMNfUloBv9S7SMiWUx8sYdYyETPbLe16Jvu8HxA8c4QgtEkJrRn Y8lMAlPqyl+NSgd93SiFuK3ZaU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=weGskZr58oOnsfOwFAdkJ6e cqeg=; b=Sg/d4LjL8lB85rwsHq74oTuVjOkIkuFiyFoh8/dwlgCM1luV35aY4Gy yNj9xfAADcy/LA0epbajGX4Njl7xfX0Zv6pNzHSFo1bOAhL8J4kDUcCAxOzFYNBI IjwDUVmH7hII+izHguUOxi2Z8WmLksJjpvIyquM3xI80E8fiSFnk= Received: (qmail 46605 invoked by alias); 1 Nov 2016 14:54:00 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 45863 invoked by uid 89); 1 Nov 2016 14:53:59 -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 autolearn=ham version=3.3.2 spammy=30pm, bare, yep, considerations X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 Nov 2016 14:53:49 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 2BC35ACC3; Tue, 1 Nov 2016 14:53:47 +0000 (UTC) Subject: Re: [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2) To: Jakub Jelinek References: <20160506122225.GH26501@tucnak.zalov.cz> <57332B69.4040001@suse.cz> <20160512104156.GY28550@tucnak.redhat.com> <57348F45.5020700@suse.cz> <20160818133609.GN14857@tucnak.redhat.com> <98f408c5-7e1e-6fd8-e589-34f8de2f4455@suse.cz> <20161007111347.GF7282@tucnak.redhat.com> <20161021142617.GG7282@tucnak.redhat.com> <3a109250-0440-7438-8e1f-7e5c6d8b6580@suse.cz> <20161027172358.GN3541@tucnak.redhat.com> Cc: GCC Patches From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <606cd948-6cba-02a4-f114-35900ab53203@suse.cz> Date: Tue, 1 Nov 2016 15:53:46 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161027172358.GN3541@tucnak.redhat.com> X-IsSubscribed: yes On 10/27/2016 07:23 PM, Jakub Jelinek wrote: > On Thu, Oct 27, 2016 at 04:40:30PM +0200, Martin Liška wrote: >> On 10/21/2016 04:26 PM, Jakub Jelinek wrote: >>> On Wed, Oct 12, 2016 at 04:07:53PM +0200, Martin Liška wrote: >>>>> Ok, first let me list some needed follow-ups that don't need to be handled >>>>> right away: >>>>> - r237814-like changes for ASAN_MARK >> >> I've spent quite some on that and that's what I begin (use-after-scope-addressable.patch). >> Problem is that as I ignore all ASAN_MARK internal fns, the code does not detect having address >> taken in: >> >> _2 = MEM[(char *)&my_char + 8B]; >> >> char *ptr; >> { >> char my_char[9]; >> ptr = &my_char[0]; >> } >> >> return *(ptr+8); >> >> and thus the code in tree-ssa.c (maybe_optimize_var) sets TREE_ADDRESSABLE (var) = 0. > > Perhaps we should do that only if the var's type is_gimple_reg_type, > then we'd rewrite that into SSA at that time, right? So, in theory if we > turned the ASAN_MARK poisoning call into another internal function > (var_5 = ASAN_POISON ()) and then after converting it into SSA looked at > all the uses of such an lhs and perhaps at sanopt part or when marked all > the use places with a library call that would complain at runtime? > Or turn those back at sanopt time into addressable memory loads which would > be poisoned or similar? Or alternatively, immediately before turning > variables addressable just because of ASAN_MARK into non-addressable use > the same framework into-ssa uses to find out if there are any poisoned > accesses, and just not optimize it in that case. > Anyway, this can be done incrementally. I've done a patch candidate (not tested yet) which is capable of ASAN_MARK removal for local variables that can be rewritten into SSA. This is done by running execute_update_addresses_taken after we create ASAN_CHECK internal fns and skipping all ASAN_MARK for having address taken considerations. This removes significant number of ASAN_MARK fns in tramp3d (due to C++ temporaries). > >> Second question I have is whether we want to handle just TREE_ADDRESSABLE stuff during gimplification? >> Basically in a way that the current patch is doing? > > How could variables that aren't TREE_ADDRESSABLE during gimplification be > accessed out of scope? Yep, TREE_ADDRESSABLE guard does what it should do. I'm going to test the patch which can be installed incrementally. Martin > >> +/* Return true if DECL should be guarded on the stack. */ >> + >> +static inline bool >> +asan_protect_stack_decl (tree decl) >> +{ >> + return DECL_P (decl) >> + && (!DECL_ARTIFICIAL (decl) >> + || (asan_sanitize_use_after_scope () && TREE_ADDRESSABLE (decl))); > > Bad formatting. Should be: > > return (DECL_P (decl) > && (!DECL_ARTIFICIAL (decl) > || (asan_sanitize_use_after_scope () > && TREE_ADDRESSABLE (decl)))); > > Ok for trunk with that change. > > Jakub > >From cf860324da41244745f04a16b184fabe343ac5d9 Mon Sep 17 00:00:00 2001 From: marxin Date: Tue, 1 Nov 2016 11:21:20 +0100 Subject: [PATCH 4/4] Use-after-scope: do not mark variables that are no longer addressable gcc/ChangeLog: 2016-11-01 Martin Liska * asan.c (asan_instrument): Call execute_update_addresses_taken_asan_sanitize right after sanitization. * tree-ssa.c (maybe_optimize_var): Mark all variables set to non-addressable. (is_asan_mark_p): New function. (execute_update_addresses_taken): Likewise. (execute_update_addresses_taken_asan_sanitize): Likewise. * tree-ssa.h (execute_update_addresses_taken_asan_sanitize): Declare new function. --- gcc/asan.c | 4 +++ gcc/tree-ssa.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++----------- gcc/tree-ssa.h | 1 + 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/gcc/asan.c b/gcc/asan.c index 95495d2..5cb37c8 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -59,6 +59,7 @@ along with GCC; see the file COPYING3. If not see #include "params.h" #include "builtins.h" #include "fnmatch.h" +#include "tree-ssa.h" /* AddressSanitizer finds out-of-bounds and use-after-free bugs with <2x slowdown on average. @@ -2973,6 +2974,9 @@ asan_instrument (void) if (shadow_ptr_types[0] == NULL_TREE) asan_init_shadow_ptr_types (); transform_statements (); + + if (optimize) + execute_update_addresses_taken_asan_sanitize (); return 0; } diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c index 135952b..0633b21 100644 --- a/gcc/tree-ssa.c +++ b/gcc/tree-ssa.c @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see #include "cfgexpand.h" #include "tree-cfg.h" #include "tree-dfa.h" +#include "asan.h" /* Pointer map of variable mappings, keyed by edge. */ static hash_map > *edge_var_maps; @@ -1504,7 +1505,7 @@ non_rewritable_lvalue_p (tree lhs) static void maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs, - bitmap suitable_for_renaming) + bitmap suitable_for_renaming, bitmap marked_nonaddressable) { /* Global Variables, result decls cannot be changed. */ if (is_global_var (var) @@ -1522,6 +1523,7 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs, || !bitmap_bit_p (not_reg_needs, DECL_UID (var)))) { TREE_ADDRESSABLE (var) = 0; + bitmap_set_bit (marked_nonaddressable, DECL_UID (var)); if (is_gimple_reg (var)) bitmap_set_bit (suitable_for_renaming, DECL_UID (var)); if (dump_file) @@ -1550,20 +1552,43 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs, } } -/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables. */ +/* Return true when STMT is ASAN mark where second argument is an address + of a local variable. */ -void -execute_update_addresses_taken (void) +static bool +is_asan_mark_p (gimple *stmt) +{ + if (!gimple_call_internal_p (stmt, IFN_ASAN_MARK)) + return false; + + tree addr = get_base_address (gimple_call_arg (stmt, 1)); + if (TREE_CODE (addr) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (addr, 0)) == VAR_DECL) + return true; + + return false; +} + +/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables. + If SANITIZE_ASAN_MARK is set to true, sanitize also ASAN_MARK built-ins. */ + + +static void +execute_update_addresses_taken (bool sanitize_asan_mark = false) { basic_block bb; bitmap addresses_taken = BITMAP_ALLOC (NULL); bitmap not_reg_needs = BITMAP_ALLOC (NULL); bitmap suitable_for_renaming = BITMAP_ALLOC (NULL); + bitmap marked_nonaddressable = BITMAP_ALLOC (NULL); tree var; unsigned i; timevar_push (TV_ADDRESS_TAKEN); + if (dump_file) + fprintf (dump_file, "call execute_update_addresses_taken\n"); + /* Collect into ADDRESSES_TAKEN all variables whose address is taken within the function body. */ FOR_EACH_BB_FN (bb, cfun) @@ -1575,17 +1600,23 @@ execute_update_addresses_taken (void) enum gimple_code code = gimple_code (stmt); tree decl; - if (code == GIMPLE_CALL - && optimize_atomic_compare_exchange_p (stmt)) + if (code == GIMPLE_CALL) { - /* For __atomic_compare_exchange_N if the second argument - is &var, don't mark var addressable; - if it becomes non-addressable, we'll rewrite it into - ATOMIC_COMPARE_EXCHANGE call. */ - tree arg = gimple_call_arg (stmt, 1); - gimple_call_set_arg (stmt, 1, null_pointer_node); - gimple_ior_addresses_taken (addresses_taken, stmt); - gimple_call_set_arg (stmt, 1, arg); + if (optimize_atomic_compare_exchange_p (stmt)) + { + /* For __atomic_compare_exchange_N if the second argument + is &var, don't mark var addressable; + if it becomes non-addressable, we'll rewrite it into + ATOMIC_COMPARE_EXCHANGE call. */ + tree arg = gimple_call_arg (stmt, 1); + gimple_call_set_arg (stmt, 1, null_pointer_node); + gimple_ior_addresses_taken (addresses_taken, stmt); + gimple_call_set_arg (stmt, 1, arg); + } + else if (sanitize_asan_mark && is_asan_mark_p (stmt)) + ; + else + gimple_ior_addresses_taken (addresses_taken, stmt); } else /* Note all addresses taken by the stmt. */ @@ -1675,15 +1706,17 @@ execute_update_addresses_taken (void) for -g vs. -g0. */ for (var = DECL_ARGUMENTS (cfun->decl); var; var = DECL_CHAIN (var)) maybe_optimize_var (var, addresses_taken, not_reg_needs, - suitable_for_renaming); + suitable_for_renaming, marked_nonaddressable); FOR_EACH_VEC_SAFE_ELT (cfun->local_decls, i, var) maybe_optimize_var (var, addresses_taken, not_reg_needs, - suitable_for_renaming); + suitable_for_renaming, marked_nonaddressable); /* Operand caches need to be recomputed for operands referencing the updated variables and operands need to be rewritten to expose bare symbols. */ - if (!bitmap_empty_p (suitable_for_renaming)) + if (!bitmap_empty_p (suitable_for_renaming) + || (asan_sanitize_use_after_scope () + && !bitmap_empty_p (marked_nonaddressable))) { FOR_EACH_BB_FN (bb, cfun) for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);) @@ -1841,6 +1874,17 @@ execute_update_addresses_taken (void) continue; } } + else if (sanitize_asan_mark && is_asan_mark_p (stmt)) + { + tree var = TREE_OPERAND (gimple_call_arg (stmt, 1), 0); + if (bitmap_bit_p (suitable_for_renaming, DECL_UID (var)) + || bitmap_bit_p (marked_nonaddressable, DECL_UID (var))) + { + unlink_stmt_vdef (stmt); + gsi_remove (&gsi, true); + continue; + } + } for (i = 0; i < gimple_call_num_args (stmt); ++i) { tree *argp = gimple_call_arg_ptr (stmt, i); @@ -1896,9 +1940,27 @@ execute_update_addresses_taken (void) BITMAP_FREE (not_reg_needs); BITMAP_FREE (addresses_taken); BITMAP_FREE (suitable_for_renaming); + BITMAP_FREE (marked_nonaddressable); timevar_pop (TV_ADDRESS_TAKEN); } +/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables. */ + +void +execute_update_addresses_taken (void) +{ + execute_update_addresses_taken (false); +} + +/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables + and sanitize ASAN_MARK built-ins. */ + +void +execute_update_addresses_taken_asan_sanitize (void) +{ + execute_update_addresses_taken (true); +} + namespace { const pass_data pass_data_update_address_taken = diff --git a/gcc/tree-ssa.h b/gcc/tree-ssa.h index 37ad7ae..912d144 100644 --- a/gcc/tree-ssa.h +++ b/gcc/tree-ssa.h @@ -53,6 +53,7 @@ extern tree tree_ssa_strip_useless_type_conversions (tree); extern bool ssa_undefined_value_p (tree, bool = true); extern bool gimple_uses_undefined_value_p (gimple *); extern void execute_update_addresses_taken (void); +extern void execute_update_addresses_taken_asan_sanitize (void); /* Given an edge_var_map V, return the PHI arg definition. */ -- 2.10.1