From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37350 invoked by alias); 16 Jan 2017 14:20:35 -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 37336 invoked by uid 89); 16 Jan 2017 14:20:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sk:SSA_NAM, sk:ssa_nam, GSI X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 16 Jan 2017 14:20:31 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 55C1CC0567A2; Mon, 16 Jan 2017 14:20:31 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-250.ams2.redhat.com [10.36.116.250]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0GEKT1q017048 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 16 Jan 2017 09:20:30 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id v0GEKQv8028532; Mon, 16 Jan 2017 15:20:27 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id v0GEKPJR028531; Mon, 16 Jan 2017 15:20:25 +0100 Date: Mon, 16 Jan 2017 14:20:00 -0000 From: Jakub Jelinek To: Martin =?utf-8?B?TGnFoWth?= Cc: Richard Biener , GCC Patches Subject: Re: [PATCH] Speed-up use-after-scope (re-writing to SSA) (version 2) Message-ID: <20170116142025.GO1867@tucnak> Reply-To: Jakub Jelinek References: <774a5d54-30f6-3212-ea4c-21e751356055@suse.cz> <20161116130733.GT3541@tucnak.redhat.com> <469bf86a-e43c-c571-66e4-87db78b6fb11@suse.cz> <20161116162841.GX3541@tucnak.redhat.com> <20161221085200.GS21933@tucnak> <4ec48432-9df6-154a-1b13-065b9772cbbf@suse.cz> <20161222172140.GF21933@tucnak> <29d32a7c-a95d-ddb1-d64e-ae8f659d3a4b@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <29d32a7c-a95d-ddb1-d64e-ae8f659d3a4b@suse.cz> User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg01101.txt.bz2 On Mon, Jan 09, 2017 at 03:58:04PM +0100, Martin Liška wrote: > >> Well, having following sample: > >> > >> int > >> main (int argc, char **argv) > >> { > >> int *ptr = 0; > >> > >> { > >> int a; > >> ptr = &a; > >> *ptr = 12345; > >> } > >> > >> *ptr = 12345; > >> return *ptr; > >> } > >> > I'm still not sure how to do that. Problem is that transformation from: > > ASAN_MARK (UNPOISON, &a, 4); > a = 5; > ASAN_MARK (POISON, &a, 4); > > to > > a_8 = 5; > a_9 = ASAN_POISON (); > > happens in tree-ssa.c, after SSA is created, in situation where we prove the 'a' > does not need to live in memory. Thus said, question is how to identify that we > need to transform into SSA in a different way: > > a_10 = ASAN_POISON (); > ASAN_POISON (a_10); I meant something like this (completely untested, and without the testcase added to the testsuite). The incremental patch as is relies on the ASAN_POISON_USE call having the argument the result of ASAN_POISON, it would ICE if that is not the case (especially if -fsanitize-recover=address). Dunno if some optimization might decide to create a PHI in between, say merge two unrelated vars for if (something) { x_1 = ASAN_POISON (); ... ASAN_POISON_USE (x_1); } else { y_2 = ASAN_POISON (); ... ASAN_POISON_USE (y_2); } to turn that into: if (something) x_1 = ASAN_POISON (); else y_2 = ASAN_POISON (); _3 = PHI ; ... ASAN_POISON_USE (_3); If it did, we would ICE because ASAN_POISON_USE would survive this way until expansion. A quick fix for the ICE (if it can ever happen) would be easy, in sanopt remove ASAN_POISON_USE calls which have argument that is not lhs of ASAN_POISON (all other ASAN_POISON_USE calls will be handled by my incremental patch). Of course that would also mean in that case we'd report a read rather than write. But if it can't happen or is very unlikely to happen, then it is a non-issue. Something missing from the patch is some change in DCE to remove ASAN_POISON calls without lhs earlier. I think we can't make ASAN_POISON ECF_CONST, we don't want it to be merged for different variables. --- gcc/internal-fn.def.jj 2017-01-16 13:19:49.000000000 +0100 +++ gcc/internal-fn.def 2017-01-16 14:25:37.427962196 +0100 @@ -167,6 +167,7 @@ DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, EC DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".R...") DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, ".R..") DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL) +DEF_INTERNAL_FN (ASAN_POISON_USE, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL) DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) --- gcc/asan.c.jj 2017-01-16 13:19:49.000000000 +0100 +++ gcc/asan.c 2017-01-16 14:52:34.022044223 +0100 @@ -3094,6 +3094,8 @@ create_asan_shadow_var (tree var_decl, return *slot; } +/* Expand ASAN_POISON ifn. */ + bool asan_expand_poison_ifn (gimple_stmt_iterator *iter, bool *need_commit_edge_insert, @@ -3107,8 +3109,8 @@ asan_expand_poison_ifn (gimple_stmt_iter return true; } - tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var), - shadow_vars_mapping); + tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var), + shadow_vars_mapping); bool recover_p; if (flag_sanitize & SANITIZE_USER_ADDRESS) @@ -3122,16 +3124,16 @@ asan_expand_poison_ifn (gimple_stmt_iter ASAN_MARK_POISON), build_fold_addr_expr (shadow_var), size); - use_operand_p use_p; + gimple *use; imm_use_iterator imm_iter; - FOR_EACH_IMM_USE_FAST (use_p, imm_iter, poisoned_var) + FOR_EACH_IMM_USE_STMT (use, imm_iter, poisoned_var) { - gimple *use = USE_STMT (use_p); if (is_gimple_debug (use)) continue; int nargs; - tree fun = report_error_func (false, recover_p, tree_to_uhwi (size), + bool store_p = gimple_call_internal_p (use, IFN_ASAN_POISON_USE); + tree fun = report_error_func (store_p, recover_p, tree_to_uhwi (size), &nargs); gcall *call = gimple_build_call (fun, 1, @@ -3160,7 +3162,10 @@ asan_expand_poison_ifn (gimple_stmt_iter else { gimple_stmt_iterator gsi = gsi_for_stmt (use); - gsi_insert_before (&gsi, call, GSI_NEW_STMT); + if (store_p) + gsi_replace (&gsi, call, true); + else + gsi_insert_before (&gsi, call, GSI_NEW_STMT); } } --- gcc/tree-into-ssa.c.jj 2017-01-01 12:45:35.000000000 +0100 +++ gcc/tree-into-ssa.c 2017-01-16 14:32:14.853808726 +0100 @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3. #include "tree-ssa.h" #include "domwalk.h" #include "statistics.h" +#include "asan.h" #define PERCENT(x,y) ((float)(x) * 100.0 / (float)(y)) @@ -1807,6 +1808,26 @@ maybe_replace_use_in_debug_stmt (use_ope } +/* If DEF has x_5 = ASAN_POISON () as its current def, add + ASAN_POISON_USE (x_5) stmt before GSI to denote the stmt writes into + a poisoned (out of scope) variable. */ + +static void +maybe_add_asan_poison_write (tree def, gimple_stmt_iterator *gsi) +{ + tree cdef = get_current_def (def); + if (cdef != NULL + && TREE_CODE (cdef) == SSA_NAME + && gimple_call_internal_p (SSA_NAME_DEF_STMT (cdef), IFN_ASAN_POISON)) + { + gcall *call + = gimple_build_call_internal (IFN_ASAN_POISON_USE, 1, cdef); + gimple_set_location (call, gimple_location (gsi_stmt (*gsi))); + gsi_insert_before (gsi, call, GSI_SAME_STMT); + } +} + + /* If the operand pointed to by DEF_P is an SSA name in NEW_SSA_NAMES or OLD_SSA_NAMES, or if it is a symbol marked for renaming, register it as the current definition for the names replaced by @@ -1837,7 +1858,11 @@ maybe_register_def (def_operand_p def_p, def = get_or_create_ssa_default_def (cfun, sym); } else - def = make_ssa_name (def, stmt); + { + if (asan_sanitize_use_after_scope ()) + maybe_add_asan_poison_write (def, &gsi); + def = make_ssa_name (def, stmt); + } SET_DEF (def_p, def); tree tracked_var = target_for_debug_bind (sym); --- gcc/internal-fn.c.jj 2017-01-16 13:19:49.000000000 +0100 +++ gcc/internal-fn.c 2017-01-16 14:26:10.828529039 +0100 @@ -388,6 +388,14 @@ expand_ASAN_POISON (internal_fn, gcall * gcc_unreachable (); } +/* This should get expanded in the sanopt pass. */ + +static void +expand_ASAN_POISON_USE (internal_fn, gcall *) +{ + gcc_unreachable (); +} + /* This should get expanded in the tsan pass. */ static void Jakub