[RFC] Using function clones for Pointer Bounds Checker

[Ilya Enkovich]

2014-05-13 1:20 GMT+04:00 Jeff Law <law@redhat.com>:
> On 01/14/14 02:15, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> I've been working for some time on the prototype of the Pointer Bounds
>> Checker which uses function clones for instrumentation
>> (http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03327.html). After
>> several experiments with this approach I want to share my results and
>> ask for some feedback to make a decision about the future steps.
>
> Sorry for the delayed reply, when you sent this message I was ignoring
> anything that wasn't an open issue for GCC 4.9 -- it all gets thrown into a
> massive queue of things to come back to once stage1 reopens.
>
> So I really like the idea of trying to use function clones to simplify some
> of the details of implementing MPX support.
>
>
>>
>> Clones approach does not use special built-in function calls to
>> associate bounds with call arguments and input parameters. Each
>> function which should be instrumented gets an additional version and
>> only this special version will be instrumented.
>
> I like this.
>
>
>
>
>> This new version gets
>> additional bound arguments to express input bounds. When function call
>> is instrumented, it is redirected to instrumented version and all
>> bounds are passed as explicit call arguments. Thus we have explicit
>> pointer bounds flow similar to regular function parameters. It should
>> allow to avoid changes in optimization, avoid miss-optimizations,
>> allow existing IPA optimizations to work with bound args (e.g.
>> propagate constant bounds value and remove checks in called function).
>
> So from a linking standpoint, presumably you have to mangle the instrumented
> caller/callee in some manner.  Right?  Or are you dynamically dispatching
> somehow?

Originally the idea was o have instrumented clone to have the same
assembler name as the original function. Since instrumented code is
fully compatible with not instrumented code, we always emit only one
version. Usage of the same assembler name allows instrumented and not
instrumented calls to look similar in assembler. It worked fine until
I tried it with LTO where assembler name is used as a unique
identifier. With linker resolutions files it became even more harder
to use such approach. To resolve these issues I started to use new
assembler name with postfix, but linked with the original name using
IDENTIFIER_TRANSPARENT_ALIAS. It gives different assembler names for
clones and originals during compilation, but both clone and original
functions have similar name in output assembler.

>
>
>
>>
>> 1. Nodes reachability. Instrumented version is actually always
>> reachable when original function is reachable because it is always
>> emitted instead of the original. Thus I had to fix reachability
>> analysis to achieve it. Another similar problem is check whether node
>> can be removed after inline when inlining instrumented function. Not
>> hard to fix but probably other similar problems exist.
>
> Jan is going to be the best person to discuss these issues with you. He's
> the architect behind most of the IPA bits we've got right now.

For now I managed to handle this via introducing a new type of thunk
for original functions. If we have instrumented function it means we
do not emit original one and therefore may remove its body. At this
point I transform the original function into a new kind of thunk which
has call edge to the instrumented version. It makes reachability
analysis to work OK for instrumented functions with no modifications.
The only problem here is to avoid function clones removal before
instrumentation pass happens. I achieve it by forbidding to remove not
instrumented instrumentation clones by adding new case into
cgraph_can_remove_if_no_direct_calls_and_refs_p.

>
>
>>>
>> In general I feel that having special function version for
>> instrumentation has a better potential, should lead to less intrusive
>> changes in the compiler and better code quality.
>
> Agreed.  It seems *much* cleaner.
>
> jeff

Thanks for your review.
Ilya