[llvm-dev] [cfe-dev] clang and -D_FORTIFY_SOURCE=1

[Serge Guelton via llvm-dev]

Got it. There are two places (one in clang, one in llvm) where being named
`memcpy' is a free pass for a function, independently of the presence of an
actual definition. The attached patch makes sure that if a function named
after an intrinsic has a body, calls to that function are not lowered to
intrinsics. It has the (expected) side effect of getting a behavior closer
to GCC wrt. -D_FORTIFY_SOURCE=n, and wrt. using function named after famous
ones.

Do we want to go that way?



On Wed, Dec 4, 2019 at 10:23 AM Serge Guelton <sguelton at redhat.com> wrote:

> > So I would conclude that the builtin works just fine, but the issue is
> > with the exact rules for when and how the inline function is preferred
> > over the externally available version.
>
> Fascinating. I can elaborate a bit more based on
> https://godbolt.org/z/EZGfqd : there's a special handling for memcpy (and
> probably other similar functions) in clang that prevents him from
> considering the inline version.I'll dig in clang source to find out more
> about this behavior.
>
> On Wed, Dec 4, 2019 at 9:11 AM Martin Storsjö <martin at martin.st> wrote:
>
>> On Wed, 4 Dec 2019, Serge Guelton via llvm-dev wrote:
>>
>> > > Are you sure you've diagnosed the issue correctly?
>> __builtin___memcpy_chk
>> > works correctly, as far as I know.
>> > 100% sure. Let's have a look at the output of
>> >
>> >   #include <string.h>
>> >   static char dest[10];
>> >   char* square(int n) {
>> >     memcpy(dest, "hello", n);
>> >     return dest;
>> >   }
>> >
>> > compiled with -D_FORTIFY_SOURCE=1 -O1 : https://godbolt.org/z/UvABWp
>> >
>> > Clang issues a call to memcpy, while gcc issues a call to __memcpy_chk.
>> > The call to __memcpy_chk performs extra runtime checks memcpy doesn't,
>> > and clang doesn't generate the extra checks inline either. This is a
>> > separate
>> > concern from the accuracy of __builtin_object_size, just a different
>> runtime
>> > behavior.
>> >
>> > Clang could generate the call to __memcpy_chk if its declaration is
>> > available, which is the case for the glibc.
>>
>> No, this doesn't seem to be the case.
>>
>> I expanded your testcase by removing the string.h include and replacing
>> it
>> with the relevant bits of it:
>>
>> https://godbolt.org/z/NKUQbW
>>
>> The regular call to memcpy ends up without the extra safety checks and
>> just generates a call to memcpy. But if I call __builtin___memcpy_chk
>> directly, it does generate a call to __memcpy_chk.
>>
>> So I would conclude that the builtin works just fine, but the issue is
>> with the exact rules for when and how the inline function is preferred
>> over the externally available version.
>>
>> I added a "#define VISIBIILTY extern" to ease testing and changing it. If
>> you change that into static, you'll see that the inline version of the
>> function actually ends up used, and it does generate a call to
>> __memcpy_chk even there.
>>
>> And this does seem to show that __builtin_object_size does work fine from
>> within an inlined function now, contrary to my earlier comments.
>>
>> // Martin
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191204/3402d36a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: memcpy.patch
Type: text/x-patch
Size: 1279 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191204/3402d36a/attachment.bin>