From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21098 invoked by alias); 15 Sep 2014 07:51:28 -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 21087 invoked by uid 89); 15 Sep 2014 07:51:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f51.google.com Received: from mail-pa0-f51.google.com (HELO mail-pa0-f51.google.com) (209.85.220.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 15 Sep 2014 07:51:25 +0000 Received: by mail-pa0-f51.google.com with SMTP id kx10so5791582pab.38 for ; Mon, 15 Sep 2014 00:51:23 -0700 (PDT) X-Received: by 10.66.161.130 with SMTP id xs2mr35785749pab.36.1410767483136; Mon, 15 Sep 2014 00:51:23 -0700 (PDT) Received: from msticlxl57.ims.intel.com ([192.55.54.42]) by mx.google.com with ESMTPSA id pn1sm10617678pbc.21.2014.09.15.00.51.21 for (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 15 Sep 2014 00:51:22 -0700 (PDT) Date: Mon, 15 Sep 2014 07:51:00 -0000 From: Ilya Enkovich To: Jeff Law Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension Message-ID: <20140915075059.GA6922@msticlxl57.ims.intel.com> References: <20140416140313.GC41722@msticlxl57.ims.intel.com> <53CF3267.8080309@redhat.com> <20140724095907.GA15208@msticlxl57.ims.intel.com> <54076CBC.8070804@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54076CBC.8070804@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg01163.txt.bz2 On 03 Sep 13:32, Jeff Law wrote: > On 07/24/14 03:59, Ilya Enkovich wrote: > >-- > >2014-07-24 Ilya Enkovich > > > > * cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args > > field. > > (cgraph_node): Add instrumented_version, orig_decl and > > instrumentation_clone fields. > > (symtab_alias_target): Allow IPA_REF_CHKP reference. > > * cgraph.c (cgraph_remove_node): Fix instrumented_version > > of the referenced node if any. > > (dump_cgraph_node): Dump instrumentation_clone and > > instrumented_version fields. > > (verify_cgraph_node): Check correctness of IPA_REF_CHKP > > references and instrumentation thunks. > > * cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP > > reference. > > (cgraph_rebuild_references): Likewise. > > * cgraphunit.c (assemble_thunks_and_aliases): Skip thunks > > calling instrumneted function version. > > * ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP. > > (ipa_ref): increase size of use field. > > * ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP. > > * lto-cgraph.c (lto_output_node): Output instrumentation_clone, > > thunk.add_pointer_bounds_args and orig_decl field. > > (lto_output_ref): Adjust to new ipa_ref::use field size. > > (input_overwrite_node): Read instrumentation_clone field. > > (input_node): Read thunk.add_pointer_bounds_args and orig_decl > > fields. > > (input_ref): Adjust to new ipa_ref::use field size. > > (input_cgraph_1): Compute instrumented_version fields and restore > > IDENTIFIER_TRANSPARENT_ALIAS chains. > > * lto-streamer.h (LTO_minor_version): Change minor version from > > 0 to 1. > > * ipa.c (symtab_remove_unreachable_nodes): Consider instrumented > > clone as address taken if the original one is address taken. > > (cgraph_externally_visible_p): Mark instrumented 'main' as > > externally visible. > > (function_and_variable_visibility): Filter instrumentation > > thunks. > Thanks for adding the additional checking. > > > >@@ -513,6 +517,11 @@ cgraph_rebuild_references (void) > > ipa_record_stmt_references (node, gsi_stmt (gsi)); > > } > > record_eh_tables (node, cfun); > >+ > >+ > >+ if (node->instrumented_version > >+ && !node->instrumentation_clone) > >+ ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL); > > } > Trivial nit here -- just one vertical space here. > > With that nit fixed, this is OK. > > jeff > Thanks for review! Below is an updated version. It also includes usage of syntab classes. No functional changes added. Thanks, Ilya -- 2014-09-15 Ilya Enkovich * cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args field. (cgraph_node): Add instrumented_version, orig_decl and instrumentation_clone fields. (symtab_node::get_alias_target): Allow IPA_REF_CHKP reference. * cgraph.c (cgraph_node::remove): Fix instrumented_version of the referenced node if any. (cgraph_node::dump): Dump instrumentation_clone and instrumented_version fields. (cgraph_node::verify_node): Check correctness of IPA_REF_CHKP references and instrumentation thunks. * cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP reference. (cgraph_rebuild_references): Likewise. * cgraphunit.c (assemble_thunks_and_aliases): Skip thunks calling instrumneted function version. * ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP. (ipa_ref): increase size of use field. * symtab.c (ipa_ref_use_name): Add element for IPA_REF_CHKP. * lto-cgraph.c (lto_output_node): Output instrumentation_clone, thunk.add_pointer_bounds_args and orig_decl field. (lto_output_ref): Adjust to new ipa_ref::use field size. (input_overwrite_node): Read instrumentation_clone field. (input_node): Read thunk.add_pointer_bounds_args and orig_decl fields. (input_ref): Adjust to new ipa_ref::use field size. (input_cgraph_1): Compute instrumented_version fields and restore IDENTIFIER_TRANSPARENT_ALIAS chains. * lto-streamer.h (LTO_minor_version): Change minor version from 0 to 1. * ipa.c (symtab_remove_unreachable_nodes): Consider instrumented clone as address taken if the original one is address taken. * ipa-visibility.c (cgraph_externally_visible_p): Mark instrumented 'main' as externally visible. (function_and_variable_visibility): Filter instrumentation thunks. diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 5a0b903..d8fdda1 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1823,6 +1823,12 @@ cgraph_node::remove (void) } cgraph_n_nodes--; + if (instrumented_version) + { + instrumented_version->instrumented_version = NULL; + instrumented_version = NULL; + } + /* Clear out the node to NULL all pointers and add the node to the free list. */ memset (this, 0, sizeof (*this)); @@ -2069,6 +2075,11 @@ cgraph_node::dump (FILE *f) if (indirect_calls_count) fprintf (f, " Has %i outgoing edges for indirect calls.\n", indirect_calls_count); + + if (instrumentation_clone) + fprintf (f, " Is instrumented version.\n"); + else if (instrumented_version) + fprintf (f, " Has instrumented version.\n"); } /* Dump call graph node NODE to stderr. */ @@ -2834,7 +2845,9 @@ cgraph_node::verify_node (void) error_found = true; } for (i = 0; iterate_reference (i, ref); i++) - if (ref->use != IPA_REF_ALIAS) + if (ref->use == IPA_REF_CHKP) + ; + else if (ref->use != IPA_REF_ALIAS) { error ("Alias has non-alias reference"); error_found = true; @@ -2852,6 +2865,64 @@ cgraph_node::verify_node (void) error_found = true; } } + + /* Check instrumented version reference. */ + if (instrumented_version + && instrumented_version->instrumented_version != this) + { + error ("Instrumentation clone does not reference original node"); + error_found = true; + } + + /* Cannot have orig_decl for not instrumented nodes. */ + if (!instrumentation_clone && orig_decl) + { + error ("Not instrumented node has non-NULL original declaration"); + error_found = true; + } + + /* If original not instrumented node still exists then we may check + original declaration is set properly. */ + if (instrumented_version + && orig_decl + && orig_decl != instrumented_version->decl) + { + error ("Instrumented node has wrong original declaration"); + error_found = true; + } + + /* Check all nodes have chkp reference to their instrumented versions. */ + if (analyzed + && instrumented_version + && !instrumentation_clone) + { + bool ref_found = false; + int i; + struct ipa_ref *ref; + + for (i = 0; iterate_reference (i, ref); i++) + if (ref->use == IPA_REF_CHKP) + { + if (ref_found) + { + error ("Node has more than one chkp reference"); + error_found = true; + } + if (ref->referred != instrumented_version) + { + error ("Wrong node is referenced with chkp reference"); + error_found = true; + } + ref_found = true; + } + + if (!ref_found) + { + error ("Analyzed node has no reference to instrumented version"); + error_found = true; + } + } + if (analyzed && thunk.thunk_p) { if (!callees) @@ -2869,6 +2940,12 @@ cgraph_node::verify_node (void) error ("Thunk is not supposed to have body"); error_found = true; } + if (thunk.add_pointer_bounds_args + && !instrumented_version->semantically_equivalent_p (callees->callee)) + { + error ("Instrumentation thunk has wrong edge callee"); + error_found = true; + } } else if (analyzed && gimple_has_body_p (decl) && !TREE_ASM_WRITTEN (decl) diff --git a/gcc/cgraph.h b/gcc/cgraph.h index eb1aad6..cc045f6 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -516,6 +516,7 @@ struct GTY(()) cgraph_thunk_info { tree alias; bool this_adjusting; bool virtual_offset_p; + bool add_pointer_bounds_args; /* Set to true when alias node is thunk. */ bool thunk_p; }; @@ -1139,6 +1140,13 @@ public: cgraph_node *prev_sibling_clone; cgraph_node *clones; cgraph_node *clone_of; + /* If instrumentation_clone is 1 then instrumented_version points + to the original function used to make instrumented version. + Otherwise points to instrumented version of the function. */ + cgraph_node *instrumented_version; + /* If instrumentation_clone is 1 then orig_decl is the original + function declaration. */ + tree orig_decl; /* For functions with many calls sites it holds map from call expression to the edge to speed up cgraph_edge function. */ htab_t GTY((param_is (struct cgraph_edge))) call_site_hash; @@ -1199,6 +1207,9 @@ public: /* True if this decl calls a COMDAT-local function. This is set up in compute_inline_parameters and inline_call. */ unsigned calls_comdat_local : 1; + /* True when function is clone created for Pointer Bounds Checker + instrumentation. */ + unsigned instrumentation_clone : 1; }; /* A cgraph node set is a collection of cgraph nodes. A cgraph node @@ -1690,6 +1701,8 @@ symtab_node::get_alias_target (void) { struct ipa_ref *ref = NULL; iterate_reference (0, ref); + if (ref->use == IPA_REF_CHKP) + iterate_reference (1, ref); gcc_checking_assert (ref->use == IPA_REF_ALIAS); return ref->referred; } diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c index a04958f..f3e6f17 100644 --- a/gcc/cgraphbuild.c +++ b/gcc/cgraphbuild.c @@ -458,6 +458,10 @@ rebuild_cgraph_edges (void) record_eh_tables (node, cfun); gcc_assert (!node->global.inlined_to); + if (node->instrumented_version + && !node->instrumentation_clone) + node->add_reference (node->instrumented_version, IPA_REF_CHKP, NULL); + return 0; } @@ -490,6 +494,10 @@ cgraph_rebuild_references (void) node->record_stmt_references (gsi_stmt (gsi)); } record_eh_tables (node, cfun); + + if (node->instrumented_version + && !node->instrumentation_clone) + node->add_reference (node->instrumented_version, IPA_REF_CHKP, NULL); } namespace { diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index d9acc65..52e4242 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -1705,7 +1705,8 @@ assemble_thunks_and_aliases (struct cgraph_node *node) struct ipa_ref *ref; for (e = node->callers; e;) - if (e->caller->thunk.thunk_p) + if (e->caller->thunk.thunk_p + && !e->caller->thunk.add_pointer_bounds_args) { struct cgraph_node *thunk = e->caller; diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h index b8b1f9e..e0d8ffd 100644 --- a/gcc/ipa-ref.h +++ b/gcc/ipa-ref.h @@ -29,7 +29,8 @@ enum GTY(()) ipa_ref_use IPA_REF_LOAD, IPA_REF_STORE, IPA_REF_ADDR, - IPA_REF_ALIAS + IPA_REF_ALIAS, + IPA_REF_CHKP }; /* Record of reference in callgraph or varpool. */ @@ -54,7 +55,7 @@ public: gimple stmt; unsigned int lto_stmt_uid; unsigned int referred_index; - ENUM_BITFIELD (ipa_ref_use) use:2; + ENUM_BITFIELD (ipa_ref_use) use:3; unsigned int speculative:1; }; diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c index bca2bc7..4fdf804 100644 --- a/gcc/ipa-visibility.c +++ b/gcc/ipa-visibility.c @@ -260,6 +260,10 @@ cgraph_externally_visible_p (struct cgraph_node *node, if (MAIN_NAME_P (DECL_NAME (node->decl))) return true; + if (node->instrumentation_clone + && MAIN_NAME_P (DECL_NAME (node->orig_decl))) + return true; + return false; } @@ -535,6 +539,7 @@ function_and_variable_visibility (bool whole_program) } if (node->thunk.thunk_p + && !node->thunk.add_pointer_bounds_args && TREE_PUBLIC (node->decl)) { struct cgraph_node *decl_node = node; diff --git a/gcc/ipa.c b/gcc/ipa.c index ce0fc34..a37ad03 100644 --- a/gcc/ipa.c +++ b/gcc/ipa.c @@ -490,6 +490,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file) node->remove_from_same_comdat_group (); node->remove_all_references (); changed = true; + if (node->thunk.thunk_p + && node->thunk.add_pointer_bounds_args) + { + node->thunk.thunk_p = false; + node->thunk.add_pointer_bounds_args = false; + } } } else @@ -564,7 +570,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file) && !node->used_from_other_partition) { if (!node->call_for_symbol_thunks_and_aliases - (has_addr_references_p, NULL, true)) + (has_addr_references_p, NULL, true) + && (!node->instrumentation_clone + || !node->instrumented_version + || !node->instrumented_version->address_taken)) { if (file) fprintf (file, " %s", node->name ()); diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c index ed715f1..3cbff8a 100644 --- a/gcc/lto-cgraph.c +++ b/gcc/lto-cgraph.c @@ -541,6 +541,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node, bp_pack_value (&bp, node->thunk.thunk_p && !boundary_p, 1); bp_pack_enum (&bp, ld_plugin_symbol_resolution, LDPR_NUM_KNOWN, node->resolution); + bp_pack_value (&bp, node->instrumentation_clone, 1); streamer_write_bitpack (&bp); streamer_write_data_stream (ob->main_stream, section, strlen (section) + 1); @@ -549,7 +550,8 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node, streamer_write_uhwi_stream (ob->main_stream, 1 + (node->thunk.this_adjusting != 0) * 2 - + (node->thunk.virtual_offset_p != 0) * 4); + + (node->thunk.virtual_offset_p != 0) * 4 + + (node->thunk.add_pointer_bounds_args != 0) * 8); streamer_write_uhwi_stream (ob->main_stream, node->thunk.fixed_offset); streamer_write_uhwi_stream (ob->main_stream, node->thunk.virtual_value); } @@ -558,6 +560,9 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node, streamer_write_hwi_stream (ob->main_stream, node->get_init_priority ()); if (DECL_STATIC_DESTRUCTOR (node->decl)) streamer_write_hwi_stream (ob->main_stream, node->get_fini_priority ()); + + if (node->instrumentation_clone) + lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl); } /* Output the varpool NODE to OB. @@ -656,7 +661,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref, struct cgraph_node *node; bp = bitpack_create (ob->main_stream); - bp_pack_value (&bp, ref->use, 2); + bp_pack_value (&bp, ref->use, 3); bp_pack_value (&bp, ref->speculative, 1); streamer_write_bitpack (&bp); nref = lto_symtab_encoder_lookup (encoder, ref->referred); @@ -1080,6 +1085,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data, node->thunk.thunk_p = bp_unpack_value (bp, 1); node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution, LDPR_NUM_KNOWN); + node->instrumentation_clone = bp_unpack_value (bp, 1); gcc_assert (flag_ltrans || (!node->in_other_partition && !node->used_from_other_partition)); @@ -1203,6 +1209,7 @@ input_node (struct lto_file_decl_data *file_data, node->thunk.this_adjusting = (type & 2); node->thunk.virtual_value = virtual_value; node->thunk.virtual_offset_p = (type & 4); + node->thunk.add_pointer_bounds_args = (type & 8); } if (node->alias && !node->analyzed && node->weakref) node->alias_target = get_alias_symbol (node->decl); @@ -1211,6 +1218,14 @@ input_node (struct lto_file_decl_data *file_data, node->set_init_priority (streamer_read_hwi (ib)); if (DECL_STATIC_DESTRUCTOR (node->decl)) node->set_fini_priority (streamer_read_hwi (ib)); + + if (node->instrumentation_clone) + { + decl_index = streamer_read_uhwi (ib); + fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index); + node->orig_decl = fn_decl; + } + return node; } @@ -1307,7 +1322,7 @@ input_ref (struct lto_input_block *ib, struct ipa_ref *ref; bp = streamer_read_bitpack (ib); - use = (enum ipa_ref_use) bp_unpack_value (&bp, 2); + use = (enum ipa_ref_use) bp_unpack_value (&bp, 3); speculative = (enum ipa_ref_use) bp_unpack_value (&bp, 1); node = nodes[streamer_read_hwi (ib)]; ref = referring_node->add_reference (node, use); @@ -1449,6 +1464,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data, = dyn_cast (nodes[ref]); else cnode->global.inlined_to = NULL; + + /* Compute instrumented_version. */ + if (cnode->instrumentation_clone) + { + gcc_assert (cnode->orig_decl); + + cnode->instrumented_version = cgraph_node::get (cnode->orig_decl); + if (cnode->instrumented_version) + cnode->instrumented_version->instrumented_version = cnode; + + /* Restore decl names reference. */ + if (IDENTIFIER_TRANSPARENT_ALIAS (DECL_ASSEMBLER_NAME (cnode->decl)) + && !TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl))) + TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl)) + = DECL_ASSEMBLER_NAME (cnode->orig_decl); + } } ref = (int) (intptr_t) node->same_comdat_group; diff --git a/gcc/symtab.c b/gcc/symtab.c index a93c299..f975682 100644 --- a/gcc/symtab.c +++ b/gcc/symtab.c @@ -45,7 +45,7 @@ along with GCC; see the file COPYING3. If not see #include "ipa-utils.h" #include "calls.h" -static const char *ipa_ref_use_name[] = {"read","write","addr","alias"}; +static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"}; const char * const ld_plugin_symbol_resolution_names[]= {