Re: [PATCH] Fix unsafe function attributes for special functions (PR 71876)

[Richard Biener <rguenther at suse dot de>]

On July 20, 2016 6:54:48 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>On 07/20/16 18:20, Jeff Law wrote:
>> On 07/20/2016 09:41 AM, Bernd Edlinger wrote:
>>> On 07/20/16 12:44, Richard Biener wrote:
>>>> On Tue, 19 Jul 2016, Bernd Edlinger wrote:
>>>>
>>>>> Hi!
>>>>>
>>>>> As discussed at
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71876,
>>>>> we have a _very_ old hack in gcc, that recognizes certain
>functions by
>>>>> name, and inserts in some cases unsafe attributes, that don't work
>for
>>>>> a freestanding environment.
>>>>>
>>>>> It is unsafe to return ECF_MAY_BE_ALLOCA, ECF_LEAF and
>ECF_NORETURN
>>>>> from special_function_p, just by the name of the function,
>especially
>>>>> for less well known functions, like "getcontext" or "savectx",
>which
>>>>> could easily used for something completely different.
>>>>
>>>> Returning ECF_MAY_BE_ALLOCA is safe.  Just wanted to mention this,
>>>> regardless of the followups you already received.
>>>
>>>
>>> I dont think so.
>>>
>>>
>>> Consider this example:
>>>
>>> cat test.cc
>>> //extern "C"
>>> void *alloca(unsigned long);
>>> void bar(unsigned long n)
>>> {
>>>    char *x = (char*) alloca(n);
>>>    if (x)
>>>      *x = 0;
>>> }
>>>
>>> g++ -O3 -S test.cc
>>>
>>> result:
>>>
>>> _Z3barm:
>>> .LFB0:
>>>     .cfi_startproc
>>>     pushq    %rbp
>>>     .cfi_def_cfa_offset 16
>>>     .cfi_offset 6, -16
>>>     movq    %rsp, %rbp
>>>     .cfi_def_cfa_register 6
>>>     call    _Z6allocam
>>>     movb    $0, (%rax)
>>>     leave
>>>     .cfi_def_cfa 7, 8
>>>     ret
>>>
>>> So we call a C++ function with name alloca, but because
>>> special_function_p adds ECF_MAY_BE_ALLOCA, the null-pointer
>>> check is eliminated, but it is not the builtin alloca,
>>> but for the C++ front end it is a pretty normal function.
>> Clearly if something "may be alloca", then the properties on the
>> arguments/return values & side effects that are specific to alloca
>can
>> not be relied upon.  That to me seems like a bug elsewhere in the
>> compiler independent of the changes you're trying to make.
>>
>
>
>Yes. That is another interesting observation.  I think, originally this
>flag was introduced by Jan Hubicka, and should mean, "it may be alloca
>or a weak alias to alloca or maybe even something different".
>But some of the later optimizations use it in a way as if it meant
>"it must be alloca".  However I have not been able to come up with
>a test case that makes this assumption false, but I probably just
>did not try hard enough.
>
>But I think that alloca just should not be recognized by name any
>more.

It was introduced to mark calls that should not be duplicated by inlining or unrolling to avoid increasing stack usage too much.  Sth worthwhile to keep even with -ffreestanding.

Richard.

>
>>
>>
>> Jeff
>>