From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 124326 invoked by alias); 21 Oct 2016 14:26:38 -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 124303 invoked by uid 89); 21 Oct 2016 14:26:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=instruments, poisoning, follow-ups, followups 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; Fri, 21 Oct 2016 14:26:26 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (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 8C1756ADB; Fri, 21 Oct 2016 14:26:25 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-44.ams2.redhat.com [10.36.116.44]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9LEQM0m007743 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 21 Oct 2016 10:26:24 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id u9LEQKWW006370; Fri, 21 Oct 2016 16:26:20 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id u9LEQH2a003563; Fri, 21 Oct 2016 16:26:17 +0200 Date: Fri, 21 Oct 2016 14:26:00 -0000 From: Jakub Jelinek To: Martin =?utf-8?B?TGnFoWth?= Cc: GCC Patches Subject: Re: [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2) Message-ID: <20161021142617.GG7282@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <572C7A3E.4000905@suse.cz> <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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg01761.txt.bz2 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 > > Yes, that's missing. I'm wondering whether the same approach would be viable as > the {un}poisoning happens during gimplification. Thus it generates &var expressions > which verifier won't be happy about? Sure, it uses &var. The trick is that the addressable sub-pass then ignores the taking of the address just in ASAN_MARK, and if some var is determined to be addressable solely because of ASAN_MARK &var uses and no other reason, the ASAN_MARK would be dropped and variable rewritten into SSA form. > > - optimization to remove ASAN_MARK unpoisoning at the start of the function > > As current implementation does not poison variables at the very beginning of > a functions (RTL stack frame emission), it won't be needed. But you still ASAN_MARK unpoison the vars when they get into scope, right? And those can be removed if the optimizers could prove that the area has not been poisoned yet since the beginning of the function. > > - optimization to remove ASAN_MARK poisoning at the end of function (and > > remove epilogue unpoisoning) > > - optimization to remove ASAN_MARK unpoisoning followed by ASAN_MARK poisoning > > or vice versa if there are no memory accesses in between > > Yep, both are not handled and are very similar from my perspective: pairing > poisoning and unpoisoning pair which are not interfered by a memory operation > in between (in dominator meaning of word). > I'm wondering whether can be done effectively as we would need to visit all BBs > in a dominance domain (get_all_dominated_blocks) and check for the memory operations. > One improvement can be set of BBs that do not have any memory operations (excluding > ASAN_CHECK, ASAN_MARK builtins), but it can be still expensive to simplify poisoning > for all local variables. Or am I wrong? My memory is weak, but isn't this something the sanopt pass (sanopt_optimize) is already doing similarly for e.g. ASAN_CHECK, UBSAN_NULL and UBSAN_VPTR checks? For ASAN_MARK, you actually don't care about any calls, those shouldn't unpoison or poison again the vars under compiler's back. > > - try to improve the goto handling > > Works for me to be target for stage3. Sure. > 2016-09-27 Martin Liska Likely newer date :) > * c-common.c (warn_for_unused_label): Save all labels used > in goto or in &label; &label. instead? > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + const char *n = DECL_NAME (decl) > + ? IDENTIFIER_POINTER (DECL_NAME (decl)) : ""; Bad formatting. const char *n = (DECL_NAME (decl) ? IDENTIFIER_POINTER (DECL_NAME (decl)) : ""); or const char *n = (DECL_NAME (decl) ? IDENTIFIER_POINTER (DECL_NAME (decl)) : ""); > /* 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); > + return DECL_P (decl) && TREE_ADDRESSABLE (decl); > } Can you explain this change? It won't affect just -fsanitize-use-after-scope, and goes in both directions, adds some decls that weren't previously protected and removes others that were previously protected. Is the removal of !DECL_ARTIFICIAL so that you can use-after-scope verify the C++ TARGET_EXPR temporaries? Do we need to stack protect those even for -fno-sanitize-use-after-scope? I'm not 100% sure if we keep TREE_ADDRESSABLE meaningful for variables that need to live in memory for other reasons until expansion. So, I wonder if we don't want && (TREE_ADDRESSABLE (decl) || !DECL_ARTIFICAL (decl)) or just DECL_P (decl); or conditionalize something on -fsanitize-use-after-scope. Perhaps the change is good as is, just want to point out that it affects also other -fsanitize=address modes in both ways (instruments something that hasn't been before, and stops instrumenting something that has been before). > @@ -1514,7 +1503,8 @@ defer_stack_allocation (tree var, bool toplevel) > /* If stack protection is enabled, *all* stack variables must be deferred, > so that we can re-order the strings to the top of the frame. > Similarly for Address Sanitizer. */ > - if (flag_stack_protect || asan_sanitize_stack_p ()) > + if (flag_stack_protect > + || asan_sanitize_stack_p ()) > return true; This hunk isn't needed, if all the conditions fit on one line, one line is better. Jakub