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 30 Oct 11:40, Jeff Law wrote:
> 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 suppose the possile problem here is when we run LTO compiler without -fcheck-pointer-bounds and give instrumented code as input. gimple_call_nobnd_arg would work wrong for instrumented code. Actually there are other places in subsequent patches wich assume that flag_check_pointer_bounds is 1 if we have instrumented code. 

Ilya

> 
> >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]