2013/11/11 Richard Biener <richard.guent...@gmail.com>: > On Thu, Nov 7, 2013 at 10:07 PM, Ilya Enkovich <enkovich....@gmail.com> wrote: >> 2013/11/7 Jeff Law <l...@redhat.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. >> >> I think there is some misunderstanding. __builtin_ia32_bndcu and >> access_and_store calls are completely independent in this example. >> This is just a code from example on how user-level builtins may be >> used with legacy code. >> >>> >>> >>>>> >>>>> 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. >> >> OK. Lets see at how expand pass handles instrumented calls. As usual, >> for each passed argument expand calls target hook to determine where >> argument is passed. Now target may return not only slot for arg, but >> also a slot for bounds in case arg may have them (PARALLEL expr is >> used to hold all slots). If bounds slot is register, then expand >> simply puts bounds to it. If it is not a register, target hook is >> called to store bounds (TARGET_STORE_BOUNDS_FOR_ARG). Incoming bounds >> are handled in a similar way but with usage of >> TARGET_LOAD_BOUNDS_FOR_ARG hook. >> >> The only remained problem for expand here is to identify which bounds >> should be passed for arg. To tie bounds to argument I put all bounds >> passed for arg (in case arg is a structure, we may have multiple >> bounds) as additional arguments in the call. Each regular argument is >> immediately followed by all bounds passed for it. For example: >> >> test (&"Hello world!"[0], 0); >> >> is translated into >> >> test (&"Hello world!"[0], __bound_tmp.0_1, 0); >> >> where __bounds_tmp.0_1 - bounds of passed string. >> >> Of course, such call modification has some implications. Some >> optimizations may rely on number of arguments in the call (e,g, strlen >> pass) and indexes of arguments. Also now index of arg in fndecl does >> not match the index of actual arg in the call. For such cases new >> functions are introduced to get args by it's original index, like >> there is no instrumentation. BTW there are not many usages of these >> new functions and almost all of them are in strlen pass. >> >> Also some changes, like the patch I sent for calls modifications and >> copy, are required. I suppose such changes are quite simple. and not >> very wide. >> >> Regarding proposal to put all bounds to the end of call's arg list - I >> do not think it makes life much easier. It still requires >> modifications in verifier (because it checks arg count), calls copy >> etc. It becomes more complex to identify bounds of arg. Also GIMPLE >> dump for calls becomes less informative. Surely it allows to get rid >> of gimple_call_nobnd_arg and gimple_call_get_nobnd_arg_index but will >> require additional interface functions for bound args. > > What about passing bounds together with the value in the same slot? > Like via > > arg = __builtin_tie_arg_and_bound (arg, bound); > foo (arg);
There are other ways to get bounds. E.g. load bounds from table. If I use the same builtin for both binding and bounds load, then It is not clear what to do in case same bounds are used for multiple aegs. For Example: int *p int foo() { bar(p, p+1, p+2); } Here all 3 args have same bounds, which are result of __builtin_bndldx call. If I use the same builtin for binding then I have 3 loads. So, probably, a separate builtin is better. > > you say you can have bounds for aggregates, do you mean > > struct X { int *x; int *y; }; > foo (struct X); > > the ABI says for a call to foo () you pass two bound arguments? > Or do you merely mean the degenerate case struct X { int *x; }? In case of structure we pass bounds for all pointers in it. Ilya > > Richard. > >> Ilya >> >>> >>> Jeff