Re: [PATCH, MPX, 2/X] Pointers Checker [5/25] Tree and gimple ifaces

[Jeff Law <law at redhat dot com>]

On 11/04/13 05:40, Richard Biener wrote:
> > Effectively the bounds are passed "on the side".
>
> Well, not exactly.  I can see __bound_tmp.0_4 passed to access_and_store.
I'm referring to how they're dealt with in FUNCTION_ARG and friends, ie, 
the low level aspects.  Maybe that's why we're crossing wires here.


> Are __builtin_ia32_bndcu and access_and_store supposed to be
> called directly after each other?  What if for example profile instrumentation
> inserts a call inbetween them?
That's a question for Ilya.

> > 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.
>
> I'm only looking at how bound arguments are passed at the GIMPLE
> level - which I think is arbitrary given they are passed on-the-side
> during code-generation.  So I'm looking for a more "sensible" option
> for the GIMPLE representation which looks somewhat fragile to me
> (it looks the argument is only there to have a data dependence
> between the bndcu intrinsic and the actual call?)
OK, we're clearly looking at two different aspects here.  While I think 
we can express them arbitrarily in GIMPLE, we have to pass them on the 
side once we get into the low level code for compatibility purposes.

I think argument passing for MPX is going to ultimately be ugly/fragile 
regardless of what we do given the constraints.  Given we're passing on 
the side at the low level, I'd prefer them passed on the side in GIMPLE, 
but I'm willing to consider other alternatives.

> > > /* 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.
>
> Well, consider compile.exp limits-fnargs.c written with int * parameters
> and bound support ;)  [there is a limit on the number of bound registers
> available, but the GIMPLE parts doesn't put any limit on instrumented
> args?]
Right.  Go back to Ilya's earlier messages :-)

There's these two magic hooks TARGET_{LOAD,STORE}_BOUNDS_FOR_ARG.  At 
some point when you run out of regs, you start shoving these things into 
memory.  I think we're ultimately going to need further clarification here.

> > > 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.
>
> Compile TU1 with -fno-chk, TU2 with -fchk, LTO both object files
> which gives you - well, either -fno-chk or -fchk.  Then you read in
> the GIMPLE and when calling gimple_call_nobnd_arg you get
> a mismatch between where you generated the gimple and where
> you use the gimple.
>
> A flag on whether a call is instrumented is better (consider inlining
> from TU1 into TU2 where a flag on struct function for example wouldn't
> be enough either).
OK.  I see what you're saying.  Whether or not we have these extra args 
is a property of the call site, not the CFUN, target DECL or the TU. 
That makes sense.


Ilya, I think there's enough confusion & issues around the call ABI/API 
that we need to sort it out before moving forward in any significant way.

Let's go back to argument passing and go through some concrete examples 
of the various cases.  Make sure to tie them into the calls to 
TARGET_{LOAD,STORE}_BOUNDS_FOR_ARG and the new gimple_call_* routines.

Jeff