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 14/x] Passes [9/n] Static constructors


On 09 Oct 13:04, Jeff Law wrote:
> On 10/08/14 13:10, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch introduces functions to handle static pointers and static bounds.
> >
> >Thanks,
> >Ilya
> >--
> >2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >	* tree-chkp.c (MAX_STMTS_IN_STATIC_CHKP_CTOR): New.
> >	(chkp_ctor_stmt_list): New.
> >	(chkp_register_var_initializer): New.
> >	(chkp_add_modification_to_stmt_list): New.
> >	(chkp_output_static_bounds): New.
> >	(chkp_finish_file): New.
> >	(chkp_instrument_function): Remove useless statements
> >	from static bounds constructors.
> >	* tree-chkp.h (chkp_register_var_initializer): New.
> >	(chkp_finish_file): New.
> Thanks for clarifying on the testcase.  I misunderstood the testing
> methodology and hence the results made no sense to me :-)
> 
> 
> Make the maximum statements a PARAM
> >
> >
> >diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
> >index b424af8..4b5a773 100644
> >--- a/gcc/tree-chkp.c
> >+++ b/gcc/tree-chkp.c
> >@@ -394,6 +394,27 @@ static bool in_chkp_pass;
> >  #define CHKP_ZERO_BOUNDS_VAR_NAME "__chkp_zero_bounds"
> >  #define CHKP_NONE_BOUNDS_VAR_NAME "__chkp_none_bounds"
> >
> >+/* Static checker constructors may become very large and their
> >+   compilation with optimization may take too much time.
> >+   Therefore we put a limit to number of statements in one
> >+   construcor.  Tests with 100 000 statically initialized
> s/construcor/constructor/
> 
> 
> 
> >+   static bounds initilization.  If VAR is added into
> >+   bounds initlization list then 1 is returned. Otherwise
> s/initilization/initialization/
> 
> >+   into list of static initilizer statementes (passed in ARG).
> s/initilizer/initializer/
> 
> This will be fine with the change to a PARAM and the nit spelling
> stuff fixed.
> 
> jeff

Thanks for review!  Here is a fixed version.

Ilya
--
2014-10-13  Ilya Enkovich  <ilya.enkovich@intel.com>

	* tree-chkp.c (MAX_STMTS_IN_STATIC_CHKP_CTOR): New.
	(chkp_ctor_stmt_list): New.
	(chkp_register_var_initializer): New.
	(chkp_add_modification_to_stmt_list): New.
	(chkp_output_static_bounds): New.
	(chkp_finish_file): New.
	(chkp_instrument_function): Remove useless statements
	from static bounds constructors.
	* tree-chkp.h (chkp_register_var_initializer): New.
	(chkp_finish_file): New.
	* doc/invoke.texi (chkp-max-ctor-size): New.
	* params.def (PARAM_CHKP_MAX_CTOR_SIZE): New.


diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 1d8ab03..8128dff 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10459,6 +10459,12 @@ is greater or equal to this number, use callbacks instead of inline checks.
 E.g. to disable inline code use
 @option{--param asan-instrumentation-with-call-threshold=0}.
 
+@item chkp-max-ctor-size
+Static constructors generated by Pointer Bounds Checker may become very
+large and significantly increase compile time at optimization level
+@option{-O1} and higher.  This parameter is a maximum nubmer of statements
+in a single generated constructor.  Default value is 5000.
+
 @end table
 @end table
 
diff --git a/gcc/params.def b/gcc/params.def
index aefdd07..af490e0 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1099,6 +1099,12 @@ DEFPARAM (PARAM_UNINIT_CONTROL_DEP_ATTEMPTS,
 	  "Maximum number of nested calls to search for control dependencies "
 	  "during uninitialized variable analysis",
 	  1000, 1, 0)
+
+DEFPARAM (PARAM_CHKP_MAX_CTOR_SIZE,
+	  "chkp-max-ctor-size",
+	  "Maximum number of statements to be included into a single static "
+	  "constructor generated by Pointer Bounds Checker",
+	  5000, 100, 0)
 /*
 
 Local variables:
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 0abe192..21c6138 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -65,6 +65,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "rtl.h" /* For MEM_P, assign_temp.  */
 #include "tree-dfa.h"
 #include "ipa-chkp.h"
+#include "params.h"
 
 /*  Pointer Bounds Checker instruments code with memory checks to find
     out-of-bounds memory accesses.  Checks are performed by computing
@@ -394,6 +395,27 @@ static bool in_chkp_pass;
 #define CHKP_ZERO_BOUNDS_VAR_NAME "__chkp_zero_bounds"
 #define CHKP_NONE_BOUNDS_VAR_NAME "__chkp_none_bounds"
 
+/* Static checker constructors may become very large and their
+   compilation with optimization may take too much time.
+   Therefore we put a limit to number of statements in one
+   constructor.  Tests with 100 000 statically initialized
+   pointers showed following compilation times on Sandy Bridge
+   server (used -O2):
+   limit    100 => ~18 sec.
+   limit    300 => ~22 sec.
+   limit   1000 => ~30 sec.
+   limit   3000 => ~49 sec.
+   limit   5000 => ~55 sec.
+   limit  10000 => ~76 sec.
+   limit 100000 => ~532 sec.  */
+#define MAX_STMTS_IN_STATIC_CHKP_CTOR (PARAM_VALUE (PARAM_CHKP_MAX_CTOR_SIZE))
+
+struct chkp_ctor_stmt_list
+{
+  tree stmts;
+  int avail;
+};
+
 /* Return 1 if function FNDECL is instrumented by Pointer
    Bounds Checker.  */
 bool
@@ -871,6 +893,53 @@ chkp_set_bounds (tree node, tree val)
   chkp_bounds_map->put (node, val);
 }
 
+/* Check if statically initialized variable VAR require
+   static bounds initialization.  If VAR is added into
+   bounds initlization list then 1 is returned. Otherwise
+   return 0.  */
+extern bool
+chkp_register_var_initializer (tree var)
+{
+  if (!flag_check_pointer_bounds)
+    return false;
+
+  gcc_assert (TREE_CODE (var) == VAR_DECL);
+  gcc_assert (DECL_INITIAL (var)
+	      && DECL_INITIAL (var) != error_mark_node);
+
+  if (TREE_STATIC (var)
+      && chkp_type_has_pointer (TREE_TYPE (var)))
+    {
+      varpool_node::get_create (var)->need_bounds_init = 1;
+      return true;
+    }
+
+  return false;
+}
+
+/* Helper function for chkp_finish_file.
+
+   Add new modification statement (RHS is assigned to LHS)
+   into list of static initializer statementes (passed in ARG).
+   If statements list becomes too big, emit checker constructor
+   and start the new one.  */
+static void
+chkp_add_modification_to_stmt_list (tree lhs,
+				    tree rhs,
+				    void *arg)
+{
+  struct chkp_ctor_stmt_list *stmts = (struct chkp_ctor_stmt_list *)arg;
+  tree modify;
+
+  if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
+    rhs = build1 (CONVERT_EXPR, TREE_TYPE (lhs), rhs);
+
+  modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, rhs);
+  append_to_statement_list (modify, &stmts->stmts);
+
+  stmts->avail--;
+}
+
 /* Build and return ADDR_EXPR for specified object OBJ.  */
 static tree
 chkp_build_addr_expr (tree obj)
@@ -880,6 +949,64 @@ chkp_build_addr_expr (tree obj)
     : build_fold_addr_expr (obj);
 }
 
+/* Helper function for chkp_finish_file.
+   Initialize bound variable BND_VAR with bounds of variable
+   VAR to statements list STMTS.  If statements list becomes
+   too big, emit checker constructor and start the new one.  */
+static void
+chkp_output_static_bounds (tree bnd_var, tree var,
+			   struct chkp_ctor_stmt_list *stmts)
+{
+  tree lb, ub, size;
+
+  if (TREE_CODE (var) == STRING_CST)
+    {
+      lb = build1 (CONVERT_EXPR, size_type_node, chkp_build_addr_expr (var));
+      size = build_int_cst (size_type_node, TREE_STRING_LENGTH (var) - 1);
+    }
+  else if (DECL_SIZE (var)
+	   && !chkp_variable_size_type (TREE_TYPE (var)))
+    {
+      /* Compute bounds using statically known size.  */
+      lb = build1 (CONVERT_EXPR, size_type_node, chkp_build_addr_expr (var));
+      size = size_binop (MINUS_EXPR, DECL_SIZE_UNIT (var), size_one_node);
+    }
+  else
+    {
+      /* Compute bounds using dynamic size.  */
+      tree call;
+
+      lb = build1 (CONVERT_EXPR, size_type_node, chkp_build_addr_expr (var));
+      call = build1 (ADDR_EXPR,
+		     build_pointer_type (TREE_TYPE (chkp_sizeof_fndecl)),
+		     chkp_sizeof_fndecl);
+      size = build_call_nary (TREE_TYPE (TREE_TYPE (chkp_sizeof_fndecl)),
+			      call, 1, var);
+
+      if (flag_chkp_zero_dynamic_size_as_infinite)
+	{
+	  tree max_size, cond;
+
+	  max_size = build2 (MINUS_EXPR, size_type_node, size_zero_node, lb);
+	  cond = build2 (NE_EXPR, boolean_type_node, size, size_zero_node);
+	  size = build3 (COND_EXPR, size_type_node, cond, size, max_size);
+	}
+
+      size = size_binop (MINUS_EXPR, size, size_one_node);
+    }
+
+  ub = size_binop (PLUS_EXPR, lb, size);
+  stmts->avail -= targetm.chkp_initialize_bounds (bnd_var, lb, ub,
+						  &stmts->stmts);
+  if (stmts->avail <= 0)
+    {
+      cgraph_build_static_cdtor ('B', stmts->stmts,
+				 MAX_RESERVED_INIT_PRIORITY + 2);
+      stmts->avail = MAX_STMTS_IN_STATIC_CHKP_CTOR;
+      stmts->stmts = NULL;
+    }
+}
+
 /* Return entry block to be used for checker initilization code.
    Create new block if required.  */
 static basic_block
@@ -3402,6 +3529,74 @@ chkp_copy_bounds_for_elem (tree lhs, tree rhs, void *arg)
   chkp_build_bndstx (addr, rhs, bounds, iter);
 }
 
+/* Emit static bound initilizers and size vars.  */
+void
+chkp_finish_file (void)
+{
+  struct varpool_node *node;
+  struct chkp_ctor_stmt_list stmts;
+
+  if (seen_error ())
+    return;
+
+  /* Iterate through varpool and generate bounds initialization
+     constructors for all statically initialized pointers.  */
+  stmts.avail = MAX_STMTS_IN_STATIC_CHKP_CTOR;
+  stmts.stmts = NULL;
+  FOR_EACH_VARIABLE (node)
+    /* Check that var is actually emitted and we need and may initialize
+       its bounds.  */
+    if (node->need_bounds_init
+	&& !POINTER_BOUNDS_P (node->decl)
+	&& DECL_RTL (node->decl)
+	&& MEM_P (DECL_RTL (node->decl))
+	&& TREE_ASM_WRITTEN (node->decl))
+      {
+	chkp_walk_pointer_assignments (node->decl,
+				       DECL_INITIAL (node->decl),
+				       &stmts,
+				       chkp_add_modification_to_stmt_list);
+
+	if (stmts.avail <= 0)
+	  {
+	    cgraph_build_static_cdtor ('P', stmts.stmts,
+				       MAX_RESERVED_INIT_PRIORITY + 3);
+	    stmts.avail = MAX_STMTS_IN_STATIC_CHKP_CTOR;
+	    stmts.stmts = NULL;
+	  }
+      }
+
+  if (stmts.stmts)
+    cgraph_build_static_cdtor ('P', stmts.stmts,
+			       MAX_RESERVED_INIT_PRIORITY + 3);
+
+  /* Iterate through varpool and generate bounds initialization
+     constructors for all static bounds vars.  */
+  stmts.avail = MAX_STMTS_IN_STATIC_CHKP_CTOR;
+  stmts.stmts = NULL;
+  FOR_EACH_VARIABLE (node)
+    if (node->need_bounds_init
+	&& POINTER_BOUNDS_P (node->decl)
+	&& TREE_ASM_WRITTEN (node->decl))
+      {
+	tree bnd = node->decl;
+	tree var;
+
+	gcc_assert (DECL_INITIAL (bnd)
+		    && TREE_CODE (DECL_INITIAL (bnd)) == ADDR_EXPR);
+
+	var = TREE_OPERAND (DECL_INITIAL (bnd), 0);
+	chkp_output_static_bounds (bnd, var, &stmts);
+      }
+
+  if (stmts.stmts)
+    cgraph_build_static_cdtor ('B', stmts.stmts,
+			       MAX_RESERVED_INIT_PRIORITY + 2);
+
+  delete chkp_static_var_bounds;
+  delete chkp_bounds_map;
+}
+
 /* An instrumentation function which is called for each statement
    having memory access we want to instrument.  It inserts check
    code and bounds copy code.
@@ -3689,7 +3884,9 @@ chkp_replace_function_pointers (gimple_stmt_iterator *gsi)
 }
 
 /* This function instruments all statements working with memory,
-   calls and rets.  */
+   calls and rets.
+
+   It also removes excess statements from static initializers.  */
 static void
 chkp_instrument_function (void)
 {
@@ -3754,6 +3951,18 @@ chkp_instrument_function (void)
             }
 
 	  gsi_next (&i);
+
+	  /* We do not need any actual pointer stores in checker
+	     static initializer.  */
+	  if (lookup_attribute ("chkp ctor", DECL_ATTRIBUTES (cfun->decl))
+	      && gimple_code (s) == GIMPLE_ASSIGN
+	      && gimple_store_p (s))
+	    {
+	      gimple_stmt_iterator del_iter = gsi_for_stmt (s);
+	      gsi_remove (&del_iter, true);
+	      unlink_stmt_vdef (s);
+	      release_defs(s);
+	    }
         }
       bb = next;
     }
diff --git a/gcc/tree-chkp.h b/gcc/tree-chkp.h
index 36ffa51..1408ded 100644
--- a/gcc/tree-chkp.h
+++ b/gcc/tree-chkp.h
@@ -30,6 +30,8 @@ along with GCC; see the file COPYING3.  If not see
 
 extern tree chkp_get_bounds (tree node);
 extern void chkp_set_bounds (tree node, tree val);
+extern bool chkp_register_var_initializer (tree var);
+extern void chkp_finish_file (void);
 extern bool chkp_type_has_pointer (const_tree type);
 extern unsigned chkp_type_bounds_count (const_tree type);
 extern tree chkp_make_bounds_for_struct_addr (tree ptr);


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