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 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 ();
>> +}
>> +
>> +/* Helper function for ix86_load_bounds and ix86_store_bounds.
>> +
>> +   Return an address to be used to load/store bounds for pointer
>> +   passed in SLOT.
>> +
>> +   SLOT_NO is an integer constant holding number of a target
>> +   dependent special slot to be used in case SLOT is not a memory.
>> +
>> +   SPECIAL_BASE is a pointer to be used as a base of fake address
>> +   to access special slots in Bounds Table.  SPECIAL_BASE[-1],
>> +   SPECIAL_BASE[-2] etc. will be used as fake pointer locations.  */
>> +
>> +static rtx
>> +ix86_get_arg_address_for_bt (rtx slot, rtx slot_no, rtx special_base)
>> +{
>> +  rtx addr = NULL;
>> +
>> +  /* NULL slot means we pass bounds for pointer not passed to the
>> +     function at all.  Register slot means we pass pointer in a
>> +     register.  In both these cases bounds are passed via Bounds
>> +     Table.  Since we do not have actual pointer stored in memory,
>> +     we have to use fake addresses to access Bounds Table.  We
>> +     start with (special_base - sizeof (void*)) and decrease this
>> +     address by pointer size to get addresses for other slots.  */
>> +  if (!slot || REG_P (slot))
>> +    {
>> +      gcc_assert (CONST_INT_P (slot_no));
>> +      addr = plus_constant (Pmode, special_base,
>> +                           -(INTVAL (slot_no) + 1) * GET_MODE_SIZE (Pmode));
>> +    }
>> +  /* If pointer is passed in a memory then its address is used to
>> +     access Bounds Table.  */
>> +  else if (MEM_P (slot))
>> +    {
>> +      addr = XEXP (slot, 0);
>> +      if (!register_operand (addr, Pmode))
>> +       addr = copy_addr_to_reg (addr);
>> +    }
>> +  else
>> +    gcc_unreachable ();
>> +
>> +  return addr;
>> +}
>> +
>> +/* 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.  */
>> +
>> +static rtx
>> +ix86_load_bounds (rtx slot, rtx ptr, rtx slot_no)
>> +{
>> +  rtx reg = gen_reg_rtx (BNDmode);
>> +  rtx addr;
>> +
>> +  /* Get address to be used to access Bounds Table.  Special slots start
>> +     at the location of return address of the current function.  */
>> +  addr = ix86_get_arg_address_for_bt (slot, slot_no, arg_pointer_rtx);
>> +
>> +  /* Load pointer value from a memory if we don't have it.  */
>> +  if (!ptr)
>> +    {
>> +      gcc_assert (MEM_P (slot));
>> +      ptr = copy_addr_to_reg (slot);
>> +    }
>> +
>> +  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)
>> +{
>> +  rtx addr;
>> +
>> +  /* Get address to be used to access Bounds Table.  Special slots start
>> +     at the location of return address of a called function.  */
>> +  addr = ix86_get_arg_address_for_bt (slot, slot_no, stack_pointer_rtx);
>> +
>> +  /* Load pointer value from a memory if we don't have it.  */
>> +  if (!ptr)
>> +    {
>> +      gcc_assert (MEM_P (slot));
>> +      ptr = copy_addr_to_reg (slot);
>> +    }
>> +
>> +  gcc_assert (POINTER_BOUNDS_MODE_P (GET_MODE (bounds)));
>> +  if (!register_operand (bounds, BNDmode))
>> +    bounds = copy_to_mode_reg (BNDmode, bounds);
>> +
>> +  emit_insn (BNDmode == BND64mode
>> +            ? gen_bnd64_stx (addr, ptr, bounds)
>> +            : gen_bnd32_stx (addr, ptr, bounds));
>> +}
>> +
>> +/* Load and return bounds returned by function in SLOT.  */
>> +
>> +static rtx
>> +ix86_load_returned_bounds (rtx slot)
>> +{
>> +  rtx res;
>> +
>> +  gcc_assert (REG_P (slot));
>> +  res = gen_reg_rtx (BNDmode);
>> +  emit_move_insn (res, slot);
>> +
>> +  return res;
>> +}
>> +
>> +/* Store BOUNDS returned by function into SLOT.  */
>> +
>> +static void
>> +ix86_store_returned_bounds (rtx slot, rtx bounds)
>> +{
>> +  gcc_assert (REG_P (slot));
>> +  emit_move_insn (slot, bounds);
>> +}
>> +
>>  /* Returns a function decl for a vectorized version of the builtin function
>>     with builtin function code FN and the result vector type TYPE, or 
>> NULL_TREE
>>     if it is not available.  */
>> @@ -47480,6 +47701,61 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree 
>> *clear, tree *update)
>>                     atomic_feraiseexcept_call);
>>  }
>>
>> +static enum machine_mode
>> +ix86_mpx_bound_mode ()
>> +{
>> +  /* Do not support pointer checker if MPX
>> +     is not enabled.  */
>> +  if (!TARGET_MPX)
>> +    {
>> +      if (flag_check_pointer_bounds)
>> +       warning (0, "Pointer Checker requires MPX support on this target."
>> +                " Use -mmpx options to enable MPX.");
>> +      return VOIDmode;
>> +    }
>> +
>> +  return BNDmode;
>> +}
>> +
>> +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);
>> +}
>> +
>> +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;
>> +}
>> +
>>  /* Initialize the GCC target structure.  */
>>  #undef TARGET_RETURN_IN_MEMORY
>>  #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory
>> @@ -47897,6 +48173,33 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree 
>> *clear, tree *update)
>>  #undef TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS
>>  #define TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS true
>>
>> +#undef TARGET_LOAD_BOUNDS_FOR_ARG
>> +#define TARGET_LOAD_BOUNDS_FOR_ARG ix86_load_bounds
>> +
>> +#undef TARGET_STORE_BOUNDS_FOR_ARG
>> +#define TARGET_STORE_BOUNDS_FOR_ARG ix86_store_bounds
>> +
>> +#undef TARGET_LOAD_RETURNED_BOUNDS
>> +#define TARGET_LOAD_RETURNED_BOUNDS ix86_load_returned_bounds
>> +
>> +#undef TARGET_STORE_RETURNED_BOUNDS
>> +#define TARGET_STORE_RETURNED_BOUNDS ix86_store_returned_bounds
>> +
>> +#undef TARGET_CHKP_BOUND_MODE
>> +#define TARGET_CHKP_BOUND_MODE ix86_mpx_bound_mode
>> +
>> +#undef TARGET_BUILTIN_CHKP_FUNCTION
>> +#define TARGET_BUILTIN_CHKP_FUNCTION ix86_builtin_mpx_function
>> +
>> +#undef TARGET_CHKP_FUNCTION_VALUE_BOUNDS
>> +#define TARGET_CHKP_FUNCTION_VALUE_BOUNDS ix86_function_value_bounds
>> +
>> +#undef TARGET_CHKP_MAKE_BOUNDS_CONSTANT
>> +#define TARGET_CHKP_MAKE_BOUNDS_CONSTANT ix86_make_bounds_constant
>> +
>> +#undef TARGET_CHKP_INITIALIZE_BOUNDS
>> +#define TARGET_CHKP_INITIALIZE_BOUNDS ix86_initialize_bounds
>> +
>>  struct gcc_target targetm = TARGET_INITIALIZER;
>>
>>  #include "gt-i386.h"

Reply via email to