[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]

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__") :)



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/20120628/812ab463/attachment.html>