From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 41213 invoked by alias); 12 May 2016 10:42: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 41189 invoked by uid 89); 12 May 2016 10:42:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Apart, Hx-languages-length:4433, month, xxx 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 12 May 2016 10:42:03 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (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 C71A8627CF; Thu, 12 May 2016 10:42:01 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-17.ams2.redhat.com [10.36.116.17]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4CAg0UQ025026 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 12 May 2016 06:42:01 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id u4CAfwvr031918; Thu, 12 May 2016 12:41:58 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id u4CAfukx031917; Thu, 12 May 2016 12:41:56 +0200 Date: Thu, 12 May 2016 10:42:00 -0000 From: Jakub Jelinek To: Martin =?utf-8?B?TGnFoWth?= Cc: GCC Patches Subject: Re: [PATCH, RFC] Introduce -fsanitize=use-after-scope Message-ID: <20160512104156.GY28550@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <572C7A3E.4000905@suse.cz> <20160506122225.GH26501@tucnak.zalov.cz> <57332B69.4040001@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <57332B69.4040001@suse.cz> User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2016-05/txt/msg00893.txt.bz2 On Wed, May 11, 2016 at 02:54:01PM +0200, Martin Liška wrote: > On 05/06/2016 02:22 PM, Jakub Jelinek wrote: > > On Fri, May 06, 2016 at 01:04:30PM +0200, Martin Liška wrote: > >> I've started working on the patch couple of month go, basically after > >> a brief discussion with Jakub on IRC. > >> > >> I'm sending the initial version which can successfully run instrumented > >> tramp3d, postgresql server and Inkscape. It catches the basic set of > >> examples which are added in following patch. > >> > >> The implementation is quite straightforward as works in following steps: > >> > >> 1) Every local variable stack slot is poisoned at the very beginning of a function (RTL emission) > >> 2) In gimplifier, once we spot a DECL_EXPR, a variable is unpoisoned (by emitting ASAN_MARK builtin) > >> and the variable is marked as addressable > > > > Not all vars have DECL_EXPRs though. Just random comments from quick skim, need to find enough spare time to actually try it and see how it works. > Yeah, I've spotted one interesting example which is part of LLVM's testsuite: > > struct IntHolder { > int val; > }; > > const IntHolder *saved; > > void save(const IntHolder &holder) { > saved = &holder; > } > > int main(int argc, char *argv[]) { > save({10}); > int x = saved->val; // BOOM > return x; > } > > It would be also good to handle such temporaries. Any suggestions how to handle that in gimplifier? Dunno, guess you need to do something in the FE for it already (talk to Jason?). At least in *.original dump there is already: <) >>>>>; int x = (int) saved->val; return = x; and the info on where the D.2263 temporary goes out of scope is lost. > Apart from that, second version of the patch changes: > + fixed issues with missing stack unpoisoning; currently, I mark all VAR_DECLs that > are in ASAN_MARK internal fns and stack prologue/epilogue is emitted just for these vars > + removed unneeded hunks (tree-vect-patterns.c and asan_poisoning.cc) > + LABEL unpoisoning code makes stable sort for variables that were already used in the context > + stack poisoning hasn't worked for -O1+ due to following guard in asan.c > /* Automatic vars in the current function will be always accessible. */ > + direct shadow memory poisoning/unpoisoning code is introduced - in both scenarios (RTL and GIMPLE), > I would appreciate feedback if storing multiple bytes is fine? What is the maximum memory wide > store mode supported by a target? How can I get such information? > + the maximum object size handled by a direct emission is guarded by use-after-scope-direct-emission-threshold > parameter; initial value (256B) should maximally emit store of 32B Would be better if user visible param was in bytes rather than bits IMHO. > Yeah, depends because of: > > static inline bool > asan_sanitize_use_after_scope (void) > { > return ((flag_sanitize & SANITIZE_ADDRESS_USE_AFTER_SCOPE) > == SANITIZE_ADDRESS_USE_AFTER_SCOPE > && flag_stack_reuse == SR_NONE > && ASAN_STACK); > } > > Where ASAN_STACK comes from params.h. I'd prefer just prototype the function in the header and define in asan.c or some other source file. Or maybe split it, do the important case (flag_sanitize check) inline and call out of line function for the rest. Why do you check flag_stack_reuse? I thought you'd arrange for it to be different when -fsanitize=use-after-scope? > @@ -243,6 +243,11 @@ static unsigned HOST_WIDE_INT asan_shadow_offset_value; > static bool asan_shadow_offset_computed; > static vec sanitized_sections; > > +/* Set of variable declarations that are going to be guarded by > + use-after-scope sanitizer. */ > + > +static hash_set asan_handled_variables(13); Not sure about the formatting here, don't we use xxx instead of xxx ? And I'd expect space before (. > @@ -1020,6 +1020,91 @@ asan_function_start (void) > current_function_funcdef_no); > } > > +/* Return number of shadow bytes that are occupied by a local variable > + of SIZE bytes. */ > + > +static unsigned HOST_WIDE_INT > +get_shadow_memory_size (unsigned HOST_WIDE_INT size) > +{ > + /* Round up size of object. */ > + unsigned HOST_WIDE_INT r; > + if ((r = size % BITS_PER_UNIT) != 0) > + size += BITS_PER_UNIT - r; Isn't there a ROUND_UP macro? Jakub