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);
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; }?
Richard.
> Ilya
>
>>
>> Jeff
