* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
@ 2014-09-18 19:34 Uros Bizjak
2014-09-19 3:10 ` Jeff Law
2014-09-19 12:53 ` Ilya Enkovich
0 siblings, 2 replies; 17+ messages in thread
From: Uros Bizjak @ 2014-09-18 19:34 UTC (permalink / raw)
To: gcc-patches; +Cc: Ilya Enkovich, Jeff Law
2014-06-11 18:00 GMT+04:00 Ilya Enkovich <enkovich.gnu@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.enkovich@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));
> +
> + /* 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?
> + }
> + 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));
> +}
> +
> +/* 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. */
> @@ -45796,6 +45997,22 @@ ix86_fn_abi_va_list (tree fndecl)
> return sysv_va_list_type_node;
> }
>
> +/* This function returns size of bounds for the calling abi
> + specific va_list node. */
> +
> +static tree
> +ix86_fn_abi_va_list_bounds_size (tree fndecl)
> +{
> + if (!TARGET_64BIT)
> + return integer_zero_node;
> + gcc_assert (fndecl != NULL_TREE);
> +
> + if (ix86_function_abi ((const_tree) fndecl) == MS_ABI)
> + return integer_zero_node;
> + else
> + return TYPE_SIZE (sysv_va_list_type_node);
> +}
> +
> /* Returns the canonical va_list type specified by TYPE. If there
> is no valid TYPE provided, it return NULL_TREE. */
>
> @@ -47246,6 +47463,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
> @@ -47642,6 +47914,36 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
> #define TARGET_FLOAT_EXCEPTIONS_ROUNDING_SUPPORTED_P \
> ix86_float_exceptions_rounding_supported_p
>
> +#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_FN_ABI_VA_LIST_BOUNDS_SIZE
> +#define TARGET_FN_ABI_VA_LIST_BOUNDS_SIZE ix86_fn_abi_va_list_bounds_size
> +
> +#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"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-18 19:34 [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target Uros Bizjak
@ 2014-09-19 3:10 ` Jeff Law
2014-09-19 12:16 ` Ilya Enkovich
2014-09-19 12:53 ` Ilya Enkovich
1 sibling, 1 reply; 17+ messages in thread
From: Jeff Law @ 2014-09-19 3:10 UTC (permalink / raw)
To: Uros Bizjak, gcc-patches; +Cc: Ilya Enkovich
On 09/18/14 13:34, Uros Bizjak wrote:
> 2014-06-11 18:00 GMT+04:00 Ilya Enkovich <enkovich.gnu@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.enkovich@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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-19 3:10 ` Jeff Law
@ 2014-09-19 12:16 ` Ilya Enkovich
0 siblings, 0 replies; 17+ messages in thread
From: Ilya Enkovich @ 2014-09-19 12:16 UTC (permalink / raw)
To: Jeff Law; +Cc: Uros Bizjak, gcc-patches
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.gnu@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.enkovich@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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-18 19:34 [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target Uros Bizjak
2014-09-19 3:10 ` Jeff Law
@ 2014-09-19 12:53 ` Ilya Enkovich
2014-09-19 16:21 ` Uros Bizjak
1 sibling, 1 reply; 17+ messages in thread
From: Ilya Enkovich @ 2014-09-19 12:53 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches, Jeff Law
On 18 Sep 21:34, Uros Bizjak wrote:
> 2014-06-11 18:00 GMT+04:00 Ilya Enkovich <enkovich.gnu@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.enkovich@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.enkovich@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_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..9549cb3 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 ();
+}
+
+/* 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 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)
+{
+ 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);
+
+ /* 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 = 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;
\f
#include "gt-i386.h"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-19 12:53 ` Ilya Enkovich
@ 2014-09-19 16:21 ` Uros Bizjak
2014-09-22 15:30 ` Ilya Enkovich
2014-09-22 18:54 ` Jeff Law
0 siblings, 2 replies; 17+ messages in thread
From: Uros Bizjak @ 2014-09-19 16:21 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: gcc-patches, Jeff Law
On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@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 = 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;
> +}
The above two functions are outside of my expertise. Although they
look trivial, I'd be more comfortable if a tree expert reviews them.
> +
> /* 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"
Uros.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-19 16:21 ` Uros Bizjak
@ 2014-09-22 15:30 ` Ilya Enkovich
2014-09-22 18:51 ` Uros Bizjak
2014-09-22 18:54 ` Jeff Law
1 sibling, 1 reply; 17+ messages in thread
From: Ilya Enkovich @ 2014-09-22 15:30 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches, Jeff Law
On 19 Sep 18:21, Uros Bizjak wrote:
> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@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);
> > + 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 = 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;
> > +}
>
> The above two functions are outside of my expertise. Although they
> look trivial, I'd be more comfortable if a tree expert reviews them.
>
> > +
> > /* 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"
>
> Uros.
Here is an updated patch version.
Thanks,
Ilya
--
2014-09-22 Ilya Enkovich <ilya.enkovich@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..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;
\f
#include "gt-i386.h"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-22 15:30 ` Ilya Enkovich
@ 2014-09-22 18:51 ` Uros Bizjak
2014-09-23 6:48 ` Ilya Enkovich
2014-09-23 7:55 ` Richard Biener
0 siblings, 2 replies; 17+ messages in thread
From: Uros Bizjak @ 2014-09-22 18:51 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: gcc-patches, Jeff Law, Richard Biener
On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 19 Sep 18:21, Uros Bizjak wrote:
>> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@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.enkovich@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"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-19 16:21 ` Uros Bizjak
2014-09-22 15:30 ` Ilya Enkovich
@ 2014-09-22 18:54 ` Jeff Law
1 sibling, 0 replies; 17+ messages in thread
From: Jeff Law @ 2014-09-22 18:54 UTC (permalink / raw)
To: Uros Bizjak, Ilya Enkovich; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-22 18:51 ` Uros Bizjak
@ 2014-09-23 6:48 ` Ilya Enkovich
2014-09-23 7:29 ` Uros Bizjak
2014-09-23 7:55 ` Richard Biener
1 sibling, 1 reply; 17+ messages in thread
From: Ilya Enkovich @ 2014-09-23 6:48 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches, Jeff Law, Richard Biener
2014-09-22 22:51 GMT+04:00 Uros Bizjak <ubizjak@gmail.com>:
> On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> On 19 Sep 18:21, Uros Bizjak wrote:
>>> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@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.enkovich@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"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-23 6:48 ` Ilya Enkovich
@ 2014-09-23 7:29 ` Uros Bizjak
0 siblings, 0 replies; 17+ messages in thread
From: Uros Bizjak @ 2014-09-23 7:29 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: gcc-patches, Jeff Law, Richard Biener
On Tue, Sep 23, 2014 at 8:48 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2014-09-22 22:51 GMT+04:00 Uros Bizjak <ubizjak@gmail.com>:
>> On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> On 19 Sep 18:21, Uros Bizjak wrote:
>>>> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-22 18:51 ` Uros Bizjak
2014-09-23 6:48 ` Ilya Enkovich
@ 2014-09-23 7:55 ` Richard Biener
2014-09-23 14:10 ` Ilya Enkovich
1 sibling, 1 reply; 17+ messages in thread
From: Richard Biener @ 2014-09-23 7:55 UTC (permalink / raw)
To: Uros Bizjak; +Cc: Ilya Enkovich, gcc-patches, Jeff Law
On Mon, 22 Sep 2014, Uros Bizjak wrote:
> On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> > On 19 Sep 18:21, Uros Bizjak wrote:
> >> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@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.enkovich@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);
> > }
> >
The function misses a comment.
> > +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);
> > }
> >
Function misses a comment.
> > +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;
> > +}
> > +
Likewise.
> > +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);
> > +}
> > +
Likewise.
> > +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[]?)
> > + 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.
> > + 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"
>
>
--
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-23 7:55 ` Richard Biener
@ 2014-09-23 14:10 ` Ilya Enkovich
2014-09-23 14:16 ` Richard Biener
2014-09-23 17:17 ` Jeff Law
0 siblings, 2 replies; 17+ messages in thread
From: Ilya Enkovich @ 2014-09-23 14:10 UTC (permalink / raw)
To: Richard Biener; +Cc: Uros Bizjak, gcc-patches, Jeff Law
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.gnu@gmail.com> wrote:
> > > On 19 Sep 18:21, Uros Bizjak wrote:
> > >> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@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.enkovich@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
word_mode. */
@@ -36754,6 +36800,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 +47713,73 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
atomic_feraiseexcept_call);
}
+/* Return mode to be used for bounds or VOIDmode
+ if bounds are not supported. */
+
+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;
+}
+
+/* Return constant used to statically initialize constant bounds.
+
+ This function is used to create special bound values. For now
+ only INIT bounds and NONE bounds are expected. More special
+ values may be added later. */
+
+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 INIT and
+ NONE bounds only. */
+ gcc_assert ((lb == 0 && ub == -1)
+ || (lb == -1 && ub == 0));
+
+ return build_complex (NULL, low, high);
+}
+
+/* Generate a list of statements STMTS to initialize pointer bounds
+ variable VAR with bounds LB and UB. Return the number of generated
+ statements. */
+
+static int
+ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts)
+{
+ tree bnd_ptr = build_pointer_type (pointer_sized_int_node);
+ tree lhs, modify, var_p;
+
+ ub = build1 (BIT_NOT_EXPR, pointer_sized_int_node, ub);
+ var_p = fold_convert (bnd_ptr, build_fold_addr_expr (var));
+
+ lhs = build1 (INDIRECT_REF, pointer_sized_int_node, var_p);
+ modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, lb);
+ append_to_statement_list (modify, stmts);
+
+ lhs = build1 (INDIRECT_REF, pointer_sized_int_node,
+ build2 (POINTER_PLUS_EXPR, bnd_ptr, var_p,
+ TYPE_SIZE_UNIT (pointer_sized_int_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 +48197,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;
\f
#include "gt-i386.h"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-23 14:10 ` Ilya Enkovich
@ 2014-09-23 14:16 ` Richard Biener
2014-09-23 17:17 ` Jeff Law
1 sibling, 0 replies; 17+ messages in thread
From: Richard Biener @ 2014-09-23 14:16 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: Uros Bizjak, gcc-patches, Jeff Law
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.gnu@gmail.com> wrote:
> > > > On 19 Sep 18:21, Uros Bizjak wrote:
> > > >> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@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.enkovich@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
> word_mode. */
>
> @@ -36754,6 +36800,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 +47713,73 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
> atomic_feraiseexcept_call);
> }
>
> +/* Return mode to be used for bounds or VOIDmode
> + if bounds are not supported. */
> +
> +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;
> +}
> +
> +/* Return constant used to statically initialize constant bounds.
> +
> + This function is used to create special bound values. For now
> + only INIT bounds and NONE bounds are expected. More special
> + values may be added later. */
> +
> +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 INIT and
> + NONE bounds only. */
> + gcc_assert ((lb == 0 && ub == -1)
> + || (lb == -1 && ub == 0));
> +
> + return build_complex (NULL, low, high);
> +}
> +
> +/* Generate a list of statements STMTS to initialize pointer bounds
> + variable VAR with bounds LB and UB. Return the number of generated
> + statements. */
> +
> +static int
> +ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts)
> +{
> + tree bnd_ptr = build_pointer_type (pointer_sized_int_node);
> + tree lhs, modify, var_p;
> +
> + ub = build1 (BIT_NOT_EXPR, pointer_sized_int_node, ub);
> + var_p = fold_convert (bnd_ptr, build_fold_addr_expr (var));
> +
> + lhs = build1 (INDIRECT_REF, pointer_sized_int_node, var_p);
> + modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, lb);
> + append_to_statement_list (modify, stmts);
> +
> + lhs = build1 (INDIRECT_REF, pointer_sized_int_node,
> + build2 (POINTER_PLUS_EXPR, bnd_ptr, var_p,
> + TYPE_SIZE_UNIT (pointer_sized_int_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 +48197,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;
> \f
> #include "gt-i386.h"
>
>
--
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-23 14:10 ` Ilya Enkovich
2014-09-23 14:16 ` Richard Biener
@ 2014-09-23 17:17 ` Jeff Law
2014-09-24 14:42 ` Ilya Enkovich
1 sibling, 1 reply; 17+ messages in thread
From: Jeff Law @ 2014-09-23 17:17 UTC (permalink / raw)
To: Ilya Enkovich, Richard Biener; +Cc: Uros Bizjak, gcc-patches
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-09-23 17:17 ` Jeff Law
@ 2014-09-24 14:42 ` Ilya Enkovich
0 siblings, 0 replies; 17+ messages in thread
From: Ilya Enkovich @ 2014-09-24 14:42 UTC (permalink / raw)
To: Jeff Law; +Cc: Richard Biener, Uros Bizjak, gcc-patches
2014-09-23 21:17 GMT+04:00 Jeff Law <law@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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
2014-06-11 14:01 Ilya Enkovich
@ 2014-09-15 7:17 ` Ilya Enkovich
0 siblings, 0 replies; 17+ messages in thread
From: Ilya Enkovich @ 2014-09-15 7:17 UTC (permalink / raw)
To: gcc-patches
Ping
2014-06-11 18:00 GMT+04:00 Ilya Enkovich <enkovich.gnu@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.enkovich@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 = plus_constant (Pmode, arg_pointer_rtx, -(INTVAL (bnd) + 1) * 8);
> + }
> + 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);
> + }
> +
> + reg = gen_reg_rtx (BNDmode);
> + emit_insn (TARGET_64BIT
> + ? 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);
> + }
> +
> + if (!addr || REG_P (addr))
> + {
> + gcc_assert (CONST_INT_P (to));
> + addr = plus_constant (Pmode, stack_pointer_rtx, -(INTVAL (to) + 1) * 8);
> + }
> + 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;
> + }
> +
> + gcc_assert (POINTER_BOUNDS_MODE_P (GET_MODE (bounds)));
> + bounds = force_reg (GET_MODE (bounds), bounds);
> +
> + emit_insn (TARGET_64BIT
> + ? 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. */
> @@ -45796,6 +45997,22 @@ ix86_fn_abi_va_list (tree fndecl)
> return sysv_va_list_type_node;
> }
>
> +/* This function returns size of bounds for the calling abi
> + specific va_list node. */
> +
> +static tree
> +ix86_fn_abi_va_list_bounds_size (tree fndecl)
> +{
> + if (!TARGET_64BIT)
> + return integer_zero_node;
> + gcc_assert (fndecl != NULL_TREE);
> +
> + if (ix86_function_abi ((const_tree) fndecl) == MS_ABI)
> + return integer_zero_node;
> + else
> + return TYPE_SIZE (sysv_va_list_type_node);
> +}
> +
> /* Returns the canonical va_list type specified by TYPE. If there
> is no valid TYPE provided, it return NULL_TREE. */
>
> @@ -47246,6 +47463,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
> @@ -47642,6 +47914,36 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
> #define TARGET_FLOAT_EXCEPTIONS_ROUNDING_SUPPORTED_P \
> ix86_float_exceptions_rounding_supported_p
>
> +#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_FN_ABI_VA_LIST_BOUNDS_SIZE
> +#define TARGET_FN_ABI_VA_LIST_BOUNDS_SIZE ix86_fn_abi_va_list_bounds_size
> +
> +#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"
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target
@ 2014-06-11 14:01 Ilya Enkovich
2014-09-15 7:17 ` Ilya Enkovich
0 siblings, 1 reply; 17+ messages in thread
From: Ilya Enkovich @ 2014-06-11 14:01 UTC (permalink / raw)
To: gcc-patches
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.enkovich@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 = plus_constant (Pmode, arg_pointer_rtx, -(INTVAL (bnd) + 1) * 8);
+ }
+ 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);
+ }
+
+ reg = gen_reg_rtx (BNDmode);
+ emit_insn (TARGET_64BIT
+ ? 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);
+ }
+
+ if (!addr || REG_P (addr))
+ {
+ gcc_assert (CONST_INT_P (to));
+ addr = plus_constant (Pmode, stack_pointer_rtx, -(INTVAL (to) + 1) * 8);
+ }
+ 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;
+ }
+
+ gcc_assert (POINTER_BOUNDS_MODE_P (GET_MODE (bounds)));
+ bounds = force_reg (GET_MODE (bounds), bounds);
+
+ emit_insn (TARGET_64BIT
+ ? 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. */
@@ -45796,6 +45997,22 @@ ix86_fn_abi_va_list (tree fndecl)
return sysv_va_list_type_node;
}
+/* This function returns size of bounds for the calling abi
+ specific va_list node. */
+
+static tree
+ix86_fn_abi_va_list_bounds_size (tree fndecl)
+{
+ if (!TARGET_64BIT)
+ return integer_zero_node;
+ gcc_assert (fndecl != NULL_TREE);
+
+ if (ix86_function_abi ((const_tree) fndecl) == MS_ABI)
+ return integer_zero_node;
+ else
+ return TYPE_SIZE (sysv_va_list_type_node);
+}
+
/* Returns the canonical va_list type specified by TYPE. If there
is no valid TYPE provided, it return NULL_TREE. */
@@ -47246,6 +47463,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
@@ -47642,6 +47914,36 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
#define TARGET_FLOAT_EXCEPTIONS_ROUNDING_SUPPORTED_P \
ix86_float_exceptions_rounding_supported_p
+#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_FN_ABI_VA_LIST_BOUNDS_SIZE
+#define TARGET_FN_ABI_VA_LIST_BOUNDS_SIZE ix86_fn_abi_va_list_bounds_size
+
+#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;
\f
#include "gt-i386.h"
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-09-24 14:42 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-18 19:34 [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target Uros Bizjak
2014-09-19 3:10 ` Jeff Law
2014-09-19 12:16 ` Ilya Enkovich
2014-09-19 12:53 ` Ilya Enkovich
2014-09-19 16:21 ` Uros Bizjak
2014-09-22 15:30 ` Ilya Enkovich
2014-09-22 18:51 ` Uros Bizjak
2014-09-23 6:48 ` Ilya Enkovich
2014-09-23 7:29 ` Uros Bizjak
2014-09-23 7:55 ` Richard Biener
2014-09-23 14:10 ` Ilya Enkovich
2014-09-23 14:16 ` Richard Biener
2014-09-23 17:17 ` Jeff Law
2014-09-24 14:42 ` Ilya Enkovich
2014-09-22 18:54 ` Jeff Law
-- strict thread matches above, loose matches on Subject: below --
2014-06-11 14:01 Ilya Enkovich
2014-09-15 7:17 ` Ilya Enkovich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).