This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension


On 22 Jul 21:56, Jeff Law wrote:
> On 04/16/14 08:03, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch introduces changes in call graph for Pointer Bounds Checker.
> >
> >New fields instrumented_version, instrumentation_clone and orig_decl are added for cgraph_node:
> >  - instrumentation_clone field is 1 for nodes created for instrumented version of functions
> >  - instrumented_version points to instrumented/original node
> >  - orig_decl holds original function declaration for instrumented nodes in case original node is removed
> >
> >IPA_REF_CHKP reference type is introduced for nodes to reference instrumented function versions from originals.  It is used to have proper reachability analysis.
> >
> >When original function bodies are not needed anymore, functions are transformed into thunks having call edge to the instrumented function.  Therefore new field appeared in cgraph_thunk_info to mark such thunks.
> >
> >Does it look OK?
> >
> >Bootstrapped and tested on linux-x86_64.
> >
> >Thanks,
> >Ilya
> >--
> >gcc/
> >
> >2014-04-16  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >	* 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.
> >
> >
> >diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> >index be3661a..6210c68 100644
> >--- a/gcc/cgraph.c
> >+++ b/gcc/cgraph.c
> >@@ -2850,7 +2861,9 @@ verify_cgraph_node (struct cgraph_node *node)
> >  	}
> >        for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
> >  						  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;
> Is there any checking you can/should be doing here?    And I'm
> asking because I'm pretty sure there's something you ought to be
> checking here :-)
> 
> There's a general desire for key datastructures to sanity check them
> as much as possible.

Thanks for comments!  I added additional check for chkp references.  It is performed later because this piece of code is for aliases only.

> 
> >+  /* 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.  */
> >+  struct cgraph_node *instrumented_version;
> >+  /* If instrumentation_clone is 1 then orig_decl is the original
> >+     function declaration.  */
> >+  tree orig_decl;
> So I don't see anything which checks these two invariants.
> 
> Mostly it looks good.  I do want to look at it again once the
> verification stuff is beefed up.
> 
> 
> Jeff

Added checks for all new fields.  Below is a new patch version.

Thanks,
Ilya
--
2014-07-24  Ilya Enkovich  <ilya.enkovich@intel.com>

	* 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.


diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index be3661a..cfd8708 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1828,6 +1828,12 @@ cgraph_remove_node (struct cgraph_node *node)
     }
   cgraph_n_nodes--;
 
+  if (node->instrumented_version)
+    {
+      node->instrumented_version->instrumented_version = NULL;
+      node->instrumented_version = NULL;
+    }
+
   /* Clear out the node to NULL all pointers and add the node to the free
      list.  */
   memset (node, 0, sizeof (*node));
@@ -2070,6 +2076,11 @@ dump_cgraph_node (FILE *f, struct cgraph_node *node)
   if (indirect_calls_count)
     fprintf (f, "  Has %i outgoing edges for indirect calls.\n",
 	     indirect_calls_count);
+
+  if (node->instrumentation_clone)
+    fprintf (f, "  Is instrumented version.\n");
+  else if (node->instrumented_version)
+    fprintf (f, "  Has instrumented version.\n");
 }
 
 
@@ -2850,7 +2861,9 @@ verify_cgraph_node (struct cgraph_node *node)
 	}
       for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
 						  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;
@@ -2868,6 +2881,65 @@ verify_cgraph_node (struct cgraph_node *node)
 	    error_found = true;
 	  }
     }
+
+  /* Check instrumented version reference.  */
+  if (node->instrumented_version
+      && node->instrumented_version->instrumented_version != node)
+    {
+      error ("Instrumentation clone does not reference original node");
+      error_found = true;
+    }
+
+  /* Cannot have orig_decl for not instrumented nodes.  */
+  if (!node->instrumentation_clone && node->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 (node->instrumented_version
+      && node->orig_decl
+      && node->orig_decl != node->instrumented_version->decl)
+    {
+      error ("Instrumented node has wrong original declaration");
+      error_found = true;
+    }
+
+  /* Check all nodes have chkp reference to their instrumented versions.  */
+  if (node->analyzed
+      && node->instrumented_version
+      && !node->instrumentation_clone)
+    {
+      bool ref_found = false;
+      int i;
+      struct ipa_ref *ref;
+
+      for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
+						  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 != node->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 (node->analyzed && node->thunk.thunk_p)
     {
       if (!node->callees)
@@ -2885,6 +2957,12 @@ verify_cgraph_node (struct cgraph_node *node)
 	  error ("Thunk is not supposed to have body");
           error_found = true;
         }
+      if (node->thunk.add_pointer_bounds_args
+	  && node->callees->callee != node->instrumented_version)
+	{
+	  error ("Instrumentation thunk has wrong edge callee");
+          error_found = true;
+	}
     }
   else if (node->analyzed && gimple_has_body_p (node->decl)
            && !TREE_ASM_WRITTEN (node->decl)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index a6a51cf..5e702a7 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -191,6 +191,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;
 };
@@ -373,6 +374,13 @@ public:
   struct cgraph_node *prev_sibling_clone;
   struct cgraph_node *clones;
   struct 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.  */
+  struct 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;
@@ -433,6 +441,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;
 };
 
 
@@ -1412,6 +1423,8 @@ symtab_alias_target (symtab_node *n)
 {
   struct ipa_ref *ref;
   ipa_ref_list_reference_iterate (&n->ref_list, 0, ref);
+  if (ref->use == IPA_REF_CHKP)
+    ipa_ref_list_reference_iterate (&n->ref_list, 1, ref);
   gcc_checking_assert (ref->use == IPA_REF_ALIAS);
   return ref->referred;
 }
diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
index 19961e2..a2b2106 100644
--- a/gcc/cgraphbuild.c
+++ b/gcc/cgraphbuild.c
@@ -481,6 +481,10 @@ rebuild_cgraph_edges (void)
   record_eh_tables (node, cfun);
   gcc_assert (!node->global.inlined_to);
 
+  if (node->instrumented_version
+      && !node->instrumentation_clone)
+    ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
+
   return 0;
 }
 
@@ -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);
 }
 
 namespace {
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 06283fc..ceb4060 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1702,7 +1702,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.c b/gcc/ipa-ref.c
index 6aa41e6..3a055d9 100644
--- a/gcc/ipa-ref.c
+++ b/gcc/ipa-ref.c
@@ -27,7 +27,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cgraph.h"
 #include "ipa-utils.h"
 
-static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
+static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
 
 /* Return ipa reference from REFERING_NODE or REFERING_VARPOOL_NODE
    to REFERED_NODE or REFERED_VARPOOL_NODE. USE_TYPE specify type
diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
index 4ce5f8d..d0df0bf 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.  */
@@ -40,7 +41,7 @@ struct GTY(()) ipa_ref
   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.c b/gcc/ipa.c
index 5ab3aed..1d7fa35 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -508,6 +508,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
 	      cgraph_node_remove_callees (node);
 	      ipa_remove_all_references (&node->ref_list);
 	      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
@@ -583,7 +589,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
     if (node->address_taken
 	&& !node->used_from_other_partition)
       {
-	if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true))
+	if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true)
+	    && (!node->instrumentation_clone
+		|| !node->instrumented_version
+		|| !node->instrumented_version->address_taken))
 	  {
 	    if (file)
 	      fprintf (file, " %s", node->name ());
@@ -814,6 +823,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;
 }
 
@@ -1016,6 +1029,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/lto-cgraph.c b/gcc/lto-cgraph.c
index 999ce3d..58105f0 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -526,6 +526,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);
 
   if (node->thunk.thunk_p && !boundary_p)
@@ -533,11 +534,15 @@ 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);
     }
   streamer_write_hwi_stream (ob->main_stream, node->profile_id);
+
+  if (node->instrumentation_clone)
+    lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl);
 }
 
 /* Output the varpool NODE to OB. 
@@ -613,7 +618,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);
@@ -1002,6 +1007,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));
@@ -1112,10 +1118,19 @@ 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);
   node->profile_id = 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;
 }
 
@@ -1196,7 +1211,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 = ipa_record_reference (referring_node, node, use, NULL);
@@ -1337,6 +1352,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data,
 	    cgraph (node)->global.inlined_to = cgraph (nodes[ref]);
 	  else
 	    cnode->global.inlined_to = NULL;
+
+	  /* Compute instrumented_version.  */
+	  if (cnode->instrumentation_clone)
+	    {
+	      gcc_assert (cnode->orig_decl);
+
+	      cnode->instrumented_version = cgraph_get_node (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/lto-streamer.h b/gcc/lto-streamer.h
index 51b1903..62a5fe0 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -141,7 +141,7 @@ along with GCC; see the file COPYING3.  If not see
 #define LTO_SECTION_NAME_PREFIX         ".gnu.lto_"
 
 #define LTO_major_version 3
-#define LTO_minor_version 0
+#define LTO_minor_version 1
 
 typedef unsigned char	lto_decl_flags_t;
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]