From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16546 invoked by alias); 24 Mar 2015 08:33:32 -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 16532 invoked by uid 89); 24 Mar 2015 08:33:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 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; Tue, 24 Mar 2015 08:33:30 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2O8XTNB029079 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 24 Mar 2015 04:33:29 -0400 Received: from tucnak.zalov.cz (ovpn-116-58.ams2.redhat.com [10.36.116.58]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2O8XR7i019234 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Tue, 24 Mar 2015 04:33:28 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.9/8.14.9) with ESMTP id t2O8XQ7H019357; Tue, 24 Mar 2015 09:33:26 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.9/8.14.9/Submit) id t2O8XPAk019356; Tue, 24 Mar 2015 09:33:25 +0100 Date: Tue, 24 Mar 2015 08:33:00 -0000 From: Jakub Jelinek To: Ilya Enkovich Cc: gcc-patches@gcc.gnu.org Subject: Re: [CHKP, PATCH] Fix instrumented indirect calls with propagated pointers Message-ID: <20150324083325.GC1746@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <20150312100931.GK27860@msticlxl57.ims.intel.com> <20150319082944.GC64546@msticlxl57.ims.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150319082944.GC64546@msticlxl57.ims.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg01235.txt.bz2 On Thu, Mar 19, 2015 at 11:29:44AM +0300, Ilya Enkovich wrote: > + /* We might propagate instrumented function pointer into > + not instrumented function and vice versa. In such a > + case we need to either fix function declaration or > + remove bounds from call statement. */ > + if (callee) > + skip_bounds = chkp_redirect_edge (e); I just want to say that I'm not really excited about all this compile time cost that is added everywhere unconditionally for chkp. I think much better would be to guard most of it with proper option check first and only do the more expensive part if the option has been used. In particular, the above call isn't inlined, > +bool > +chkp_redirect_edge (cgraph_edge *e) > +{ > + bool instrumented = false; > + tree decl = e->callee->decl; > + > + if (e->callee->instrumentation_clone > + || chkp_function_instrumented_p (decl)) > + instrumented = true; Calls here for non-instrumented code another function that calls lookup_attribute (cheap if DECL_ATTRIBUTES is NULL, not really cheap otherwise). > + if (instrumented > + && !gimple_call_with_bounds_p (e->call_stmt)) > + e->redirect_callee (cgraph_node::get_create (e->callee->orig_decl)); > + else if (!instrumented > + && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDCL) > + && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDCU) > + && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDSTX) > + && gimple_call_with_bounds_p (e->call_stmt)) Plus the ordering of the conditions above is bad, you first check for 3 out of a few thousands builtin and only then call the predicate, which should be probably done right after the !instrumented case. So, for the very likely case of -fcheck-pointer-bounds not being used at all, you've added at least 7-8 non-inlinable calls here. There are dozens of similar calls inserted just about everywhere. Jakub