From patchwork Mon Nov 17 13:31:37 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Enkovich X-Patchwork-Id: 411677 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 820C314012B for ; Tue, 18 Nov 2014 00:32:00 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=tS4G8uNRpUEiVz7Lj MJ2TFqUvOQHNfHaePRmWYWRIY0h91uLR+LZgCflVwGnq6ROPqQvWRvjTUZHe1Z1/ NFbVjCBmw5PC4dz08ZngUMFuiKQhshXlkGRd8ZzFEsNOvF8XPp0Hpyv31QBJPf7o WBgxgnGrcQpnjX//vW/dg7mzk8= 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:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=05ap1G567awJRjnP5kXyPQ/ vzzc=; b=ydtfkG6C5tbVO26rfE63WMq7+/FIZF05eNsd9Ev+eswUqAMqyJiB1yo afRzmbf0B53u7lfyn5uik5fGPxvuFXAYw6pPeF2a/THZMP6jrQSf2KgZnyZVOZqP dQQaVqWsHSxMoJjjthZvW4NAlo8nzQa4x/0X7hZfV8+NpW6SmUuc= Received: (qmail 16389 invoked by alias); 17 Nov 2014 13:31:52 -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 16374 invoked by uid 89); 17 Nov 2014 13:31:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f173.google.com Received: from mail-pd0-f173.google.com (HELO mail-pd0-f173.google.com) (209.85.192.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 17 Nov 2014 13:31:49 +0000 Received: by mail-pd0-f173.google.com with SMTP id ft15so274717pdb.4 for ; Mon, 17 Nov 2014 05:31:47 -0800 (PST) X-Received: by 10.69.31.138 with SMTP id km10mr30052406pbd.6.1416231107770; Mon, 17 Nov 2014 05:31:47 -0800 (PST) Received: from msticlxl57.ims.intel.com ([192.55.54.40]) by mx.google.com with ESMTPSA id xd3sm5554236pbc.54.2014.11.17.05.31.45 for (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 17 Nov 2014 05:31:47 -0800 (PST) Date: Mon, 17 Nov 2014 16:31:37 +0300 From: Ilya Enkovich To: Jeff Law Cc: gcc-patches Subject: Re: [PATCH, Pointer Bounds Checker, Builtins instrumentation 1/5] Builtin codes and decls Message-ID: <20141117133137.GA5422@msticlxl57.ims.intel.com> References: <20141106114818.GB53257@msticlxl57.ims.intel.com> <5465A4AD.80300@redhat.com> <5466FAC1.50709@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5466FAC1.50709@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes On 15 Nov 00:03, Jeff Law wrote: > On 11/14/14 01:22, Ilya Enkovich wrote: > > > >I don't think I'm hiding some problem here. Builtin function calls > >may be removed during various optimizations. Therefore we may remove > >all calls to some instrumented builtins and corresponding cgraph_node > >is removed as unreachable (but fndecl still exists). Later calls to > >removed function may be created again. IIRC in my test case it > >happened in strlen pass which may replace builtin calls with another > >ones. In this case cgraph_node is recreated but fndecl recreation > >should be avoided, existing one should be used instead. > OK. Please add those details to the comment. With that comment > updated, OK for the trunk. > > jeff > Thanks! Below is a final version. Ilya diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c index 19a9894..3e343d4 100644 --- a/gcc/ipa-chkp.c +++ b/gcc/ipa-chkp.c @@ -129,6 +129,16 @@ chkp_build_instrumented_fndecl (tree fndecl) make own copy. */ DECL_ATTRIBUTES (new_decl) = copy_list (DECL_ATTRIBUTES (fndecl)); + /* Change builtin function code. */ + if (DECL_BUILT_IN (new_decl)) + { + gcc_assert (DECL_BUILT_IN_CLASS (new_decl) == BUILT_IN_NORMAL); + gcc_assert (DECL_FUNCTION_CODE (new_decl) < BEGIN_CHKP_BUILTINS); + DECL_FUNCTION_CODE (new_decl) + = (enum built_in_function)(DECL_FUNCTION_CODE (new_decl) + + BEGIN_CHKP_BUILTINS + 1); + } + return new_decl; } @@ -354,6 +364,33 @@ chkp_add_bounds_params_to_function (tree fndecl) chkp_copy_function_type_adding_bounds (TREE_TYPE (fndecl)); } +/* Return an instrumentation clone for builtin function + FNDECL. Create one if needed. */ + +tree +chkp_maybe_clone_builtin_fndecl (tree fndecl) +{ + tree clone; + enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl); + + gcc_assert (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL + && fcode < BEGIN_CHKP_BUILTINS); + + fcode = (enum built_in_function) (fcode + BEGIN_CHKP_BUILTINS + 1); + clone = builtin_decl_explicit (fcode); + if (clone) + return clone; + + clone = chkp_build_instrumented_fndecl (fndecl); + chkp_add_bounds_params_to_function (clone); + + gcc_assert (DECL_FUNCTION_CODE (clone) == fcode); + + set_builtin_decl (fcode, clone, false); + + return clone; +} + /* Return clone created for instrumentation of NODE or NULL. */ cgraph_node * @@ -364,6 +401,54 @@ chkp_maybe_create_clone (tree fndecl) gcc_assert (!node->instrumentation_clone); + if (DECL_BUILT_IN (fndecl) + && (DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL + || DECL_FUNCTION_CODE (fndecl) >= BEGIN_CHKP_BUILTINS)) + return NULL; + + clone = node->instrumented_version; + + /* Some instrumented builtin function calls may be optimized and + cgraph nodes may be removed as unreachable. Later optimizations + may generate new calls to removed functions and in this case + we have to recreate cgraph node. FUNCTION_DECL for instrumented + builtin still exists and should be reused in such case. */ + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL + && fndecl == builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)) + && !clone) + { + enum built_in_function fncode = DECL_FUNCTION_CODE (fndecl); + tree new_decl; + + fncode = (enum built_in_function) (fncode + BEGIN_CHKP_BUILTINS + 1); + new_decl = builtin_decl_explicit (fncode); + + /* We've actually already created an instrumented clone once. + Restore it. */ + if (new_decl) + { + clone = cgraph_node::get (new_decl); + + if (!clone) + { + gcc_assert (!gimple_has_body_p (fndecl)); + clone = cgraph_node::get_create (new_decl); + clone->externally_visible = node->externally_visible; + clone->local = node->local; + clone->address_taken = node->address_taken; + clone->thunk = node->thunk; + clone->alias = node->alias; + clone->weakref = node->weakref; + clone->cpp_implicit_alias = node->cpp_implicit_alias; + clone->orig_decl = fndecl; + clone->instrumentation_clone = true; + } + + clone->instrumented_version = node; + node->instrumented_version = clone; + } + } + if (!clone) { tree new_decl = chkp_build_instrumented_fndecl (fndecl); @@ -408,6 +493,15 @@ chkp_maybe_create_clone (tree fndecl) actually copies args list from the original decl. */ chkp_add_bounds_params_to_function (new_decl); + /* Remember builtin fndecl. */ + if (DECL_BUILT_IN_CLASS (clone->decl) == BUILT_IN_NORMAL + && fndecl == builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) + { + gcc_assert (!builtin_decl_explicit (DECL_FUNCTION_CODE (clone->decl))); + set_builtin_decl (DECL_FUNCTION_CODE (clone->decl), + clone->decl, false); + } + /* Clones have the same comdat group as originals. */ if (node->same_comdat_group || DECL_ONE_ONLY (node->decl)) diff --git a/gcc/ipa-chkp.h b/gcc/ipa-chkp.h index d4ad113..b2d03ad 100644 --- a/gcc/ipa-chkp.h +++ b/gcc/ipa-chkp.h @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #define GCC_IPA_CHKP_H extern tree chkp_copy_function_type_adding_bounds (tree orig_type); +extern tree chkp_maybe_clone_builtin_fndecl (tree fndecl); extern cgraph_node *chkp_maybe_create_clone (tree fndecl); #endif /* GCC_IPA_CHKP_H */ diff --git a/gcc/tree-core.h b/gcc/tree-core.h index 58bdfff..fe3fbb4 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -168,6 +168,14 @@ enum built_in_class { enum built_in_function { #include "builtins.def" + BEGIN_CHKP_BUILTINS, + +#undef DEF_BUILTIN +#define DEF_BUILTIN(ENUM, N, C, T, LT, B, F, NA, AT, IM, COND) ENUM##_CHKP, +#include "builtins.def" + + END_CHKP_BUILTINS, + /* Complex division routines in libgcc. These are done via builtins because emit_library_call_value can't handle complex values. */ BUILT_IN_COMPLEX_MUL_MIN, diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c index a11a46e..fba6048 100644 --- a/gcc/tree-streamer-in.c +++ b/gcc/tree-streamer-in.c @@ -49,6 +49,7 @@ along with GCC; see the file COPYING3. If not see #include "streamer-hooks.h" #include "lto-streamer.h" #include "builtins.h" +#include "ipa-chkp.h" /* Read a STRING_CST from the string table in DATA_IN using input block IB. */ @@ -1113,6 +1114,14 @@ streamer_get_builtin_tree (struct lto_input_block *ib, struct data_in *data_in) if (fcode >= END_BUILTINS) fatal_error ("machine independent builtin code out of range"); result = builtin_decl_explicit (fcode); + if (!result + && fcode > BEGIN_CHKP_BUILTINS + && fcode < END_CHKP_BUILTINS) + { + fcode = (enum built_in_function) (fcode - BEGIN_CHKP_BUILTINS - 1); + result = builtin_decl_explicit (fcode); + result = chkp_maybe_clone_builtin_fndecl (result); + } gcc_assert (result); } else if (fclass == BUILT_IN_MD)