[llvm-commits] [compiler-rt] r159294 - in /compiler-rt/trunk/lib: sanitizer_common/sanitizer_internal_defs.h tsan/rtl/tsan_defs.h tsan/rtl/tsan_flags.cc tsan/rtl/tsan_interceptors.cc tsan/rtl/tsan_md5.cc tsan/rtl/tsan_rtl_report.cc tsan/rtl/tsan_

[Dmitry Vyukov]

On Thu, Jun 28, 2012 at 11:56 PM, Dmitry Vyukov <dvyukov at google.com> wrote:

> There is a small problem. I declare the functions inside the runtime as:
> extern "C" inline void *__attribute__((always_inline,
> __nothrow__)) memcpy(void *dst, const void *src, uptr size) {
>    ...
> }
> They are inlined into runtime, that's OK. But they are also compiled into
> runtime library and supersede our interceptors. So that when user code
> calls memcpy, it calls this non-instrumented function. That's not OK.
> Any suggestions?
> FWIW, tomorrow I am going to try to add __asm__("__fake_memcpy__") :)
>


I see why it happens. We take address of memcpy in some places, and so
compiler decides to generate the function. (The address is intended to be
an address of the "other" memcpy).
I finally get it working state. But it become extremely messy with
significant amount of changes all over the place.




> On Thu, Jun 28, 2012 at 1:12 PM, Dmitry Vyukov <dvyukov at google.com> wrote:
>
>> On Thu, Jun 28, 2012 at 2:09 AM, Chandler Carruth <chandlerc at google.com>wrote:
>>
>>> It specifically prevents replacing your implementation of memcpy with a
>>> call to memcpy, resulting in an infloop. This will actually happen in some
>>> cases.
>>
>>
>> Why can't the same happen with a hosted impl?
>>
>>
>> I personally think we should also provide a way to avoid the introduction
>>> of *any* calls to memcpy, and instead synthesize the loops directly. That
>>> said, while we are close to that today, there are still a few places that
>>> will do this.
>>>
>>
>> It would be nice. For some reason I still have a feeling that I am
>> fighting against implementation details of the particular compiler.
>>
>>
>>
>> On Wed, Jun 27, 2012 at 3:04 PM, Dmitry Vyukov <dvyukov at google.com>wrote:
>>>
>>>> On Thu, Jun 28, 2012 at 1:56 AM, Chandler Carruth <chandlerc at google.com
>>>> > wrote:
>>>>
>>>>> Unless you specify -fno-builtin, the compiler (even the low-level code
>>>>> generator) is free to insert a call to 'memcpy', assuming that a libc will
>>>>> eventually show up and provide that function. It may not be happening yet,
>>>>> but it will be eventually.
>>>>>
>>>>> No amount of always-inline annotations help this, because the calls
>>>>> may get inserted after inlining has taken place.
>>>>>
>>>>
>>>>
>>>> Since freestanding implementations provide memcpy, how can
>>>> -ffreestanding/-fno-builtin help with this?... Why a compiler is disallows
>>>> to insert memcpy's in this case?
>>>>
>>>>
>>>>
>>>>
>>>>> On Wed, Jun 27, 2012 at 2:44 PM, Dmitry Vyukov <dvyukov at google.com>wrote:
>>>>>
>>>>>>
>>>>>> On Thu, Jun 28, 2012 at 1:32 AM, Chandler Carruth <
>>>>>> chandlerc at google.com> wrote:
>>>>>>
>>>>>>> On Wed, Jun 27, 2012 at 2:16 PM, Dmitry Vyukov <dvyukov at google.com>wrote:
>>>>>>>
>>>>>>>> On Thu, Jun 28, 2012 at 1:07 AM, Chandler Carruth <
>>>>>>>> chandlerc at google.com> wrote:
>>>>>>>>
>>>>>>>>> I suspect you'll need to work still harder than this... =[
>>>>>>>>
>>>>>>>>
>>>>>>>> I know :)
>>>>>>>> Now I need to understand how exactly it is broken.
>>>>>>>>
>>>>>>>
>>>>>>>  Hmm, I may also need to better understand what you're attempting to
>>>>>>> achieve with this patch (based on Timur's comment).
>>>>>>>
>>>>>>> You define memset, memcpy, etc in a header file -- what callers use
>>>>>>> them? Most callers moved to 'internal_foo' versions.
>>>>>>>
>>>>>>> To answer Timur's question -- yes its ok, but it's hard to control
>>>>>>> which (instrumented or not-instrumented) gets used if they are inserted by
>>>>>>> LLVM, and thus have the normal names. It occurs to me that my post-process
>>>>>>> name-mangling tool would solve this problem as well...
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> You'll need to compile with -fno-builtins in order to prevent
>>>>>>>>> LLVM's optimizer from recognizing your code as equivalent to 'memset' or
>>>>>>>>> 'memcpy', and replacing it with a call to those functions. =/
>>>>>>>>>
>>>>>>>>
>>>>>>>> I believe we are compiling with -fno-builtin.
>>>>>>>> You mentioned that I also need -ffreestanding:
>>>>>>>>
>>>>>>>
>>>>>>> Yea, -ffreestanding may be better. It is a superset of -fno-builtin
>>>>>>>
>>>>>>>
>>>>>>>>  "Yes, there are attributes which can be attached to the
>>>>>>>> non-instrumented memcpy function, provided by the runtime and selected due
>>>>>>>> to -ffreestanding, which will force inlining. __attribute__((always_inline)),
>>>>>>>> __attribute__((flatten))"
>>>>>>>>
>>>>>>>> Why do I need it?
>>>>>>>>
>>>>>>>
>>>>>>> Need which?
>>>>>>>
>>>>>>> You need '-ffreestanding' so that your source code can implement
>>>>>>> 'memset' itself, and that implementation will be used rather than assuming
>>>>>>> that at some point 'libc' will get linked in and we can call *its* memset.
>>>>>>> This is necessary even if your function isn't named 'memset' but something
>>>>>>> eles.
>>>>>>>
>>>>>>> The __attribute__((always_inline)) is needed if you want to force
>>>>>>> this function to be inlined into its callers. I don't recall off hand why
>>>>>>> this was important?
>>>>>>>
>>>>>>
>>>>>>
>>>>>> I need to prevent compiler from inserting memxxx() calls into random
>>>>>> places of the runtime. At first glance the patch achieves the goal.
>>>>>> always_inline is required to eliminate calls.
>>>>>> I do not fully understand why  -fno-builtins/-ffreestanding are
>>>>>> required. The idea is that the compiler leaves no calls to memxxx
>>>>>> (everything is inlined).
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120629/471ecb08/attachment.html>