This is the mail archive of the 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 [4/n] Memory accesses instrumentation

On 10/08/14 13:01, Ilya Enkovich wrote:

This is the main chunk of instrumentation codes.  This patch introduces instrumentation pass which instruments memory accesses.

2014-10-08  Ilya Enkovich<>

	* tree-chkp.c (chkp_may_complete_phi_bounds): New.
	(chkp_may_finish_incomplete_bounds): New.
	(chkp_recompute_phi_bounds): New.
	(chkp_find_valid_phi_bounds): New.
	(chkp_finish_incomplete_bounds): New.
	(chkp_maybe_copy_and_register_bounds): New.
	(chkp_build_returned_bound): New.
	(chkp_get_bound_for_parm): New.
	(chkp_compute_bounds_for_assignment): New.
	(chkp_get_bounds_by_definition): New.
	(chkp_get_bounds_for_decl_addr): New.
	(chkp_get_bounds_for_string_cst): New.
	(chkp_parse_array_and_component_ref): New.
	(chkp_make_addressed_object_bounds): New.
	(chkp_find_bounds_1): New.
	(chkp_find_bounds): New.
	(chkp_find_bounds_loaded): New.
	(chkp_copy_bounds_for_elem): New.
	(chkp_process_stmt): New.
	(chkp_fix_cfg): New.
	(chkp_instrument_function): New.
	(chkp_fini): New.
	(chkp_execute): New.
	(chkp_gate): New.
	(pass_data_chkp): New.
	(pass_chkp): New.
	(make_pass_chkp): New.

@@ -491,6 +910,129 @@ chkp_get_bounds_var (tree ptr_var)
    return bnd_var;

+/* Register bounds BND for object PTR in global bounds table.
+   A copy of bounds may be created for abnormal ssa names.
+   Returns bounds to use for PTR.  */
+static tree
+chkp_maybe_copy_and_register_bounds (tree ptr, tree bnd)
+  bool abnormal_ptr;
+  if (!chkp_reg_bounds)
+    return bnd;
+  /* Do nothing if bounds are incomplete_bounds
+     because it means bounds will be recomputed.  */
+  if (bnd == incomplete_bounds)
+    return bnd;
+  abnormal_ptr = (TREE_CODE (ptr) == SSA_NAME
+		  && gimple_code (SSA_NAME_DEF_STMT (ptr)) != GIMPLE_PHI);
+  /* A single bounds value may be reused multiple times for
+     different pointer values.  It may cause coalescing issues
+     for abnormal SSA names.  To avoid it we create a bounds
+     copy in case it is copmputed for abnormal SSA name.

 +  if (!bounds)
+    {
+      tree orig_decl = cgraph_node::get (cfun->decl)->orig_decl;
+      /* For static chain param we return zero bounds
+	 because currently we do not check dereferences
+	 of this pointer.  */
+      /* ?? Is it a correct way to identify such parm?  */
+      if (cfun->decl && DECL_STATIC_CHAIN (cfun->decl)
+	  && DECL_ARTIFICIAL (decl))
+	bounds = chkp_get_zero_bounds ();
Are you just looking for the parameter in which we pass the static chain? Look at get_chain_decl for how we set it up. You may actually have to peek at more fields. I don't think there's a single magic bit that says "this is the static chain". Though it may always appear in the same location on the parameter list. Nested functions aren't something I'd poked with much. Richard Henderson might know more since he wrote tree-nested a while back.

@@ -1107,6 +1821,323 @@ chkp_build_bndstx (tree addr, tree ptr, tree bounds,

+/* Compute bounds for pointer NODE which was assigned in
+   assignment statement ASSIGN.  Return computed bounds.  */
+static tree
+chkp_compute_bounds_for_assignment (tree node, gimple assign)
Ugh. Note how this introduces another place that anyone who might add a new RHS gimple statement needs to edit. We need a pointer back to this code so that folks will know it needs updating. The question is where to put it.

Basically we want a place where anyone adding a new code that can appear on the RHS of an assignment must change already. Thoughts on a good location?

I realize there's probably many other places that probably need these kinds of documentation back links, I'm not asking you to address all of them.

+/* Compute and returne bounds for address of OBJ.  */

+/* Some code transformation made during instrumentation pass
+   may put code into inconsistent state.  Here we find and fix
+   such flaws.  */
+static void
+chkp_fix_cfg ()
Presumably none of the code you're inserting that causes these problems is ever supposed to be executed on the non-fallthru edge? Else your "creative" method of hiding the abnormal nature of the edge for a period of time, then recreating it won't work.

I'm a bit worried by this code and while I'll approve it, it's something we may have to come back and revisit if it causes problems.

So I think there's a couple typo nits to fix and one backlink doc issue to address and possibly a tweak to the code to identify static chains. With those fixed this should be good to go onto the trunk.


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