This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, i386, Pointer Bounds Checker 34/x] Vararg functions support
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Uros Bizjak <ubizjak at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 22 Sep 2014 19:33:43 +0400
- Subject: Re: [PATCH, i386, Pointer Bounds Checker 34/x] Vararg functions support
- Authentication-results: sourceware.org; auth=none
- References: <CAFULd4bMFc8g5J6AWcF5dvf-nbbty1SCnS7xN=LRaK47jKMdBg at mail dot gmail dot com>
On 21 Sep 18:08, Uros Bizjak wrote:
> Hello!
>
> > This patch introduces initialization of incoming bounds for vararg function on i386 target.
> >
> > Bootstrapped and tested on linux-x86_64.
> >
> > Thanks,
> > Ilya
> > --
> > gcc/
> >
> > 2014-06-11 Ilya Enkovich <ilya.enkovich@intel.com>
> >
> > * config/i386/i386.c (ix86_setup_incoming_varargs): New.
> > (ix86_va_start): Initialize bounds for pointers in va_list.
> > (TARGET_SETUP_INCOMING_VARARG_BOUNDS): New.
>
> OK with a couple of smallchanges below.
>
> Thanks,
> Uros.
>
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index a67e6e7..c520f26 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -8456,6 +8456,72 @@ ix86_setup_incoming_varargs (cumulative_args_t cum_v, enum machine_mode mode,
> > setup_incoming_varargs_64 (&next_cum);
> > }
> >
> > +static void
> > +ix86_setup_incoming_vararg_bounds (cumulative_args_t cum_v,
> > + enum machine_mode mode,
> > + tree type,
> > + int *pretend_size ATTRIBUTE_UNUSED,
> > + int no_rtl)
> > +{
> > + CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
> > + CUMULATIVE_ARGS next_cum;
> > + tree fntype;
> > + rtx save_area;
> > + int bnd_reg, i, max;
> > +
> > + gcc_assert (!no_rtl);
> > +
>
> Please add a comment for following condition.
>
> > + if (!TARGET_64BIT)
> > + return;
> > +
> > + fntype = TREE_TYPE (current_function_decl);
> > +
> > + /* For varargs, we do not want to skip the dummy va_dcl argument.
> > + For stdargs, we do want to skip the last named argument. */
> > + next_cum = *cum;
> > + if (stdarg_p (fntype))
> > + ix86_function_arg_advance (pack_cumulative_args (&next_cum), mode, type,
> > + true);
> > + if (cum->call_abi == MS_ABI)
> > + return;
>
> Put this early exit at the top of the function.
>
> > + save_area = frame_pointer_rtx;
> > +
> > + max = cum->regno + cfun->va_list_gpr_size / UNITS_PER_WORD;
> > + if (max > X86_64_REGPARM_MAX)
> > + max = X86_64_REGPARM_MAX;
> > +
> > + bnd_reg = cum->bnd_regno + cum->force_bnd_pass;
> > + if (chkp_function_instrumented_p (current_function_decl))
> > + for (i = cum->regno; i < max; i++)
> > + {
> > + rtx addr = plus_constant (Pmode, save_area, i * UNITS_PER_WORD);
> > + rtx reg = gen_rtx_REG (DImode,
> > + x86_64_int_parameter_registers[i]);
> > + rtx ptr = reg;
> > + rtx bounds;
> > +
> > + if (bnd_reg <= LAST_BND_REG)
> > + bounds = gen_rtx_REG (BNDmode, bnd_reg);
> > + else
> > + {
> > + rtx ldx_addr = plus_constant (Pmode, arg_pointer_rtx,
> > + (LAST_BND_REG - bnd_reg) * 8);
>
> No magic constants!
>
> > + bounds = gen_reg_rtx (BNDmode);
> > + emit_insn (TARGET_64BIT
> > + ? gen_bnd64_ldx (bounds, ldx_addr, ptr)
> > + : gen_bnd32_ldx (bounds, ldx_addr, ptr));
> > + }
> > +
> > + emit_insn (TARGET_64BIT
> > + ? gen_bnd64_stx (addr, ptr, bounds)
> > + : gen_bnd32_stx (addr, ptr, bounds));
>
> Please check BNDmode instead of TARGET_64BIT.
>
> > + bnd_reg++;
> > + }
> > +}
> > +
> > +
> > /* Checks if TYPE is of kind va_list char *. */
> >
> > static bool
> > @@ -8478,7 +8544,7 @@ ix86_va_start (tree valist, rtx nextarg)
> > {
> > HOST_WIDE_INT words, n_gpr, n_fpr;
> > tree f_gpr, f_fpr, f_ovf, f_sav;
> > - tree gpr, fpr, ovf, sav, t;
> > + tree gpr, fpr, ovf, sav, t, t1;
> > tree type;
> > rtx ovf_rtx;
> >
> > @@ -8529,6 +8595,13 @@ ix86_va_start (tree valist, rtx nextarg)
> > crtl->args.arg_offset_rtx,
> > NULL_RTX, 0, OPTAB_LIB_WIDEN);
> > convert_move (va_r, next, 0);
> > +
> > + /* Store zero bounds for va_list. */
> > + if (chkp_function_instrumented_p (current_function_decl))
> > + chkp_expand_bounds_reset_for_mem (valist,
> > + make_tree (TREE_TYPE (valist),
> > + next));
> > +
> > }
> > return;
> > }
> > @@ -8582,10 +8655,15 @@ ix86_va_start (tree valist, rtx nextarg)
> > t = make_tree (type, ovf_rtx);
> > if (words != 0)
> > t = fold_build_pointer_plus_hwi (t, words * UNITS_PER_WORD);
> > + t1 = t;
> > t = build2 (MODIFY_EXPR, type, ovf, t);
> > TREE_SIDE_EFFECTS (t) = 1;
> > expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
> >
> > + /* Store zero bounds for overflow area pointer. */
> > + if (chkp_function_instrumented_p (current_function_decl))
> > + chkp_expand_bounds_reset_for_mem (ovf, t1);
>
> Can you please move this above to avoid another temporary (t1)?
>
> > if (ix86_varargs_gpr_size || ix86_varargs_fpr_size)
> > {
> > /* Find the register save area.
> > @@ -8594,9 +8672,14 @@ ix86_va_start (tree valist, rtx nextarg)
> > t = make_tree (type, frame_pointer_rtx);
> > if (!ix86_varargs_gpr_size)
> > t = fold_build_pointer_plus_hwi (t, -8 * X86_64_REGPARM_MAX);
> > + t1 = t;
> > t = build2 (MODIFY_EXPR, type, sav, t);
> > TREE_SIDE_EFFECTS (t) = 1;
> > expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
> > +
> > + /* Store zero bounds for save area pointer. */
> > + if (chkp_function_instrumented_p (current_function_decl))
> > + chkp_expand_bounds_reset_for_mem (sav, t1);
>
> Also move this part upper to remove another temporary.
>
> > }
> > }
> >
> > @@ -48057,6 +48140,9 @@ ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts)
> > #undef TARGET_CHKP_INITIALIZE_BOUNDS
> > #define TARGET_CHKP_INITIALIZE_BOUNDS ix86_initialize_bounds
> >
> > +#undef TARGET_SETUP_INCOMING_VARARG_BOUNDS
> > +#define TARGET_SETUP_INCOMING_VARARG_BOUNDS ix86_setup_incoming_vararg_bounds
> > +
> > struct gcc_target targetm = TARGET_INITIALIZER;
> >
> > #include "gt-i386.h"
Hi,
Thank you for review. Below is a fixed version.
Thanks,
Ilya
--
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 66622d2..c183501 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -8477,6 +8477,71 @@ ix86_setup_incoming_varargs (cumulative_args_t cum_v, enum machine_mode mode,
setup_incoming_varargs_64 (&next_cum);
}
+static void
+ix86_setup_incoming_vararg_bounds (cumulative_args_t cum_v,
+ enum machine_mode mode,
+ tree type,
+ int *pretend_size ATTRIBUTE_UNUSED,
+ int no_rtl)
+{
+ CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
+ CUMULATIVE_ARGS next_cum;
+ tree fntype;
+ rtx save_area;
+ int bnd_reg, i, max;
+
+ gcc_assert (!no_rtl);
+
+ /* Do nothing if we use plain pointer to argument area. */
+ if (!TARGET_64BIT || cum->call_abi == MS_ABI)
+ return;
+
+ fntype = TREE_TYPE (current_function_decl);
+
+ /* For varargs, we do not want to skip the dummy va_dcl argument.
+ For stdargs, we do want to skip the last named argument. */
+ next_cum = *cum;
+ if (stdarg_p (fntype))
+ ix86_function_arg_advance (pack_cumulative_args (&next_cum), mode, type,
+ true);
+ save_area = frame_pointer_rtx;
+
+ max = cum->regno + cfun->va_list_gpr_size / UNITS_PER_WORD;
+ if (max > X86_64_REGPARM_MAX)
+ max = X86_64_REGPARM_MAX;
+
+ bnd_reg = cum->bnd_regno + cum->force_bnd_pass;
+ if (chkp_function_instrumented_p (current_function_decl))
+ for (i = cum->regno; i < max; i++)
+ {
+ rtx addr = plus_constant (Pmode, save_area, i * UNITS_PER_WORD);
+ rtx reg = gen_rtx_REG (DImode,
+ x86_64_int_parameter_registers[i]);
+ rtx ptr = reg;
+ rtx bounds;
+
+ if (bnd_reg <= LAST_BND_REG)
+ bounds = gen_rtx_REG (BNDmode, bnd_reg);
+ else
+ {
+ rtx ldx_addr =
+ plus_constant (Pmode, arg_pointer_rtx,
+ (LAST_BND_REG - bnd_reg) * GET_MODE_SIZE (Pmode));
+ bounds = gen_reg_rtx (BNDmode);
+ emit_insn (BNDmode == BND64mode
+ ? gen_bnd64_ldx (bounds, ldx_addr, ptr)
+ : gen_bnd32_ldx (bounds, ldx_addr, ptr));
+ }
+
+ emit_insn (BNDmode == BND64mode
+ ? gen_bnd64_stx (addr, ptr, bounds)
+ : gen_bnd32_stx (addr, ptr, bounds));
+
+ bnd_reg++;
+ }
+}
+
+
/* Checks if TYPE is of kind va_list char *. */
static bool
@@ -8550,6 +8615,13 @@ ix86_va_start (tree valist, rtx nextarg)
crtl->args.arg_offset_rtx,
NULL_RTX, 0, OPTAB_LIB_WIDEN);
convert_move (va_r, next, 0);
+
+ /* Store zero bounds for va_list. */
+ if (chkp_function_instrumented_p (current_function_decl))
+ chkp_expand_bounds_reset_for_mem (valist,
+ make_tree (TREE_TYPE (valist),
+ next));
+
}
return;
}
@@ -8603,6 +8675,11 @@ ix86_va_start (tree valist, rtx nextarg)
t = make_tree (type, ovf_rtx);
if (words != 0)
t = fold_build_pointer_plus_hwi (t, words * UNITS_PER_WORD);
+
+ /* Store zero bounds for overflow area pointer. */
+ if (chkp_function_instrumented_p (current_function_decl))
+ chkp_expand_bounds_reset_for_mem (ovf, t);
+
t = build2 (MODIFY_EXPR, type, ovf, t);
TREE_SIDE_EFFECTS (t) = 1;
expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
@@ -8615,6 +8692,11 @@ ix86_va_start (tree valist, rtx nextarg)
t = make_tree (type, frame_pointer_rtx);
if (!ix86_varargs_gpr_size)
t = fold_build_pointer_plus_hwi (t, -8 * X86_64_REGPARM_MAX);
+
+ /* Store zero bounds for save area pointer. */
+ if (chkp_function_instrumented_p (current_function_decl))
+ chkp_expand_bounds_reset_for_mem (sav, t);
+
t = build2 (MODIFY_EXPR, type, sav, t);
TREE_SIDE_EFFECTS (t) = 1;
expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
@@ -48313,6 +48395,9 @@ ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts)
#undef TARGET_CHKP_INITIALIZE_BOUNDS
#define TARGET_CHKP_INITIALIZE_BOUNDS ix86_initialize_bounds
+#undef TARGET_SETUP_INCOMING_VARARG_BOUNDS
+#define TARGET_SETUP_INCOMING_VARARG_BOUNDS ix86_setup_incoming_vararg_bounds
+
struct gcc_target targetm = TARGET_INITIALIZER;
#include "gt-i386.h"