This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, i386, Pointer Bounds Checker 34/x] Vararg functions support


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"


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]