This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.

>
>
> Jeff
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]