Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-24 Thread Ilya Enkovich 
2014-09-23 21:17 GMT+04:00 Jeff Law l...@redhat.com:
 On 09/23/14 08:10, Ilya Enkovich wrote:

 Please use fold_convert (size_ptr, build_fold_addr_expr (var)).

 Is 'var' always accessed via a size_t effective type?  Watch out
 for TBAA issues if not.  (if it is, why is 'var' not of type size_t
 or size_t[]?)


 var has pointer bounds type.  I have to initialize it by parts and
 thus access it as a couple of integers having size of a pointer (I
 use integer instead of pointer because non poiner arithmetic is
 used).  Size type is not the best for this purpose and therefore I
 replace it with pointer_sized_int_node.

 So I have accesses of var's parts as integers and accesses of whole
 var as bounds.  Should I expect some problems from TBAA here?  How
 can I avoid problems with TBAA if any exists?

 In general, anytime you access a hunk of memory using two different types,
 then you run the risk of problems with TBAA.  In the case of bounds, we
 aren't exposing them to usercode, so you just have to worry about the
 refs/sets that you create.

 I think you could create an alias set for the bounds and attach it to every
 load/store if you aren't type safe for all the loads/stores.  That will
 create a dependency between all the bounds loads/stores, but not with
 unrelated loads/stores.   Alternately ensure all the loads/stores are in
 alias set 0, but that will likely have performance implications.

I access parts of bounds using pointer_sized_int_node only in
constructors which initialize static bound variables.  These
constructors do not have other usages of these vars and all other
usages of these vars in other functions use bounds type for access.
That should make me safe from TBAA point of view.

Ilya


 Jeff

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-23 Thread Ilya Enkovich 
2014-09-22 22:51 GMT+04:00 Uros Bizjak ubiz...@gmail.com:
 On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich enkovich@gmail.com wrote:
 On 19 Sep 18:21, Uros Bizjak wrote:
 On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich enkovich@gmail.com 
 wrote:

   This patch adds i386 target hooks for Pointer Bounds Checker.

  New version with fixes and better documentation for ix86_load_bounds and 
  ix86_store_bounds is below.

  +/* Expand pass uses this hook to load bounds for function parameter
  +   PTR passed in SLOT in case its bounds are not passed in a register.
  +
  +   If SLOT is a memory, then bounds are loaded as for regular pointer
  +   loaded from memory.  PTR may be NULL in case SLOT is a memory.
  +   In such case value of PTR (if required) may be loaded from SLOT.
  +
  +   If SLOT is NULL or a register then SLOT_NO is an integer constant
  +   holding number of the target dependent special slot which should be
  +   used to obtain bounds.
  +
  +   Return loaded bounds.  */

 OK, I hope I understand this target-handling of SLOT_NO. Can you
 please clarify when SLOT is a register?

 For functions with more than four pointers passed in registers we do not 
 have enough bound registers to pass bounds.  These hooks are called then 
 with SLOT set to register used to pass pointer


 I propose to write this function in the following (hopefully equivalent) 
 way:

 Since addr computation is very similar for both loading and storing (the 
 only difference is usage of either arg_pointer_rtx or stack_pointer_rtx) I 
 decided additionally move it into a separate function.  This should make 
 functions simplier for understanding.

 LGTM, just add the explanation when NULL is returned.

There is no path in this function returning NULL (we are talking about
ix86_get_arg_address_for_bt, right?).

Ilya


 Here is an updated patch version.

 Thanks,
 Ilya
 --
 2014-09-22  Ilya Enkovich  ilya.enkov...@intel.com

 * config/i386/i386.c: Include tree-iterator.h.
 (ix86_function_value_bounds): New.
 (ix86_builtin_mpx_function): New.
 (ix86_get_arg_address_for_bt): New.
 (ix86_load_bounds): New.
 (ix86_store_bounds): New.
 (ix86_load_returned_bounds): New.
 (ix86_store_returned_bounds): New.
 (ix86_mpx_bound_mode): New.
 (ix86_make_bounds_constant): New.
 (ix86_initialize_bounds):
 (TARGET_LOAD_BOUNDS_FOR_ARG): New.
 (TARGET_STORE_BOUNDS_FOR_ARG): New.
 (TARGET_LOAD_RETURNED_BOUNDS): New.
 (TARGET_STORE_RETURNED_BOUNDS): New.
 (TARGET_CHKP_BOUND_MODE): New.
 (TARGET_BUILTIN_CHKP_FUNCTION): New.
 (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
 (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
 (TARGET_CHKP_INITIALIZE_BOUNDS): New.

 Assuming that tree part is OK (I have CC'd Richi for his opinion), the
 patch is OK.

 Thanks,
 Uros.



 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
 index d493983..8a3f577 100644
 --- a/gcc/config/i386/i386.c
 +++ b/gcc/config/i386/i386.c
 @@ -85,6 +85,7 @@ along with GCC; see the file COPYING3.  If not see
  #include tree-vectorizer.h
  #include shrink-wrap.h
  #include builtins.h
 +#include tree-iterator.h
  #include tree-chkp.h
  #include rtl-chkp.h

 @@ -8001,6 +8002,39 @@ ix86_function_value (const_tree valtype, const_tree 
 fntype_or_decl, bool)
return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode);
  }

 +static rtx
 +ix86_function_value_bounds (const_tree valtype,
 +   const_tree fntype_or_decl ATTRIBUTE_UNUSED,
 +   bool outgoing ATTRIBUTE_UNUSED)
 +{
 +  rtx res = NULL_RTX;
 +
 +  if (BOUNDED_TYPE_P (valtype))
 +res = gen_rtx_REG (BNDmode, FIRST_BND_REG);
 +  else if (chkp_type_has_pointer (valtype))
 +{
 +  bitmap slots = chkp_find_bound_slots (valtype);
 +  rtx bounds[2];
 +  bitmap_iterator bi;
 +  unsigned i, bnd_no = 0;
 +
 +  EXECUTE_IF_SET_IN_BITMAP (slots, 0, i, bi)
 +   {
 + rtx reg = gen_rtx_REG (BNDmode, FIRST_BND_REG + bnd_no);
 + rtx offs = GEN_INT (i * POINTER_SIZE / BITS_PER_UNIT);
 + gcc_assert (bnd_no  2);
 + bounds[bnd_no++] = gen_rtx_EXPR_LIST (VOIDmode, reg, offs);
 +   }
 +
 +  res = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (bnd_no, bounds));
 +  BITMAP_FREE (slots);
 +}
 +  else
 +res = NULL_RTX;
 +
 +  return res;
 +}
 +
  /* Pointer function arguments and return values are promoted to
 word_mode.  */

 @@ -36754,6 +36788,193 @@ static tree ix86_get_builtin (enum ix86_builtins 
 code)
  return NULL_TREE;
  }

 +/* Return function decl for target specific builtin
 +   for given MPX builtin passed i FCODE.  */
 +static tree
 +ix86_builtin_mpx_function (unsigned fcode)
 +{
 +  switch (fcode)
 +{
 +case BUILT_IN_CHKP_BNDMK:
 +  return ix86_builtins[IX86_BUILTIN_BNDMK];
 +
 +case BUILT_IN_CHKP_BNDSTX:
 +  return

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-23 Thread Uros Bizjak 
On Tue, Sep 23, 2014 at 8:48 AM, Ilya Enkovich enkovich@gmail.com wrote:
 2014-09-22 22:51 GMT+04:00 Uros Bizjak ubiz...@gmail.com:
 On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich enkovich@gmail.com 
 wrote:
 On 19 Sep 18:21, Uros Bizjak wrote:
 On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich enkovich@gmail.com 
 wrote:

   This patch adds i386 target hooks for Pointer Bounds Checker.

  New version with fixes and better documentation for ix86_load_bounds and 
  ix86_store_bounds is below.

  +/* Expand pass uses this hook to load bounds for function parameter
  +   PTR passed in SLOT in case its bounds are not passed in a register.
  +
  +   If SLOT is a memory, then bounds are loaded as for regular pointer
  +   loaded from memory.  PTR may be NULL in case SLOT is a memory.
  +   In such case value of PTR (if required) may be loaded from SLOT.
  +
  +   If SLOT is NULL or a register then SLOT_NO is an integer constant
  +   holding number of the target dependent special slot which should be
  +   used to obtain bounds.
  +
  +   Return loaded bounds.  */

 OK, I hope I understand this target-handling of SLOT_NO. Can you
 please clarify when SLOT is a register?

 For functions with more than four pointers passed in registers we do not 
 have enough bound registers to pass bounds.  These hooks are called then 
 with SLOT set to register used to pass pointer


 I propose to write this function in the following (hopefully equivalent) 
 way:

 Since addr computation is very similar for both loading and storing (the 
 only difference is usage of either arg_pointer_rtx or stack_pointer_rtx) I 
 decided additionally move it into a separate function.  This should make 
 functions simplier for understanding.

 LGTM, just add the explanation when NULL is returned.

 There is no path in this function returning NULL (we are talking about
 ix86_get_arg_address_for_bt, right?).

Oh, in fact, I was looking at ix86_function_value_bounds which doesn't
have comment at all... Looking a bit more, there are some other
functions without comments

Uros.

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-23 Thread Ilya Enkovich 
On 23 Sep 09:51, Richard Biener wrote:
 On Mon, 22 Sep 2014, Uros Bizjak wrote:
 
  On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich enkovich@gmail.com 
  wrote:
   On 19 Sep 18:21, Uros Bizjak wrote:
   On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich enkovich@gmail.com 
   wrote:
   +static int
   +ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts)
   +{
   +  tree size_ptr = build_pointer_type (size_type_node);
   +  tree lhs, modify, var_p;
   +
   +  ub = build1 (BIT_NOT_EXPR, size_type_node, ub);
   +  var_p = build1 (CONVERT_EXPR, size_ptr,
   + build_fold_addr_expr (var));
 
 Please use fold_convert (size_ptr, build_fold_addr_expr (var)).
 
 Is 'var' always accessed via a size_t effective type?  Watch out
 for TBAA issues if not.  (if it is, why is 'var' not of type
 size_t or size_t[]?)

var has pointer bounds type.  I have to initialize it by parts and thus access 
it as a couple of integers having size of a pointer (I use integer instead of 
pointer because non poiner arithmetic is used).  Size type is not the best for 
this purpose and therefore I replace it with pointer_sized_int_node.

So I have accesses of var's parts as integers and accesses of whole var as 
bounds.  Should I expect some problems from TBAA here?  How can I avoid 
problems with TBAA if any exists?

 
   +  lhs = build1 (INDIRECT_REF, size_type_node, var_p);
   +  modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, lb);
   +  append_to_statement_list (modify, stmts);
   +
   +  lhs = build1 (INDIRECT_REF, size_type_node,
   +   build2 (POINTER_PLUS_EXPR, size_ptr, var_p,
   +   TYPE_SIZE_UNIT (size_type_node)));
   +  modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, ub);
   +  append_to_statement_list (modify, stmts);
 
 This was used from inside the gimplifier?  Thus this is why you
 build GENERIC here and not GIMPLE?
 
 Didn't spot any other tree stuff.
 
 Richard.
 

I build GENERIC to pass it to cgraph_build_static_cdtor.  Below is a new 
version.

Thanks,
Ilya
--
2014-09-23  Ilya Enkovich  ilya.enkov...@intel.com

* config/i386/i386.c: Include tree-iterator.h.
(ix86_function_value_bounds): New.
(ix86_builtin_mpx_function): New.
(ix86_get_arg_address_for_bt): New.
(ix86_load_bounds): New.
(ix86_store_bounds): New.
(ix86_load_returned_bounds): New.
(ix86_store_returned_bounds): New.
(ix86_mpx_bound_mode): New.
(ix86_make_bounds_constant): New.
(ix86_initialize_bounds):
(TARGET_LOAD_BOUNDS_FOR_ARG): New.
(TARGET_STORE_BOUNDS_FOR_ARG): New.
(TARGET_LOAD_RETURNED_BOUNDS): New.
(TARGET_STORE_RETURNED_BOUNDS): New.
(TARGET_CHKP_BOUND_MODE): New.
(TARGET_BUILTIN_CHKP_FUNCTION): New.
(TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
(TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
(TARGET_CHKP_INITIALIZE_BOUNDS): New.


diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index d493983..64bf5b4 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -85,6 +85,7 @@ along with GCC; see the file COPYING3.  If not see
 #include tree-vectorizer.h
 #include shrink-wrap.h
 #include builtins.h
+#include tree-iterator.h
 #include tree-chkp.h
 #include rtl-chkp.h
 
@@ -8001,6 +8002,51 @@ ix86_function_value (const_tree valtype, const_tree 
fntype_or_decl, bool)
   return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode);
 }
 
+/*  Return an RTX representing a place where a function returns
+or recieves pointer bounds or NULL if no bounds are returned.
+
+VALTYPE is a data type of a value returned by the function.
+
+FN_DECL_OR_TYPE is a tree node representing FUNCTION_DECL
+or FUNCTION_TYPE of the function.
+
+If OUTGOING is false, return a place in which the caller will
+see the return value.  Otherwise, return a place where a
+function returns a value.  */
+
+static rtx
+ix86_function_value_bounds (const_tree valtype,
+   const_tree fntype_or_decl ATTRIBUTE_UNUSED,
+   bool outgoing ATTRIBUTE_UNUSED)
+{
+  rtx res = NULL_RTX;
+
+  if (BOUNDED_TYPE_P (valtype))
+res = gen_rtx_REG (BNDmode, FIRST_BND_REG);
+  else if (chkp_type_has_pointer (valtype))
+{
+  bitmap slots = chkp_find_bound_slots (valtype);
+  rtx bounds[2];
+  bitmap_iterator bi;
+  unsigned i, bnd_no = 0;
+
+  EXECUTE_IF_SET_IN_BITMAP (slots, 0, i, bi)
+   {
+ rtx reg = gen_rtx_REG (BNDmode, FIRST_BND_REG + bnd_no);
+ rtx offs = GEN_INT (i * POINTER_SIZE / BITS_PER_UNIT);
+ gcc_assert (bnd_no  2);
+ bounds[bnd_no++] = gen_rtx_EXPR_LIST (VOIDmode, reg, offs);
+   }
+
+  res = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (bnd_no, bounds));
+  BITMAP_FREE (slots);
+}
+  else
+res = NULL_RTX;
+
+  return res;
+}
+
 /* Pointer function arguments and return values are promoted to

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-23 Thread Richard Biener 
On Tue, 23 Sep 2014, Ilya Enkovich wrote:

 On 23 Sep 09:51, Richard Biener wrote:
  On Mon, 22 Sep 2014, Uros Bizjak wrote:
  
   On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich enkovich@gmail.com 
   wrote:
On 19 Sep 18:21, Uros Bizjak wrote:
On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich 
enkovich@gmail.com wrote:
+static int
+ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts)
+{
+  tree size_ptr = build_pointer_type (size_type_node);
+  tree lhs, modify, var_p;
+
+  ub = build1 (BIT_NOT_EXPR, size_type_node, ub);
+  var_p = build1 (CONVERT_EXPR, size_ptr,
+ build_fold_addr_expr (var));
  
  Please use fold_convert (size_ptr, build_fold_addr_expr (var)).
  
  Is 'var' always accessed via a size_t effective type?  Watch out
  for TBAA issues if not.  (if it is, why is 'var' not of type
  size_t or size_t[]?)
 
 var has pointer bounds type.  I have to initialize it by parts and thus 
 access it as a couple of integers having size of a pointer (I use 
 integer instead of pointer because non poiner arithmetic is used).  

But you use size_t which may not be of the size of pointers (what
do you do if a target has different address spaces with different
pointer sizes btw?).

 Size type is not the best for this purpose and therefore I replace it 
 with pointer_sized_int_node.

Ok, that should work.

 So I have accesses of var's parts as integers and accesses of whole var 
 as bounds.  Should I expect some problems from TBAA here?  How can I 
 avoid problems with TBAA if any exists?

What type is 'bounds'?  Yes, you should expect problems.  How
is 'bounds' type represented?

As a way out you can always use alias-set zero (but that might
be pessimizing), or you can use the alias set of 'bounds'.
For both the best thing is to build MEM_REFs instead of
INDIRECT_REFs and supply a zero offset of appropriate type
(build_int_cst (build_pointer_type (bounds_type), 0)).
You can then also omit casting of the pointer type you dereference.

Richard.

  
+  lhs = build1 (INDIRECT_REF, size_type_node, var_p);
+  modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, lb);
+  append_to_statement_list (modify, stmts);
+
+  lhs = build1 (INDIRECT_REF, size_type_node,
+   build2 (POINTER_PLUS_EXPR, size_ptr, var_p,
+   TYPE_SIZE_UNIT (size_type_node)));
+  modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, ub);
+  append_to_statement_list (modify, stmts);
  
  This was used from inside the gimplifier?  Thus this is why you
  build GENERIC here and not GIMPLE?
  
  Didn't spot any other tree stuff.
  
  Richard.
  
 
 I build GENERIC to pass it to cgraph_build_static_cdtor.  Below is a new 
 version.
 
 Thanks,
 Ilya
 --
 2014-09-23  Ilya Enkovich  ilya.enkov...@intel.com
 
   * config/i386/i386.c: Include tree-iterator.h.
   (ix86_function_value_bounds): New.
   (ix86_builtin_mpx_function): New.
   (ix86_get_arg_address_for_bt): New.
   (ix86_load_bounds): New.
   (ix86_store_bounds): New.
   (ix86_load_returned_bounds): New.
   (ix86_store_returned_bounds): New.
   (ix86_mpx_bound_mode): New.
   (ix86_make_bounds_constant): New.
   (ix86_initialize_bounds):
   (TARGET_LOAD_BOUNDS_FOR_ARG): New.
   (TARGET_STORE_BOUNDS_FOR_ARG): New.
   (TARGET_LOAD_RETURNED_BOUNDS): New.
   (TARGET_STORE_RETURNED_BOUNDS): New.
   (TARGET_CHKP_BOUND_MODE): New.
   (TARGET_BUILTIN_CHKP_FUNCTION): New.
   (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
   (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
   (TARGET_CHKP_INITIALIZE_BOUNDS): New.
 
 
 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
 index d493983..64bf5b4 100644
 --- a/gcc/config/i386/i386.c
 +++ b/gcc/config/i386/i386.c
 @@ -85,6 +85,7 @@ along with GCC; see the file COPYING3.  If not see
  #include tree-vectorizer.h
  #include shrink-wrap.h
  #include builtins.h
 +#include tree-iterator.h
  #include tree-chkp.h
  #include rtl-chkp.h
  
 @@ -8001,6 +8002,51 @@ ix86_function_value (const_tree valtype, const_tree 
 fntype_or_decl, bool)
return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode);
  }
  
 +/*  Return an RTX representing a place where a function returns
 +or recieves pointer bounds or NULL if no bounds are returned.
 +
 +VALTYPE is a data type of a value returned by the function.
 +
 +FN_DECL_OR_TYPE is a tree node representing FUNCTION_DECL
 +or FUNCTION_TYPE of the function.
 +
 +If OUTGOING is false, return a place in which the caller will
 +see the return value.  Otherwise, return a place where a
 +function returns a value.  */
 +
 +static rtx
 +ix86_function_value_bounds (const_tree valtype,
 + const_tree fntype_or_decl ATTRIBUTE_UNUSED,
 + bool outgoing ATTRIBUTE_UNUSED)
 +{
 +  rtx res = NULL_RTX;
 +
 +  if (BOUNDED_TYPE_P (valtype))
 +res =

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-23 Thread Jeff Law 

On 09/23/14 08:10, Ilya Enkovich wrote:

Please use fold_convert (size_ptr, build_fold_addr_expr (var)).

Is 'var' always accessed via a size_t effective type?  Watch out
for TBAA issues if not.  (if it is, why is 'var' not of type size_t
or size_t[]?)


var has pointer bounds type.  I have to initialize it by parts and
thus access it as a couple of integers having size of a pointer (I
use integer instead of pointer because non poiner arithmetic is
used).  Size type is not the best for this purpose and therefore I
replace it with pointer_sized_int_node.

So I have accesses of var's parts as integers and accesses of whole
var as bounds.  Should I expect some problems from TBAA here?  How
can I avoid problems with TBAA if any exists?
In general, anytime you access a hunk of memory using two different 
types, then you run the risk of problems with TBAA.  In the case of 
bounds, we aren't exposing them to usercode, so you just have to worry 
about the refs/sets that you create.


I think you could create an alias set for the bounds and attach it to 
every load/store if you aren't type safe for all the loads/stores.  That 
will create a dependency between all the bounds loads/stores, but not 
with unrelated loads/stores.   Alternately ensure all the loads/stores 
are in alias set 0, but that will likely have performance implications.


Jeff

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-22 Thread Ilya Enkovich 
On 19 Sep 18:21, Uros Bizjak wrote:
 On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich enkovich@gmail.com wrote:
 
   This patch adds i386 target hooks for Pointer Bounds Checker.
 
  New version with fixes and better documentation for ix86_load_bounds and 
  ix86_store_bounds is below.
 
  +/* Expand pass uses this hook to load bounds for function parameter
  +   PTR passed in SLOT in case its bounds are not passed in a register.
  +
  +   If SLOT is a memory, then bounds are loaded as for regular pointer
  +   loaded from memory.  PTR may be NULL in case SLOT is a memory.
  +   In such case value of PTR (if required) may be loaded from SLOT.
  +
  +   If SLOT is NULL or a register then SLOT_NO is an integer constant
  +   holding number of the target dependent special slot which should be
  +   used to obtain bounds.
  +
  +   Return loaded bounds.  */
 
 OK, I hope I understand this target-handling of SLOT_NO. Can you
 please clarify when SLOT is a register?

For functions with more than four pointers passed in registers we do not have 
enough bound registers to pass bounds.  These hooks are called then with SLOT 
set to register used to pass pointer

 
 I propose to write this function in the following (hopefully equivalent) way:

Since addr computation is very similar for both loading and storing (the only 
difference is usage of either arg_pointer_rtx or stack_pointer_rtx) I decided 
additionally move it into a separate function.  This should make functions 
simplier for understanding.

 
 --cut here--
 {
   if (!slot)
 {
   gcc_assert (CONST_INT_P (slot_no));
   addr = plus_constant (Pmode, arg_pointer_rtx,
 -(INTVAL (slot_no) + 1) * GET_MODE_SIZE (Pmode));
   gcc_assert (ptr);
 }
   else if (REG_P (slot))
 {
   gcc_assert (CONST_INT_P (slot_no));
   addr = plus_constant (Pmode, arg_pointer_rtx,
 -(INTVAL (slot_no) + 1) * GET_MODE_SIZE (Pmode));
   ptr = slot;
 }
   else if (MEM_P (slot))
 {
   addr = XEXP (slot, 0);
   if (!register_operand (addr, Pmode))
 addr = copy_addr_to_reg (addr);
 
   if (!ptr)
 ptr = copy_addr_to_reg (slot);
 }
   else
 gcc_unreachable ();
 
   if (!index_register_operand (ptr, Pmode))
 ptr = copy_addr_to_reg (ptr);
 
   ...
 }
 --cut here--
 
 Please add a comment in each of if/else, explaining what the code is
 doing. This is non-trivial to understand.
 
  +
  +static rtx
  +ix86_load_bounds (rtx slot, rtx ptr, rtx slot_no)
  +{
  +  rtx addr = NULL;
  +  rtx reg;
  +
  +  if (!ptr)
  +{
  +  gcc_assert (MEM_P (slot));
  +  ptr = copy_addr_to_reg (slot);
  +}
  +
  +  if (!slot || REG_P (slot))
  +{
  +  if (slot)
  +   ptr = slot;
  +
  +  gcc_assert (CONST_INT_P (slot_no));
  +
  +  /* Here we have the case when more than four pointers are
  +passed in registers.  In this case we are out of bound
  +registers and have to use bndldx to load bound.  RA,
  +RA - 8, etc. are used for address translation in bndldx.  */
  +  addr = plus_constant (Pmode, arg_pointer_rtx,
  +   -(INTVAL (slot_no) + 1) * GET_MODE_SIZE 
  (Pmode));
  +}
  +  else if (MEM_P (slot))
  +{
  +  addr = XEXP (slot, 0);
  +  if (!register_operand (addr, Pmode))
  +   addr = copy_addr_to_reg (addr);
  +}
  +  else
  +gcc_unreachable ();
  +
  +  if (!register_operand (ptr, Pmode))
  +ptr = copy_addr_to_reg (ptr);
  +
  +  reg = gen_reg_rtx (BNDmode);
  +  emit_insn (BNDmode == BND64mode
  +? gen_bnd64_ldx (reg, addr, ptr)
  +: gen_bnd32_ldx (reg, addr, ptr));
  +
  +  return reg;
  +}
  +
  +/* Expand pass uses this hook to store BOUNDS for call argument PTR
  +   passed in SLOT in case BOUNDS are not passed in a register.
  +
  +   If SLOT is a memory, then BOUNDS are stored as for regular pointer
  +   stored in memory.  PTR may be NULL in case SLOT is a memory.
  +   In such case value of PTR (if required) may be loaded from SLOT.
  +
  +   If SLOT is NULL or a register then SLOT_NO is an integer constant
  +   holding number of the target dependent special slot which should be
  +   used to store BOUNDS.  */
  +
  +static void
  +ix86_store_bounds (rtx ptr, rtx slot, rtx bounds, rtx slot_no)
 
 This function can be written in exactly the same way as the above
 proposed code, up to the check for ptr register_operand.
 
  +{
  +  rtx addr;
  +
  +  if (ptr)
  +{
  +  if (!register_operand (ptr, Pmode))
  +   ptr = copy_addr_to_reg (ptr);
  +}
  +  else
  +{
  +  gcc_assert (MEM_P (slot));
  +  ptr = copy_addr_to_reg (slot);
  +}
  +
  +  if (!slot || REG_P (slot))
  +{
  +  gcc_assert (CONST_INT_P (slot_no));
  +  addr = plus_constant (Pmode, stack_pointer_rtx,
  +   -(INTVAL (slot_no) + 1) * GET_MODE_SIZE 
  (Pmode));
  +}
  +  else if (MEM_P (slot))
  +addr = XEXP (slot, 0);

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-22 Thread Uros Bizjak 
On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich enkovich@gmail.com wrote:
 On 19 Sep 18:21, Uros Bizjak wrote:
 On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich enkovich@gmail.com 
 wrote:

   This patch adds i386 target hooks for Pointer Bounds Checker.

  New version with fixes and better documentation for ix86_load_bounds and 
  ix86_store_bounds is below.

  +/* Expand pass uses this hook to load bounds for function parameter
  +   PTR passed in SLOT in case its bounds are not passed in a register.
  +
  +   If SLOT is a memory, then bounds are loaded as for regular pointer
  +   loaded from memory.  PTR may be NULL in case SLOT is a memory.
  +   In such case value of PTR (if required) may be loaded from SLOT.
  +
  +   If SLOT is NULL or a register then SLOT_NO is an integer constant
  +   holding number of the target dependent special slot which should be
  +   used to obtain bounds.
  +
  +   Return loaded bounds.  */

 OK, I hope I understand this target-handling of SLOT_NO. Can you
 please clarify when SLOT is a register?

 For functions with more than four pointers passed in registers we do not have 
 enough bound registers to pass bounds.  These hooks are called then with SLOT 
 set to register used to pass pointer


 I propose to write this function in the following (hopefully equivalent) way:

 Since addr computation is very similar for both loading and storing (the only 
 difference is usage of either arg_pointer_rtx or stack_pointer_rtx) I decided 
 additionally move it into a separate function.  This should make functions 
 simplier for understanding.

LGTM, just add the explanation when NULL is returned.

 Here is an updated patch version.

 Thanks,
 Ilya
 --
 2014-09-22  Ilya Enkovich  ilya.enkov...@intel.com

 * config/i386/i386.c: Include tree-iterator.h.
 (ix86_function_value_bounds): New.
 (ix86_builtin_mpx_function): New.
 (ix86_get_arg_address_for_bt): New.
 (ix86_load_bounds): New.
 (ix86_store_bounds): New.
 (ix86_load_returned_bounds): New.
 (ix86_store_returned_bounds): New.
 (ix86_mpx_bound_mode): New.
 (ix86_make_bounds_constant): New.
 (ix86_initialize_bounds):
 (TARGET_LOAD_BOUNDS_FOR_ARG): New.
 (TARGET_STORE_BOUNDS_FOR_ARG): New.
 (TARGET_LOAD_RETURNED_BOUNDS): New.
 (TARGET_STORE_RETURNED_BOUNDS): New.
 (TARGET_CHKP_BOUND_MODE): New.
 (TARGET_BUILTIN_CHKP_FUNCTION): New.
 (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
 (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
 (TARGET_CHKP_INITIALIZE_BOUNDS): New.

Assuming that tree part is OK (I have CC'd Richi for his opinion), the
patch is OK.

Thanks,
Uros.



 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
 index d493983..8a3f577 100644
 --- a/gcc/config/i386/i386.c
 +++ b/gcc/config/i386/i386.c
 @@ -85,6 +85,7 @@ along with GCC; see the file COPYING3.  If not see
  #include tree-vectorizer.h
  #include shrink-wrap.h
  #include builtins.h
 +#include tree-iterator.h
  #include tree-chkp.h
  #include rtl-chkp.h

 @@ -8001,6 +8002,39 @@ ix86_function_value (const_tree valtype, const_tree 
 fntype_or_decl, bool)
return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode);
  }

 +static rtx
 +ix86_function_value_bounds (const_tree valtype,
 +   const_tree fntype_or_decl ATTRIBUTE_UNUSED,
 +   bool outgoing ATTRIBUTE_UNUSED)
 +{
 +  rtx res = NULL_RTX;
 +
 +  if (BOUNDED_TYPE_P (valtype))
 +res = gen_rtx_REG (BNDmode, FIRST_BND_REG);
 +  else if (chkp_type_has_pointer (valtype))
 +{
 +  bitmap slots = chkp_find_bound_slots (valtype);
 +  rtx bounds[2];
 +  bitmap_iterator bi;
 +  unsigned i, bnd_no = 0;
 +
 +  EXECUTE_IF_SET_IN_BITMAP (slots, 0, i, bi)
 +   {
 + rtx reg = gen_rtx_REG (BNDmode, FIRST_BND_REG + bnd_no);
 + rtx offs = GEN_INT (i * POINTER_SIZE / BITS_PER_UNIT);
 + gcc_assert (bnd_no  2);
 + bounds[bnd_no++] = gen_rtx_EXPR_LIST (VOIDmode, reg, offs);
 +   }
 +
 +  res = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (bnd_no, bounds));
 +  BITMAP_FREE (slots);
 +}
 +  else
 +res = NULL_RTX;
 +
 +  return res;
 +}
 +
  /* Pointer function arguments and return values are promoted to
 word_mode.  */

 @@ -36754,6 +36788,193 @@ static tree ix86_get_builtin (enum ix86_builtins 
 code)
  return NULL_TREE;
  }

 +/* Return function decl for target specific builtin
 +   for given MPX builtin passed i FCODE.  */
 +static tree
 +ix86_builtin_mpx_function (unsigned fcode)
 +{
 +  switch (fcode)
 +{
 +case BUILT_IN_CHKP_BNDMK:
 +  return ix86_builtins[IX86_BUILTIN_BNDMK];
 +
 +case BUILT_IN_CHKP_BNDSTX:
 +  return ix86_builtins[IX86_BUILTIN_BNDSTX];
 +
 +case BUILT_IN_CHKP_BNDLDX:
 +  return ix86_builtins[IX86_BUILTIN_BNDLDX];
 +
 +case BUILT_IN_CHKP_BNDCL:
 +  return

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-22 Thread Jeff Law 

On 09/19/14 10:21, Uros Bizjak wrote:


+static tree
+ix86_make_bounds_constant (HOST_WIDE_INT lb, HOST_WIDE_INT ub)
+{
+  tree low = lb ? build_minus_one_cst (pointer_sized_int_node)
+: build_zero_cst (pointer_sized_int_node);
+  tree high = ub ? build_zero_cst (pointer_sized_int_node)
+: build_minus_one_cst (pointer_sized_int_node);
+
+  /* This function is supposed to be used to create zero and
+ none bounds only.  */
+  gcc_assert (lb == 0 || lb == -1);
+  gcc_assert (ub == 0 || ub == -1);
+
+  return build_complex (NULL, low, high);
Needs a comment, even more so than normal given the restrictive nature 
of the input values.  One could bikeshed on using true/false vs 0/-1 
since it appears that lb/ub are really boolean values.




+}
+
+static int
+ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts)
+{
+  tree size_ptr = build_pointer_type (size_type_node);
+  tree lhs, modify, var_p;
+
+  ub = build1 (BIT_NOT_EXPR, size_type_node, ub);
+  var_p = build1 (CONVERT_EXPR, size_ptr,
+ build_fold_addr_expr (var));
+
+  lhs = build1 (INDIRECT_REF, size_type_node, var_p);
+  modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, lb);
+  append_to_statement_list (modify, stmts);
+
+  lhs = build1 (INDIRECT_REF, size_type_node,
+   build2 (POINTER_PLUS_EXPR, size_ptr, var_p,
+   TYPE_SIZE_UNIT (size_type_node)));
+  modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, ub);
+  append_to_statement_list (modify, stmts);
+
+  return 2;
+}
Are we supposed to be emitting gimple or generic here?  I don't think 
this is valid gimple as we have a CONVERT_EXPR embedded in the 
INDIRECT_REF's argument.


I think it's valid generic and does the obvious thing.  I just Ilya to 
make sure what we create does get gimplified.


jeff

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-19 Thread Ilya Enkovich 
On 18 Sep 21:09, Jeff Law wrote:
 On 09/18/14 13:34, Uros Bizjak wrote:
 2014-06-11 18:00 GMT+04:00 Ilya Enkovich enkovich@gmail.com:
 Hi,
 
 This patch adds i386 target hooks for Pointer Bounds Checker.
 
 Bootstrapped and tested on linux-x86_64.
 
 Thanks,
 Ilya
 --
 gcc/
 
 2014-06-11  Ilya Enkovich  ilya.enkov...@intel.com
 
  * config/i386/i386.c: Include tree-iterator.h.
  (ix86_function_value_bounds): New.
  (ix86_builtin_mpx_function): New.
  (ix86_load_bounds): New.
  (ix86_store_bounds): New.
  (ix86_load_returned_bounds): New.
  (ix86_store_returned_bounds): New.
  (ix86_fn_abi_va_list_bounds_size): New.
  (ix86_mpx_bound_mode): New.
  (ix86_make_bounds_constant): New.
  (ix86_initialize_bounds):
  (TARGET_LOAD_BOUNDS_FOR_ARG): New.
  (TARGET_STORE_BOUNDS_FOR_ARG): New.
  (TARGET_LOAD_RETURNED_BOUNDS): New.
  (TARGET_STORE_RETURNED_BOUNDS): New.
  (TARGET_CHKP_BOUND_MODE): New.
  (TARGET_BUILTIN_CHKP_FUNCTION): New.
  (TARGET_FN_ABI_VA_LIST_BOUNDS_SIZE): New.
  (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
  (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
  (TARGET_CHKP_INITIALIZE_BOUNDS): New.
 
 I have comments from the implementation side, but IMO Jeff (CC'd)
 should give the final approval on the functionality and general
 approach of the patch. I was not able to follow the meaning and logic
 behind SLOT (which may be register ?), PTR, TO, and special BND
 addresses.
 Most, if not all of the general principle has been approved,
 including the creation of the target hooks that Ilya wants to
 exploit in the x86 backend.
 
 Given that we've got two folks (Uros  myself) that have really
 struggled wrapping our heads around the docs for the new hooks,
 particularly those related to passing parameters  their bounds, I'd
 like Ilya to make another attempt to clarify the docs around those
 hooks.

I recall I worked on documentation of target hooks used to load/store bounds 
and believe I made it much cleaner.  I didn't duplicate new descriptions for 
i386 implementations though.  Thus I suppose Uros read old less informative 
descriptions in i386.c rather than better ones in tm.texi.  I copy more 
informative descriptions now to i386.c and hope it will be more clear what 
these functions do.

Thanks,
Ilya

 
 Uros, if you could go ahead and give your implementation side
 comments, it'd be appreciated.  I wouldn't expect major changes to
 the implementation to occur as a result of further iteration on the
 docs for the hooks.
 
 Thanks,
 Jeff

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-19 Thread Ilya Enkovich 
On 18 Sep 21:34, Uros Bizjak wrote:
 2014-06-11 18:00 GMT+04:00 Ilya Enkovich enkovich@gmail.com:
  Hi,
 
  This patch adds i386 target hooks for Pointer Bounds Checker.
 
  Bootstrapped and tested on linux-x86_64.
 
  Thanks,
  Ilya
  --
  gcc/
 
  2014-06-11  Ilya Enkovich  ilya.enkov...@intel.com
 
  * config/i386/i386.c: Include tree-iterator.h.
  (ix86_function_value_bounds): New.
  (ix86_builtin_mpx_function): New.
  (ix86_load_bounds): New.
  (ix86_store_bounds): New.
  (ix86_load_returned_bounds): New.
  (ix86_store_returned_bounds): New.
  (ix86_fn_abi_va_list_bounds_size): New.
  (ix86_mpx_bound_mode): New.
  (ix86_make_bounds_constant): New.
  (ix86_initialize_bounds):
  (TARGET_LOAD_BOUNDS_FOR_ARG): New.
  (TARGET_STORE_BOUNDS_FOR_ARG): New.
  (TARGET_LOAD_RETURNED_BOUNDS): New.
  (TARGET_STORE_RETURNED_BOUNDS): New.
  (TARGET_CHKP_BOUND_MODE): New.
  (TARGET_BUILTIN_CHKP_FUNCTION): New.
  (TARGET_FN_ABI_VA_LIST_BOUNDS_SIZE): New.
  (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
  (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
  (TARGET_CHKP_INITIALIZE_BOUNDS): New.
 
 I have comments from the implementation side, but IMO Jeff (CC'd)
 should give the final approval on the functionality and general
 approach of the patch. I was not able to follow the meaning and logic
 behind SLOT (which may be register ?), PTR, TO, and special BND
 addresses.
 
 Uros.
 
  +
  +  /* Here we have the case when more than four pointers are
  +passed in registers.  In this case we are out of bound
  +registers and have to use bndldx to load bound.  RA,
  +RA - 8, etc. are used for address translation in bndldx.  */
  +  addr = plus_constant (Pmode, arg_pointer_rtx, -(INTVAL (bnd) + 1) * 
  8);
 
 Magic value 8 refers to what?

This code is supposed to work for 64 bit target only and 8 is a size of Pmode.  
I'll replace this and other cases with mode size.

 
  +}
  +  else if (MEM_P (slot))
  +{
  +  addr = XEXP (slot, 0);
  +  addr = force_reg (Pmode, addr);
  +}
  +  else
  +gcc_unreachable ();
  +
  +  ptr = force_reg (Pmode, ptr);
  +  /* If ptr was a register originally then it may have
  + mode other than Pmode.  We need to extend in such
  + case because bndldx may work only with Pmode regs.  */
  +  if (GET_MODE (ptr) != Pmode)
  +{
  +  rtx ext = gen_rtx_ZERO_EXTEND (Pmode, ptr);
  +  ptr = gen_reg_rtx (Pmode);
  +  emit_move_insn (ptr, ext);
  +}
 
 Please use ix86_zero_extend_to_Pmode instead.
 
  +  reg = gen_reg_rtx (BNDmode);
  +  emit_insn (TARGET_64BIT
 
 Check for BNDmode here, as in patch 31/x.
 
  +? gen_bnd64_ldx (reg, addr, ptr)
  +: gen_bnd32_ldx (reg, addr, ptr));
  +
  +  return reg;
  +}
  +
  +/* Store bounds BOUNDS for PTR pointer value stored in
  +   specified ADDR.  If ADDR is a register then TO identifies
  +   which special address to use for bounds store.  */
  +static void
  +ix86_store_bounds (rtx ptr, rtx addr, rtx bounds, rtx to)
  +{
  +  if (!ptr)
  +{
  +  gcc_assert (MEM_P (addr));
  +  ptr = copy_to_mode_reg (Pmode, addr);
 
 copy_addr_to_reg
 
  +}
  +
  +  if (!addr || REG_P (addr))
  +{
  +  gcc_assert (CONST_INT_P (to));
  +  addr = plus_constant (Pmode, stack_pointer_rtx, -(INTVAL (to) + 1) * 
  8);
 
 What is magic number 8? Size of Pmode pointer, different between 32bit
 and 64bit targets?
 
  +}
  +  else if (MEM_P (addr))
  +addr = XEXP (addr, 0);
  +  else
  +gcc_unreachable ();
  +
  +  /* Should we also ignore integer modes of incorrect size?.  */
  +  ptr = force_reg (Pmode, ptr);
  +  addr = force_reg (Pmode, addr);
  +
  +  /* Avoid registers which connot be used as index.  */
  +  if (!index_register_operand (ptr, Pmode))
  +{
  +  rtx temp = gen_reg_rtx (Pmode);
  +  emit_move_insn (temp, ptr);
  +  ptr = temp;
  +}
 
 Please merge together handling of ptr. Something like:
 
 if (ptr)
 {
   if (!index_register_operand (ptr, Pmode))
 ptr = copy_addr_to_reg (ptr);
 }
 else
 {
   gcc_assert (MEM_P (addr))
   ptr = copy_addr_to_reg (addr);
 }
 
 if (!addr || REG_P (addr)
 {
 ...
 }
 
  +
  +  gcc_assert (POINTER_BOUNDS_MODE_P (GET_MODE (bounds)));
  +  bounds = force_reg (GET_MODE (bounds), bounds);
  +
  +  emit_insn (TARGET_64BIT
 
 Check BNDmode.
 
  +? gen_bnd64_stx (addr, ptr, bounds)
  +: gen_bnd32_stx (addr, ptr, bounds));

New version with fixes and better documentation for ix86_load_bounds and 
ix86_store_bounds is below.

Thanks,
Ilya
--
2014-09-18  Ilya Enkovich  ilya.enkov...@intel.com

* config/i386/i386.c: Include tree-iterator.h.
(ix86_function_value_bounds): New.
(ix86_builtin_mpx_function): New.
(ix86_load_bounds): New.
(ix86_store_bounds): New.

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-19 Thread Uros Bizjak 
On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich enkovich@gmail.com wrote:

  This patch adds i386 target hooks for Pointer Bounds Checker.

 New version with fixes and better documentation for ix86_load_bounds and 
 ix86_store_bounds is below.

 +/* Expand pass uses this hook to load bounds for function parameter
 +   PTR passed in SLOT in case its bounds are not passed in a register.
 +
 +   If SLOT is a memory, then bounds are loaded as for regular pointer
 +   loaded from memory.  PTR may be NULL in case SLOT is a memory.
 +   In such case value of PTR (if required) may be loaded from SLOT.
 +
 +   If SLOT is NULL or a register then SLOT_NO is an integer constant
 +   holding number of the target dependent special slot which should be
 +   used to obtain bounds.
 +
 +   Return loaded bounds.  */

OK, I hope I understand this target-handling of SLOT_NO. Can you
please clarify when SLOT is a register?

I propose to write this function in the following (hopefully equivalent) way:

--cut here--
{
  if (!slot)
{
  gcc_assert (CONST_INT_P (slot_no));
  addr = plus_constant (Pmode, arg_pointer_rtx,
-(INTVAL (slot_no) + 1) * GET_MODE_SIZE (Pmode));
  gcc_assert (ptr);
}
  else if (REG_P (slot))
{
  gcc_assert (CONST_INT_P (slot_no));
  addr = plus_constant (Pmode, arg_pointer_rtx,
-(INTVAL (slot_no) + 1) * GET_MODE_SIZE (Pmode));
  ptr = slot;
}
  else if (MEM_P (slot))
{
  addr = XEXP (slot, 0);
  if (!register_operand (addr, Pmode))
addr = copy_addr_to_reg (addr);

  if (!ptr)
ptr = copy_addr_to_reg (slot);
}
  else
gcc_unreachable ();

  if (!index_register_operand (ptr, Pmode))
ptr = copy_addr_to_reg (ptr);

  ...
}
--cut here--

Please add a comment in each of if/else, explaining what the code is
doing. This is non-trivial to understand.

 +
 +static rtx
 +ix86_load_bounds (rtx slot, rtx ptr, rtx slot_no)
 +{
 +  rtx addr = NULL;
 +  rtx reg;
 +
 +  if (!ptr)
 +{
 +  gcc_assert (MEM_P (slot));
 +  ptr = copy_addr_to_reg (slot);
 +}
 +
 +  if (!slot || REG_P (slot))
 +{
 +  if (slot)
 +   ptr = slot;
 +
 +  gcc_assert (CONST_INT_P (slot_no));
 +
 +  /* Here we have the case when more than four pointers are
 +passed in registers.  In this case we are out of bound
 +registers and have to use bndldx to load bound.  RA,
 +RA - 8, etc. are used for address translation in bndldx.  */
 +  addr = plus_constant (Pmode, arg_pointer_rtx,
 +   -(INTVAL (slot_no) + 1) * GET_MODE_SIZE (Pmode));
 +}
 +  else if (MEM_P (slot))
 +{
 +  addr = XEXP (slot, 0);
 +  if (!register_operand (addr, Pmode))
 +   addr = copy_addr_to_reg (addr);
 +}
 +  else
 +gcc_unreachable ();
 +
 +  if (!register_operand (ptr, Pmode))
 +ptr = copy_addr_to_reg (ptr);
 +
 +  reg = gen_reg_rtx (BNDmode);
 +  emit_insn (BNDmode == BND64mode
 +? gen_bnd64_ldx (reg, addr, ptr)
 +: gen_bnd32_ldx (reg, addr, ptr));
 +
 +  return reg;
 +}
 +
 +/* Expand pass uses this hook to store BOUNDS for call argument PTR
 +   passed in SLOT in case BOUNDS are not passed in a register.
 +
 +   If SLOT is a memory, then BOUNDS are stored as for regular pointer
 +   stored in memory.  PTR may be NULL in case SLOT is a memory.
 +   In such case value of PTR (if required) may be loaded from SLOT.
 +
 +   If SLOT is NULL or a register then SLOT_NO is an integer constant
 +   holding number of the target dependent special slot which should be
 +   used to store BOUNDS.  */
 +
 +static void
 +ix86_store_bounds (rtx ptr, rtx slot, rtx bounds, rtx slot_no)

This function can be written in exactly the same way as the above
proposed code, up to the check for ptr register_operand.

 +{
 +  rtx addr;
 +
 +  if (ptr)
 +{
 +  if (!register_operand (ptr, Pmode))
 +   ptr = copy_addr_to_reg (ptr);
 +}
 +  else
 +{
 +  gcc_assert (MEM_P (slot));
 +  ptr = copy_addr_to_reg (slot);
 +}
 +
 +  if (!slot || REG_P (slot))
 +{
 +  gcc_assert (CONST_INT_P (slot_no));
 +  addr = plus_constant (Pmode, stack_pointer_rtx,
 +   -(INTVAL (slot_no) + 1) * GET_MODE_SIZE (Pmode));
 +}
 +  else if (MEM_P (slot))
 +addr = XEXP (slot, 0);
 +  else
 +gcc_unreachable ();
 +
 +  if (!register_operand (addr, Pmode))
 +addr = copy_addr_to_reg (addr);

The above check is needed since addr handling in the gen_bndX_stx
expander (patch 2/n) is different that addr handling in gen_bndX_ldx.
This handling should be unified in a follow-up patch.

 +  /* Avoid registers which connot be used as index.  */
 +  if (!index_register_operand (ptr, Pmode))
 +{
 +  rtx temp = gen_reg_rtx (Pmode);
 +  emit_move_insn (temp, ptr);
 +  ptr = temp;
 +}
 +
 +  gcc_assert (POINTER_BOUNDS_MODE_P (GET_MODE (bounds)));
 +  if (!register_operand (bounds, BNDmode))
 +bounds =

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-18 Thread Uros Bizjak 
2014-06-11 18:00 GMT+04:00 Ilya Enkovich enkovich@gmail.com:
 Hi,

 This patch adds i386 target hooks for Pointer Bounds Checker.

 Bootstrapped and tested on linux-x86_64.

 Thanks,
 Ilya
 --
 gcc/

 2014-06-11  Ilya Enkovich  ilya.enkov...@intel.com

 * config/i386/i386.c: Include tree-iterator.h.
 (ix86_function_value_bounds): New.
 (ix86_builtin_mpx_function): New.
 (ix86_load_bounds): New.
 (ix86_store_bounds): New.
 (ix86_load_returned_bounds): New.
 (ix86_store_returned_bounds): New.
 (ix86_fn_abi_va_list_bounds_size): New.
 (ix86_mpx_bound_mode): New.
 (ix86_make_bounds_constant): New.
 (ix86_initialize_bounds):
 (TARGET_LOAD_BOUNDS_FOR_ARG): New.
 (TARGET_STORE_BOUNDS_FOR_ARG): New.
 (TARGET_LOAD_RETURNED_BOUNDS): New.
 (TARGET_STORE_RETURNED_BOUNDS): New.
 (TARGET_CHKP_BOUND_MODE): New.
 (TARGET_BUILTIN_CHKP_FUNCTION): New.
 (TARGET_FN_ABI_VA_LIST_BOUNDS_SIZE): New.
 (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
 (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
 (TARGET_CHKP_INITIALIZE_BOUNDS): New.

I have comments from the implementation side, but IMO Jeff (CC'd)
should give the final approval on the functionality and general
approach of the patch. I was not able to follow the meaning and logic
behind SLOT (which may be register ?), PTR, TO, and special BND
addresses.

Uros.

 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
 index ac79231..dac83d0 100644
 --- a/gcc/config/i386/i386.c
 +++ b/gcc/config/i386/i386.c
 @@ -81,6 +81,7 @@ along with GCC; see the file COPYING3.  If not see
  #include context.h
  #include pass_manager.h
  #include target-globals.h
 +#include tree-iterator.h
  #include tree-chkp.h
  #include rtl-chkp.h

 @@ -7971,6 +7972,39 @@ ix86_function_value (const_tree valtype, const_tree 
 fntype_or_decl,
return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode);
  }

 +static rtx
 +ix86_function_value_bounds (const_tree valtype,
 +   const_tree fntype_or_decl ATTRIBUTE_UNUSED,
 +   bool outgoing ATTRIBUTE_UNUSED)
 +{
 +  rtx res = NULL_RTX;
 +
 +  if (BOUNDED_TYPE_P (valtype))
 +res = gen_rtx_REG (BNDmode, FIRST_BND_REG);
 +  else if (chkp_type_has_pointer (valtype))
 +{
 +  bitmap slots = chkp_find_bound_slots (valtype);
 +  rtx bounds[2];
 +  bitmap_iterator bi;
 +  unsigned i, bnd_no = 0;
 +
 +  EXECUTE_IF_SET_IN_BITMAP (slots, 0, i, bi)
 +   {
 + rtx reg = gen_rtx_REG (BNDmode, FIRST_BND_REG + bnd_no);
 + rtx offs = GEN_INT (i * POINTER_SIZE / BITS_PER_UNIT);
 + gcc_assert (bnd_no  2);
 + bounds[bnd_no++] = gen_rtx_EXPR_LIST (VOIDmode, reg, offs);
 +   }
 +
 +  res = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (bnd_no, bounds));
 +  BITMAP_FREE (slots);
 +}
 +  else
 +res = NULL_RTX;
 +
 +  return res;
 +}
 +
  /* Pointer function arguments and return values are promoted to
 word_mode.  */

 @@ -36620,6 +36654,173 @@ static tree ix86_get_builtin (enum ix86_builtins 
 code)
  return NULL_TREE;
  }

 +/* Return function decl for target specific builtin
 +   for given MPX builtin passed i FCODE.  */
 +static tree
 +ix86_builtin_mpx_function (unsigned fcode)
 +{
 +  switch (fcode)
 +{
 +case BUILT_IN_CHKP_BNDMK:
 +  return ix86_builtins[IX86_BUILTIN_BNDMK];
 +
 +case BUILT_IN_CHKP_BNDSTX:
 +  return ix86_builtins[IX86_BUILTIN_BNDSTX];
 +
 +case BUILT_IN_CHKP_BNDLDX:
 +  return ix86_builtins[IX86_BUILTIN_BNDLDX];
 +
 +case BUILT_IN_CHKP_BNDCL:
 +  return ix86_builtins[IX86_BUILTIN_BNDCL];
 +
 +case BUILT_IN_CHKP_BNDCU:
 +  return ix86_builtins[IX86_BUILTIN_BNDCU];
 +
 +case BUILT_IN_CHKP_BNDRET:
 +  return ix86_builtins[IX86_BUILTIN_BNDRET];
 +
 +case BUILT_IN_CHKP_INTERSECT:
 +  return ix86_builtins[IX86_BUILTIN_BNDINT];
 +
 +case BUILT_IN_CHKP_NARROW:
 +  return ix86_builtins[IX86_BUILTIN_BNDNARROW];
 +
 +case BUILT_IN_CHKP_SIZEOF:
 +  return ix86_builtins[IX86_BUILTIN_SIZEOF];
 +
 +case BUILT_IN_CHKP_EXTRACT_LOWER:
 +  return ix86_builtins[IX86_BUILTIN_BNDLOWER];
 +
 +case BUILT_IN_CHKP_EXTRACT_UPPER:
 +  return ix86_builtins[IX86_BUILTIN_BNDUPPER];
 +
 +default:
 +  return NULL_TREE;
 +}
 +
 +  gcc_unreachable ();
 +}
 +
 +/* Load bounds PTR pointer value loaded from SLOT.
 +   if SLOT is a register then load bounds associated
 +   with special address identified by BND.
 +
 +   Return loaded bounds.  */
 +static rtx
 +ix86_load_bounds (rtx slot, rtx ptr, rtx bnd)
 +{
 +  rtx addr = NULL;
 +  rtx reg;
 +
 +  if (!ptr)
 +{
 +  gcc_assert (MEM_P (slot));
 +  ptr = copy_to_mode_reg (Pmode, slot);

copy_addr_to_reg

 +}
 +
 +  if (!slot || REG_P (slot))
 +{
 +  if (slot)
 +   ptr = slot;
 +
 +  gcc_assert (CONST_INT_P (bnd));

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-18 Thread Jeff Law 

On 09/18/14 13:34, Uros Bizjak wrote:

2014-06-11 18:00 GMT+04:00 Ilya Enkovich enkovich@gmail.com:

Hi,

This patch adds i386 target hooks for Pointer Bounds Checker.

Bootstrapped and tested on linux-x86_64.

Thanks,
Ilya
--
gcc/

2014-06-11  Ilya Enkovich  ilya.enkov...@intel.com

 * config/i386/i386.c: Include tree-iterator.h.
 (ix86_function_value_bounds): New.
 (ix86_builtin_mpx_function): New.
 (ix86_load_bounds): New.
 (ix86_store_bounds): New.
 (ix86_load_returned_bounds): New.
 (ix86_store_returned_bounds): New.
 (ix86_fn_abi_va_list_bounds_size): New.
 (ix86_mpx_bound_mode): New.
 (ix86_make_bounds_constant): New.
 (ix86_initialize_bounds):
 (TARGET_LOAD_BOUNDS_FOR_ARG): New.
 (TARGET_STORE_BOUNDS_FOR_ARG): New.
 (TARGET_LOAD_RETURNED_BOUNDS): New.
 (TARGET_STORE_RETURNED_BOUNDS): New.
 (TARGET_CHKP_BOUND_MODE): New.
 (TARGET_BUILTIN_CHKP_FUNCTION): New.
 (TARGET_FN_ABI_VA_LIST_BOUNDS_SIZE): New.
 (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
 (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
 (TARGET_CHKP_INITIALIZE_BOUNDS): New.


I have comments from the implementation side, but IMO Jeff (CC'd)
should give the final approval on the functionality and general
approach of the patch. I was not able to follow the meaning and logic
behind SLOT (which may be register ?), PTR, TO, and special BND
addresses.
Most, if not all of the general principle has been approved, including 
the creation of the target hooks that Ilya wants to exploit in the x86 
backend.


Given that we've got two folks (Uros  myself) that have really 
struggled wrapping our heads around the docs for the new hooks, 
particularly those related to passing parameters  their bounds, I'd 
like Ilya to make another attempt to clarify the docs around those hooks.


Uros, if you could go ahead and give your implementation side comments, 
it'd be appreciated.  I wouldn't expect major changes to the 
implementation to occur as a result of further iteration on the docs for 
the hooks.


Thanks,
Jeff

Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

2014-09-15 Thread Ilya Enkovich 
Ping

2014-06-11 18:00 GMT+04:00 Ilya Enkovich enkovich@gmail.com:
 Hi,

 This patch adds i386 target hooks for Pointer Bounds Checker.

 Bootstrapped and tested on linux-x86_64.

 Thanks,
 Ilya
 --
 gcc/

 2014-06-11  Ilya Enkovich  ilya.enkov...@intel.com

 * config/i386/i386.c: Include tree-iterator.h.
 (ix86_function_value_bounds): New.
 (ix86_builtin_mpx_function): New.
 (ix86_load_bounds): New.
 (ix86_store_bounds): New.
 (ix86_load_returned_bounds): New.
 (ix86_store_returned_bounds): New.
 (ix86_fn_abi_va_list_bounds_size): New.
 (ix86_mpx_bound_mode): New.
 (ix86_make_bounds_constant): New.
 (ix86_initialize_bounds):
 (TARGET_LOAD_BOUNDS_FOR_ARG): New.
 (TARGET_STORE_BOUNDS_FOR_ARG): New.
 (TARGET_LOAD_RETURNED_BOUNDS): New.
 (TARGET_STORE_RETURNED_BOUNDS): New.
 (TARGET_CHKP_BOUND_MODE): New.
 (TARGET_BUILTIN_CHKP_FUNCTION): New.
 (TARGET_FN_ABI_VA_LIST_BOUNDS_SIZE): New.
 (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New.
 (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New.
 (TARGET_CHKP_INITIALIZE_BOUNDS): New.


 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
 index ac79231..dac83d0 100644
 --- a/gcc/config/i386/i386.c
 +++ b/gcc/config/i386/i386.c
 @@ -81,6 +81,7 @@ along with GCC; see the file COPYING3.  If not see
  #include context.h
  #include pass_manager.h
  #include target-globals.h
 +#include tree-iterator.h
  #include tree-chkp.h
  #include rtl-chkp.h

 @@ -7971,6 +7972,39 @@ ix86_function_value (const_tree valtype, const_tree 
 fntype_or_decl,
return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode);
  }

 +static rtx
 +ix86_function_value_bounds (const_tree valtype,
 +   const_tree fntype_or_decl ATTRIBUTE_UNUSED,
 +   bool outgoing ATTRIBUTE_UNUSED)
 +{
 +  rtx res = NULL_RTX;
 +
 +  if (BOUNDED_TYPE_P (valtype))
 +res = gen_rtx_REG (BNDmode, FIRST_BND_REG);
 +  else if (chkp_type_has_pointer (valtype))
 +{
 +  bitmap slots = chkp_find_bound_slots (valtype);
 +  rtx bounds[2];
 +  bitmap_iterator bi;
 +  unsigned i, bnd_no = 0;
 +
 +  EXECUTE_IF_SET_IN_BITMAP (slots, 0, i, bi)
 +   {
 + rtx reg = gen_rtx_REG (BNDmode, FIRST_BND_REG + bnd_no);
 + rtx offs = GEN_INT (i * POINTER_SIZE / BITS_PER_UNIT);
 + gcc_assert (bnd_no  2);
 + bounds[bnd_no++] = gen_rtx_EXPR_LIST (VOIDmode, reg, offs);
 +   }
 +
 +  res = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (bnd_no, bounds));
 +  BITMAP_FREE (slots);
 +}
 +  else
 +res = NULL_RTX;
 +
 +  return res;
 +}
 +
  /* Pointer function arguments and return values are promoted to
 word_mode.  */

 @@ -36620,6 +36654,173 @@ static tree ix86_get_builtin (enum ix86_builtins 
 code)
  return NULL_TREE;
  }

 +/* Return function decl for target specific builtin
 +   for given MPX builtin passed i FCODE.  */
 +static tree
 +ix86_builtin_mpx_function (unsigned fcode)
 +{
 +  switch (fcode)
 +{
 +case BUILT_IN_CHKP_BNDMK:
 +  return ix86_builtins[IX86_BUILTIN_BNDMK];
 +
 +case BUILT_IN_CHKP_BNDSTX:
 +  return ix86_builtins[IX86_BUILTIN_BNDSTX];
 +
 +case BUILT_IN_CHKP_BNDLDX:
 +  return ix86_builtins[IX86_BUILTIN_BNDLDX];
 +
 +case BUILT_IN_CHKP_BNDCL:
 +  return ix86_builtins[IX86_BUILTIN_BNDCL];
 +
 +case BUILT_IN_CHKP_BNDCU:
 +  return ix86_builtins[IX86_BUILTIN_BNDCU];
 +
 +case BUILT_IN_CHKP_BNDRET:
 +  return ix86_builtins[IX86_BUILTIN_BNDRET];
 +
 +case BUILT_IN_CHKP_INTERSECT:
 +  return ix86_builtins[IX86_BUILTIN_BNDINT];
 +
 +case BUILT_IN_CHKP_NARROW:
 +  return ix86_builtins[IX86_BUILTIN_BNDNARROW];
 +
 +case BUILT_IN_CHKP_SIZEOF:
 +  return ix86_builtins[IX86_BUILTIN_SIZEOF];
 +
 +case BUILT_IN_CHKP_EXTRACT_LOWER:
 +  return ix86_builtins[IX86_BUILTIN_BNDLOWER];
 +
 +case BUILT_IN_CHKP_EXTRACT_UPPER:
 +  return ix86_builtins[IX86_BUILTIN_BNDUPPER];
 +
 +default:
 +  return NULL_TREE;
 +}
 +
 +  gcc_unreachable ();
 +}
 +
 +/* Load bounds PTR pointer value loaded from SLOT.
 +   if SLOT is a register then load bounds associated
 +   with special address identified by BND.
 +
 +   Return loaded bounds.  */
 +static rtx
 +ix86_load_bounds (rtx slot, rtx ptr, rtx bnd)
 +{
 +  rtx addr = NULL;
 +  rtx reg;
 +
 +  if (!ptr)
 +{
 +  gcc_assert (MEM_P (slot));
 +  ptr = copy_to_mode_reg (Pmode, slot);
 +}
 +
 +  if (!slot || REG_P (slot))
 +{
 +  if (slot)
 +   ptr = slot;
 +
 +  gcc_assert (CONST_INT_P (bnd));
 +
 +  /* Here we have the case when more than four pointers are
 +passed in registers.  In this case we are out of bound
 +registers and have to use bndldx to load bound.  RA,
 +RA - 8, etc. are used for address translation in bndldx.  */
 +  addr =