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, MPX, 2/X] Pointers Checker [5/25] Tree and gimple ifaces


On 10/30/13 04:48, Richard Biener wrote:
foo (int * p, unsigned int size)
{
   <unnamed type> __bound_tmp.0;
   long unsigned int D.2239;
   long unsigned int _2;
   sizetype _6;
   int * _7;

   <bb 3>:
   __bound_tmp.0_4 = __builtin_ia32_arg_bnd (p_3(D));

   <bb 2>:
   _2 = (long unsigned int) size_1(D);
   __builtin_ia32_bndcl (__bound_tmp.0_4, p_3(D));
   _6 = _2 + 18446744073709551615;
   _7 = p_3(D) + _6;
   __builtin_ia32_bndcu (__bound_tmp.0_4, _7);
   access_and_store (p_3(D), __bound_tmp.0_4, size_1(D));

so it seems there is now a mismatch between DECL_ARGUMENTS
and the GIMPLE call stmt arguments.  How (if) did you amend
the GIMPLE stmt verifier for this?
Effectively the bounds are passed "on the side".


How does regular code deal with this which may expect matching
to DECL_ARGUMENTS?  In fact interleaving the additional
arguments sounds very error-prone for existing code - I'd have
appended all bound args at the end.  Also you unconditionally
claim all pointer arguments have a bound - that looks like bad
design as well.  Why didn't you add a flag to the relevant
PARM_DECL (and then, what do you do for indirect calls?).
You can't actually interleave them -- that results in MPX and normal code not being able to interact. Passing the bound at the end doesn't really work either -- varargs and the desire to pass some of the bounds around in bound registers.



/* Return the number of arguments used by call statement GS
    ignoring bound ones.  */

static inline unsigned
gimple_call_num_nobnd_args (const_gimple gs)
{
   unsigned num_args = gimple_call_num_args (gs);
   unsigned res = num_args;
   for (unsigned n = 0; n < num_args; n++)
     if (POINTER_BOUNDS_P (gimple_call_arg (gs, n)))
       res--;
   return res;
}

the choice means that gimple_call_num_nobnd_args is not O(1).
Yes, but I don't see that's terribly problematical.



/* Return INDEX's call argument ignoring bound ones.  */
static inline tree
gimple_call_nobnd_arg (const_gimple gs, unsigned index)
{
   /* No bound args may exist if pointers checker is off.  */
   if (!flag_check_pointer_bounds)
     return gimple_call_arg (gs, index);
   return gimple_call_arg (gs, gimple_call_get_nobnd_arg_index (gs, index));
}

GIMPLE layout depending on flag_check_pointer_bounds sounds
like a recipie for desaster if you consider TUs compiled with and
TUs compiled without and LTO.  Or if you consider using
optimized attribute with that flag.
Sorry, I don't follow.  Can you elaborate please.

I hope the reviewers that approved the patch will work with you to
address the above issues.  I can't be everywhere.
Obviously I will.

jeff


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