From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104355 invoked by alias); 12 Mar 2015 12:10:01 -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 104342 invoked by uid 89); 12 Mar 2015 12:10:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=3.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_FROM_URIBL_PCCC,SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: eggs.gnu.org Received: from eggs.gnu.org (HELO eggs.gnu.org) (208.118.235.92) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 12 Mar 2015 12:09:57 +0000 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YW0sx-00042a-Rp for gcc-patches@gcc.gnu.org; Thu, 12 Mar 2015 07:02:47 -0400 Received: from mail-ie0-x234.google.com ([2607:f8b0:4001:c03::234]:35921) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW0sx-00042W-KF for gcc-patches@gcc.gnu.org; Thu, 12 Mar 2015 07:02:39 -0400 Received: by iegc3 with SMTP id c3so33706947ieg.3 for ; Thu, 12 Mar 2015 03:57:39 -0700 (PDT) X-Received: by 10.107.133.154 with SMTP id p26mr43971429ioi.63.1426156034717; Thu, 12 Mar 2015 03:27:14 -0700 (PDT) Received: from msticlxl57.ims.intel.com (fmdmzpr02-ext.fm.intel.com. [192.55.55.37]) by mx.google.com with ESMTPSA id j8sm4170148ioi.13.2015.03.12.03.27.13 for (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 12 Mar 2015 03:27:14 -0700 (PDT) Date: Thu, 12 Mar 2015 12:10:00 -0000 From: Ilya Enkovich To: gcc-patches@gcc.gnu.org Subject: [CHKP, PATCH] Fix LTO cgraph merge for instrumented functions Message-ID: <20150312102706.GL27860@msticlxl57.ims.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:4001:c03::234 X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00672.txt.bz2 Hi, Currently cgraph merge has several issues with instrumented code: - original function node may be removed => no assembler name conflict is detected between function and variable - only orig_decl name is privatized for instrumented function => node still shares assembler name which causes infinite privatization loop - information about changed name is stored in file_data of instrumented node => original section name may be not found for original function - chkp reference is not fixed when nodes are merged This patch should fix theese problems by keeping instrumentation thunks reachable, privatizing both nodes and fixing chkp references. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk? Thanks, Ilya -- gcc/ 2015-03-12 Ilya Enkovich * ipa-chkp.h (chkp_maybe_fix_chkp_ref): New. * ipa-chkp.c (chkp_maybe_fix_chkp_ref): New. * ipa.c (symbol_table::remove_unreachable_nodes): Don't remove instumentation thunks calling reachable functions. * lto-cgraph.c: Include ipa-chkp.h. (input_symtab): Fix chkp references for boundary nodes. * lto/lto-partition.c (privatize_symbol_name_1): New. (privatize_symbol_name): Privatize both decl and orig_decl names for instrumented functions. * lto/lto-symtab.c: Include ipa-chkp.h. (lto_cgraph_replace_node): Fix chkp references for merged function nodes. gcc/testsuite/ 2015-03-12 Ilya Enkovich * gcc.dg/lto/chkp-privatize-1_0.c: New. * gcc.dg/lto/chkp-privatize-1_1.c: New. * gcc.dg/lto/chkp-privatize-2_0.c: New. * gcc.dg/lto/chkp-privatize-2_1.c: New. diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c index 0b857ff..223f4ed 100644 --- a/gcc/ipa-chkp.c +++ b/gcc/ipa-chkp.c @@ -414,6 +414,36 @@ chkp_instrumentable_p (tree fndecl) && (!fn || !copy_forbidden (fn, fndecl))); } +/* Check NODE has a correct IPA_REF_CHKP reference. + Create a new reference if required. */ + +void +chkp_maybe_fix_chkp_ref (cgraph_node *node) +{ + /* Firstly check node needs IPA_REF_CHKP. */ + if (node->instrumentation_clone + || !node->instrumented_version) + return; + + /* Check we already have a proper IPA_REF_CHKP. + Remove incorrect refs. */ + int i; + ipa_ref *ref = NULL; + for (i = 0; node->iterate_reference (i, ref); i++) + if (ref->use == IPA_REF_CHKP) + { + /* Found proper reference. */ + if (ref->referred == node->instrumented_version) + return; + + /* Need to recreate reference. */ + ref->remove_reference (); + break; + } + + node->create_reference (node->instrumented_version, IPA_REF_CHKP, NULL); +} + /* Return clone created for instrumentation of NODE or NULL. */ cgraph_node * diff --git a/gcc/ipa-chkp.h b/gcc/ipa-chkp.h index 6708fe9..5fa7d88 100644 --- a/gcc/ipa-chkp.h +++ b/gcc/ipa-chkp.h @@ -24,5 +24,6 @@ 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); extern bool chkp_instrumentable_p (tree fndecl); +extern void chkp_maybe_fix_chkp_ref (cgraph_node *node); #endif /* GCC_IPA_CHKP_H */ diff --git a/gcc/ipa.c b/gcc/ipa.c index b3752de..ae6269f 100644 --- a/gcc/ipa.c +++ b/gcc/ipa.c @@ -492,7 +492,18 @@ symbol_table::remove_unreachable_nodes (FILE *file) } else if (cnode->thunk.thunk_p) enqueue_node (cnode->callees->callee, &first, &reachable); - + + /* For instrumentation clones we always need original + function node for proper LTO privatization. */ + if (cnode->instrumentation_clone + && reachable.contains (cnode) + && cnode->definition) + { + gcc_assert (cnode->instrumented_version); + enqueue_node (cnode->instrumented_version, &first, &reachable); + reachable.add (cnode->instrumented_version); + } + /* If any reachable function has simd clones, mark them as reachable as well. */ if (cnode->simd_clones) diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c index c875fed..b9196eb 100644 --- a/gcc/lto-cgraph.c +++ b/gcc/lto-cgraph.c @@ -80,6 +80,7 @@ along with GCC; see the file COPYING3. If not see #include "pass_manager.h" #include "ipa-utils.h" #include "omp-low.h" +#include "ipa-chkp.h" /* True when asm nodes has been output. */ bool asm_nodes_output = false; @@ -1888,6 +1889,10 @@ input_symtab (void) context of the nested function. */ if (node->lto_file_data) node->aux = NULL; + + /* May need to fix chkp reference because we don't stream + them for boundary symbols. */ + chkp_maybe_fix_chkp_ref (node); } } diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c index 235b735..7d117e9 100644 --- a/gcc/lto/lto-partition.c +++ b/gcc/lto/lto-partition.c @@ -877,27 +877,13 @@ validize_symbol_for_target (symtab_node *node) } } -/* Mangle NODE symbol name into a local name. - This is necessary to do - 1) if two or more static vars of same assembler name - are merged into single ltrans unit. - 2) if previously static var was promoted hidden to avoid possible conflict - with symbols defined out of the LTO world. */ +/* Helper for privatize_symbol_name. Mangle NODE symbol name + represented by DECL. */ static bool -privatize_symbol_name (symtab_node *node) +privatize_symbol_name_1 (symtab_node *node, tree decl) { - tree decl = node->decl; - const char *name; - cgraph_node *cnode = dyn_cast (node); - - /* If we want to privatize instrumentation clone - then we need to change original function name - which is used via transparent alias chain. */ - if (cnode && cnode->instrumentation_clone) - decl = cnode->orig_decl; - - name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)); + const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)); if (must_not_rename (node, name)) return false; @@ -906,31 +892,57 @@ privatize_symbol_name (symtab_node *node) symtab->change_decl_assembler_name (decl, clone_function_name_1 (name, "lto_priv")); + if (node->lto_file_data) lto_record_renamed_decl (node->lto_file_data, name, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl))); + + if (symtab->dump_file) + fprintf (symtab->dump_file, + "Privatizing symbol name: %s -> %s\n", + name, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl))); + + return true; +} + +/* Mangle NODE symbol name into a local name. + This is necessary to do + 1) if two or more static vars of same assembler name + are merged into single ltrans unit. + 2) if previously static var was promoted hidden to avoid possible conflict + with symbols defined out of the LTO world. */ + +static bool +privatize_symbol_name (symtab_node *node) +{ + if (!privatize_symbol_name_1 (node, node->decl)) + return false; + /* We could change name which is a target of transparent alias chain of instrumented function name. Fix alias chain if so .*/ - if (cnode) + if (cgraph_node *cnode = dyn_cast (node)) { tree iname = NULL_TREE; if (cnode->instrumentation_clone) - iname = DECL_ASSEMBLER_NAME (cnode->decl); + { + /* If we want to privatize instrumentation clone + then we also need to privatize original function. */ + if (cnode->instrumented_version) + privatize_symbol_name (cnode->instrumented_version); + else + privatize_symbol_name_1 (cnode, cnode->orig_decl); + iname = DECL_ASSEMBLER_NAME (cnode->decl); + TREE_CHAIN (iname) = DECL_ASSEMBLER_NAME (cnode->orig_decl); + } else if (cnode->instrumented_version - && cnode->instrumented_version->orig_decl == decl) - iname = DECL_ASSEMBLER_NAME (cnode->instrumented_version->decl); - - if (iname) + && cnode->instrumented_version->orig_decl == cnode->decl) { - gcc_assert (IDENTIFIER_TRANSPARENT_ALIAS (iname)); - TREE_CHAIN (iname) = DECL_ASSEMBLER_NAME (decl); + iname = DECL_ASSEMBLER_NAME (cnode->instrumented_version->decl); + TREE_CHAIN (iname) = DECL_ASSEMBLER_NAME (cnode->decl); } } - if (symtab->dump_file) - fprintf (symtab->dump_file, - "Privatizing symbol name: %s -> %s\n", - name, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl))); + return true; } diff --git a/gcc/lto/lto-symtab.c b/gcc/lto/lto-symtab.c index c00fd87..e0b9762 100644 --- a/gcc/lto/lto-symtab.c +++ b/gcc/lto/lto-symtab.c @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3. If not see #include "ipa-prop.h" #include "ipa-inline.h" #include "builtins.h" +#include "ipa-chkp.h" /* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging all edges and removing the old node. */ @@ -116,7 +117,11 @@ lto_cgraph_replace_node (struct cgraph_node *node, == prevailing_node->instrumentation_clone); node->instrumented_version->instrumented_version = prevailing_node; if (!prevailing_node->instrumented_version) - prevailing_node->instrumented_version = node->instrumented_version; + { + prevailing_node->instrumented_version = node->instrumented_version; + chkp_maybe_fix_chkp_ref (prevailing_node); + } + chkp_maybe_fix_chkp_ref (node->instrumented_version); /* Need to reset node->instrumented_version to NULL, otherwise node removal code would reset node->instrumented_version->instrumented_version. */ diff --git a/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_0.c b/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_0.c new file mode 100644 index 0000000..2054aa15 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_0.c @@ -0,0 +1,17 @@ +/* { dg-lto-do link } */ +/* { dg-require-effective-target mpx } */ +/* { dg-lto-options { { -Ofast -flto -fcheck-pointer-bounds -mmpx } } } */ + +extern int __attribute__((noinline)) f1 (int i); + +static int __attribute__((noinline)) +f2 (int i) +{ + return i + 6; +} + +int +main (int argc, char **argv) +{ + return f1 (argc) + f2 (argc); +} diff --git a/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_1.c b/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_1.c new file mode 100644 index 0000000..4fa8656 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_1.c @@ -0,0 +1,11 @@ +static int __attribute__((noinline)) +f2 (int i) +{ + return 2 * i; +} + +int __attribute__((noinline)) +f1 (int i) +{ + return f2 (i) + 10; +} diff --git a/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_0.c b/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_0.c new file mode 100644 index 0000000..be7f601 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_0.c @@ -0,0 +1,18 @@ +/* { dg-lto-do link } */ +/* { dg-require-effective-target mpx } */ +/* { dg-lto-options { { -Ofast -flto -fcheck-pointer-bounds -mmpx } } } */ + +static int +__attribute__ ((noinline)) +func1 (int i) +{ + return i + 2; +} + +extern int func2 (int i); + +int +main (int argc, char **argv) +{ + return func1 (argc) + func2 (argc) + 1; +} diff --git a/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_1.c b/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_1.c new file mode 100644 index 0000000..db39e7f --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_1.c @@ -0,0 +1,8 @@ +int func1 = 10; + +int +func2 (int i) +{ + func1++; + return i + func1; +}