2014-09-22 22:51 GMT+04:00 Uros Bizjak <ubiz...@gmail.com>: > On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich <enkovich....@gmail.com> wrote: >> On 19 Sep 18:21, Uros Bizjak wrote: >>> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich....@gmail.com> >>> wrote: >>> >>> >> > This patch adds i386 target hooks for Pointer Bounds Checker. >>> >>> > New version with fixes and better documentation for ix86_load_bounds and >>> > ix86_store_bounds is below. >>> >>> > +/* Expand pass uses this hook to load bounds for function parameter >>> > + PTR passed in SLOT in case its bounds are not passed in a register. >>> > + >>> > + If SLOT is a memory, then bounds are loaded as for regular pointer >>> > + loaded from memory. PTR may be NULL in case SLOT is a memory. >>> > + In such case value of PTR (if required) may be loaded from SLOT. >>> > + >>> > + If SLOT is NULL or a register then SLOT_NO is an integer constant >>> > + holding number of the target dependent special slot which should be >>> > + used to obtain bounds. >>> > + >>> > + Return loaded bounds. */ >>> >>> OK, I hope I understand this target-handling of SLOT_NO. Can you >>> please clarify when SLOT is a register? >> >> For functions with more than four pointers passed in registers we do not >> have enough bound registers to pass bounds. These hooks are called then >> with SLOT set to register used to pass pointer >> >>> >>> I propose to write this function in the following (hopefully equivalent) >>> way: >> >> Since addr computation is very similar for both loading and storing (the >> only difference is usage of either arg_pointer_rtx or stack_pointer_rtx) I >> decided additionally move it into a separate function. This should make >> functions simplier for understanding. > > LGTM, just add the explanation when NULL is returned.
There is no path in this function returning NULL (we are talking about ix86_get_arg_address_for_bt, right?). Ilya > >> Here is an updated patch version. >> >> Thanks, >> Ilya >> -- >> 2014-09-22 Ilya Enkovich <ilya.enkov...@intel.com> >> >> * config/i386/i386.c: Include tree-iterator.h. >> (ix86_function_value_bounds): New. >> (ix86_builtin_mpx_function): New. >> (ix86_get_arg_address_for_bt): New. >> (ix86_load_bounds): New. >> (ix86_store_bounds): New. >> (ix86_load_returned_bounds): New. >> (ix86_store_returned_bounds): New. >> (ix86_mpx_bound_mode): New. >> (ix86_make_bounds_constant): New. >> (ix86_initialize_bounds): >> (TARGET_LOAD_BOUNDS_FOR_ARG): New. >> (TARGET_STORE_BOUNDS_FOR_ARG): New. >> (TARGET_LOAD_RETURNED_BOUNDS): New. >> (TARGET_STORE_RETURNED_BOUNDS): New. >> (TARGET_CHKP_BOUND_MODE): New. >> (TARGET_BUILTIN_CHKP_FUNCTION): New. >> (TARGET_CHKP_FUNCTION_VALUE_BOUNDS): New. >> (TARGET_CHKP_MAKE_BOUNDS_CONSTANT): New. >> (TARGET_CHKP_INITIALIZE_BOUNDS): New. > > Assuming that tree part is OK (I have CC'd Richi for his opinion), the > patch is OK. > > Thanks, > Uros. > >> >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index d493983..8a3f577 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -85,6 +85,7 @@ along with GCC; see the file COPYING3. If not see >> #include "tree-vectorizer.h" >> #include "shrink-wrap.h" >> #include "builtins.h" >> +#include "tree-iterator.h" >> #include "tree-chkp.h" >> #include "rtl-chkp.h" >> >> @@ -8001,6 +8002,39 @@ ix86_function_value (const_tree valtype, const_tree >> fntype_or_decl, bool) >> return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode); >> } >> >> +static rtx >> +ix86_function_value_bounds (const_tree valtype, >> + const_tree fntype_or_decl ATTRIBUTE_UNUSED, >> + bool outgoing ATTRIBUTE_UNUSED) >> +{ >> + rtx res = NULL_RTX; >> + >> + if (BOUNDED_TYPE_P (valtype)) >> + res = gen_rtx_REG (BNDmode, FIRST_BND_REG); >> + else if (chkp_type_has_pointer (valtype)) >> + { >> + bitmap slots = chkp_find_bound_slots (valtype); >> + rtx bounds[2]; >> + bitmap_iterator bi; >> + unsigned i, bnd_no = 0; >> + >> + EXECUTE_IF_SET_IN_BITMAP (slots, 0, i, bi) >> + { >> + rtx reg = gen_rtx_REG (BNDmode, FIRST_BND_REG + bnd_no); >> + rtx offs = GEN_INT (i * POINTER_SIZE / BITS_PER_UNIT); >> + gcc_assert (bnd_no < 2); >> + bounds[bnd_no++] = gen_rtx_EXPR_LIST (VOIDmode, reg, offs); >> + } >> + >> + res = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (bnd_no, bounds)); >> + BITMAP_FREE (slots); >> + } >> + else >> + res = NULL_RTX; >> + >> + return res; >> +} >> + >> /* Pointer function arguments and return values are promoted to >> word_mode. */ >> >> @@ -36754,6 +36788,193 @@ static tree ix86_get_builtin (enum ix86_builtins >> code) >> return NULL_TREE; >> } >> >> +/* Return function decl for target specific builtin >> + for given MPX builtin passed i FCODE. */ >> +static tree >> +ix86_builtin_mpx_function (unsigned fcode) >> +{ >> + switch (fcode) >> + { >> + case BUILT_IN_CHKP_BNDMK: >> + return ix86_builtins[IX86_BUILTIN_BNDMK]; >> + >> + case BUILT_IN_CHKP_BNDSTX: >> + return ix86_builtins[IX86_BUILTIN_BNDSTX]; >> + >> + case BUILT_IN_CHKP_BNDLDX: >> + return ix86_builtins[IX86_BUILTIN_BNDLDX]; >> + >> + case BUILT_IN_CHKP_BNDCL: >> + return ix86_builtins[IX86_BUILTIN_BNDCL]; >> + >> + case BUILT_IN_CHKP_BNDCU: >> + return ix86_builtins[IX86_BUILTIN_BNDCU]; >> + >> + case BUILT_IN_CHKP_BNDRET: >> + return ix86_builtins[IX86_BUILTIN_BNDRET]; >> + >> + case BUILT_IN_CHKP_INTERSECT: >> + return ix86_builtins[IX86_BUILTIN_BNDINT]; >> + >> + case BUILT_IN_CHKP_NARROW: >> + return ix86_builtins[IX86_BUILTIN_BNDNARROW]; >> + >> + case BUILT_IN_CHKP_SIZEOF: >> + return ix86_builtins[IX86_BUILTIN_SIZEOF]; >> + >> + case BUILT_IN_CHKP_EXTRACT_LOWER: >> + return ix86_builtins[IX86_BUILTIN_BNDLOWER]; >> + >> + case BUILT_IN_CHKP_EXTRACT_UPPER: >> + return ix86_builtins[IX86_BUILTIN_BNDUPPER]; >> + >> + default: >> + return NULL_TREE; >> + } >> + >> + gcc_unreachable (); >> +} >> + >> +/* Helper function for ix86_load_bounds and ix86_store_bounds. >> + >> + Return an address to be used to load/store bounds for pointer >> + passed in SLOT. >> + >> + SLOT_NO is an integer constant holding number of a target >> + dependent special slot to be used in case SLOT is not a memory. >> + >> + SPECIAL_BASE is a pointer to be used as a base of fake address >> + to access special slots in Bounds Table. SPECIAL_BASE[-1], >> + SPECIAL_BASE[-2] etc. will be used as fake pointer locations. */ >> + >> +static rtx >> +ix86_get_arg_address_for_bt (rtx slot, rtx slot_no, rtx special_base) >> +{ >> + rtx addr = NULL; >> + >> + /* NULL slot means we pass bounds for pointer not passed to the >> + function at all. Register slot means we pass pointer in a >> + register. In both these cases bounds are passed via Bounds >> + Table. Since we do not have actual pointer stored in memory, >> + we have to use fake addresses to access Bounds Table. We >> + start with (special_base - sizeof (void*)) and decrease this >> + address by pointer size to get addresses for other slots. */ >> + if (!slot || REG_P (slot)) >> + { >> + gcc_assert (CONST_INT_P (slot_no)); >> + addr = plus_constant (Pmode, special_base, >> + -(INTVAL (slot_no) + 1) * GET_MODE_SIZE (Pmode)); >> + } >> + /* If pointer is passed in a memory then its address is used to >> + access Bounds Table. */ >> + else if (MEM_P (slot)) >> + { >> + addr = XEXP (slot, 0); >> + if (!register_operand (addr, Pmode)) >> + addr = copy_addr_to_reg (addr); >> + } >> + else >> + gcc_unreachable (); >> + >> + return addr; >> +} >> + >> +/* Expand pass uses this hook to load bounds for function parameter >> + PTR passed in SLOT in case its bounds are not passed in a register. >> + >> + If SLOT is a memory, then bounds are loaded as for regular pointer >> + loaded from memory. PTR may be NULL in case SLOT is a memory. >> + In such case value of PTR (if required) may be loaded from SLOT. >> + >> + If SLOT is NULL or a register then SLOT_NO is an integer constant >> + holding number of the target dependent special slot which should be >> + used to obtain bounds. >> + >> + Return loaded bounds. */ >> + >> +static rtx >> +ix86_load_bounds (rtx slot, rtx ptr, rtx slot_no) >> +{ >> + rtx reg = gen_reg_rtx (BNDmode); >> + rtx addr; >> + >> + /* Get address to be used to access Bounds Table. Special slots start >> + at the location of return address of the current function. */ >> + addr = ix86_get_arg_address_for_bt (slot, slot_no, arg_pointer_rtx); >> + >> + /* Load pointer value from a memory if we don't have it. */ >> + if (!ptr) >> + { >> + gcc_assert (MEM_P (slot)); >> + ptr = copy_addr_to_reg (slot); >> + } >> + >> + emit_insn (BNDmode == BND64mode >> + ? gen_bnd64_ldx (reg, addr, ptr) >> + : gen_bnd32_ldx (reg, addr, ptr)); >> + >> + return reg; >> +} >> + >> +/* Expand pass uses this hook to store BOUNDS for call argument PTR >> + passed in SLOT in case BOUNDS are not passed in a register. >> + >> + If SLOT is a memory, then BOUNDS are stored as for regular pointer >> + stored in memory. PTR may be NULL in case SLOT is a memory. >> + In such case value of PTR (if required) may be loaded from SLOT. >> + >> + If SLOT is NULL or a register then SLOT_NO is an integer constant >> + holding number of the target dependent special slot which should be >> + used to store BOUNDS. */ >> + >> +static void >> +ix86_store_bounds (rtx ptr, rtx slot, rtx bounds, rtx slot_no) >> +{ >> + rtx addr; >> + >> + /* Get address to be used to access Bounds Table. Special slots start >> + at the location of return address of a called function. */ >> + addr = ix86_get_arg_address_for_bt (slot, slot_no, stack_pointer_rtx); >> + >> + /* Load pointer value from a memory if we don't have it. */ >> + if (!ptr) >> + { >> + gcc_assert (MEM_P (slot)); >> + ptr = copy_addr_to_reg (slot); >> + } >> + >> + gcc_assert (POINTER_BOUNDS_MODE_P (GET_MODE (bounds))); >> + if (!register_operand (bounds, BNDmode)) >> + bounds = copy_to_mode_reg (BNDmode, bounds); >> + >> + emit_insn (BNDmode == BND64mode >> + ? gen_bnd64_stx (addr, ptr, bounds) >> + : gen_bnd32_stx (addr, ptr, bounds)); >> +} >> + >> +/* Load and return bounds returned by function in SLOT. */ >> + >> +static rtx >> +ix86_load_returned_bounds (rtx slot) >> +{ >> + rtx res; >> + >> + gcc_assert (REG_P (slot)); >> + res = gen_reg_rtx (BNDmode); >> + emit_move_insn (res, slot); >> + >> + return res; >> +} >> + >> +/* Store BOUNDS returned by function into SLOT. */ >> + >> +static void >> +ix86_store_returned_bounds (rtx slot, rtx bounds) >> +{ >> + gcc_assert (REG_P (slot)); >> + emit_move_insn (slot, bounds); >> +} >> + >> /* Returns a function decl for a vectorized version of the builtin function >> with builtin function code FN and the result vector type TYPE, or >> NULL_TREE >> if it is not available. */ >> @@ -47480,6 +47701,61 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree >> *clear, tree *update) >> atomic_feraiseexcept_call); >> } >> >> +static enum machine_mode >> +ix86_mpx_bound_mode () >> +{ >> + /* Do not support pointer checker if MPX >> + is not enabled. */ >> + if (!TARGET_MPX) >> + { >> + if (flag_check_pointer_bounds) >> + warning (0, "Pointer Checker requires MPX support on this target." >> + " Use -mmpx options to enable MPX."); >> + return VOIDmode; >> + } >> + >> + return BNDmode; >> +} >> + >> +static tree >> +ix86_make_bounds_constant (HOST_WIDE_INT lb, HOST_WIDE_INT ub) >> +{ >> + tree low = lb ? build_minus_one_cst (pointer_sized_int_node) >> + : build_zero_cst (pointer_sized_int_node); >> + tree high = ub ? build_zero_cst (pointer_sized_int_node) >> + : build_minus_one_cst (pointer_sized_int_node); >> + >> + /* This function is supposed to be used to create zero and >> + none bounds only. */ >> + gcc_assert (lb == 0 || lb == -1); >> + gcc_assert (ub == 0 || ub == -1); >> + >> + return build_complex (NULL, low, high); >> +} >> + >> +static int >> +ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts) >> +{ >> + tree size_ptr = build_pointer_type (size_type_node); >> + tree lhs, modify, var_p; >> + >> + ub = build1 (BIT_NOT_EXPR, size_type_node, ub); >> + var_p = build1 (CONVERT_EXPR, size_ptr, >> + build_fold_addr_expr (var)); >> + >> + lhs = build1 (INDIRECT_REF, size_type_node, var_p); >> + modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, lb); >> + append_to_statement_list (modify, stmts); >> + >> + lhs = build1 (INDIRECT_REF, size_type_node, >> + build2 (POINTER_PLUS_EXPR, size_ptr, var_p, >> + TYPE_SIZE_UNIT (size_type_node))); >> + modify = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, ub); >> + append_to_statement_list (modify, stmts); >> + >> + return 2; >> +} >> + >> /* Initialize the GCC target structure. */ >> #undef TARGET_RETURN_IN_MEMORY >> #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory >> @@ -47897,6 +48173,33 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree >> *clear, tree *update) >> #undef TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS >> #define TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS true >> >> +#undef TARGET_LOAD_BOUNDS_FOR_ARG >> +#define TARGET_LOAD_BOUNDS_FOR_ARG ix86_load_bounds >> + >> +#undef TARGET_STORE_BOUNDS_FOR_ARG >> +#define TARGET_STORE_BOUNDS_FOR_ARG ix86_store_bounds >> + >> +#undef TARGET_LOAD_RETURNED_BOUNDS >> +#define TARGET_LOAD_RETURNED_BOUNDS ix86_load_returned_bounds >> + >> +#undef TARGET_STORE_RETURNED_BOUNDS >> +#define TARGET_STORE_RETURNED_BOUNDS ix86_store_returned_bounds >> + >> +#undef TARGET_CHKP_BOUND_MODE >> +#define TARGET_CHKP_BOUND_MODE ix86_mpx_bound_mode >> + >> +#undef TARGET_BUILTIN_CHKP_FUNCTION >> +#define TARGET_BUILTIN_CHKP_FUNCTION ix86_builtin_mpx_function >> + >> +#undef TARGET_CHKP_FUNCTION_VALUE_BOUNDS >> +#define TARGET_CHKP_FUNCTION_VALUE_BOUNDS ix86_function_value_bounds >> + >> +#undef TARGET_CHKP_MAKE_BOUNDS_CONSTANT >> +#define TARGET_CHKP_MAKE_BOUNDS_CONSTANT ix86_make_bounds_constant >> + >> +#undef TARGET_CHKP_INITIALIZE_BOUNDS >> +#define TARGET_CHKP_INITIALIZE_BOUNDS ix86_initialize_bounds >> + >> struct gcc_target targetm = TARGET_INITIALIZER; >> >> #include "gt-i386.h"