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/19/16 18:37, Jakub Jelinek wrote:
> On Tue, Jul 19, 2016 at 04:20:55PM +0000, Bernd Edlinger wrote:
>> 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.
>>
>> Moreover, from the backend library we cannot check flag_hosted, or if
>> the function has "C" or "C++" binding.
>
> I believe this will regress handling of various functions.
> E.g. for alloca (as opposed to __builtin_alloca/__builtin_alloca_with_align,
> this means EFC_MAY_BE_ALLOCA will not be set anymore.
>

That depends on which options are used: with -ansi and -ffreestanding, 
alloca is just a normal function, which is kind of correct.

If you include glibc's <alloca.h>, alloca is directly defined to 
__builtin_alloca(x), which should work always.

If alloca is declared as void *alloca(size_t); it is also recognized as
built-in unless -ansi or -ffreestanding is used, so that handling was
in a way duplicated already.

So I see no regression here.

> _longjmp/siglongjmp will no longer be ECF_NORETURN (glibc
> doesn't declare them as such), __sigsetjmp will no longer be ECF_LEAF.

Which version of glibc do you refer to?

My 2.19-0ubuntu6.9 has:

extern void _longjmp (struct __jmp_buf_tag __env[1], int __val)
      __THROWNL __attribute__ ((__noreturn__));

extern int __sigsetjmp (struct __jmp_buf_tag __env[1], int __savemask) 
__THROWNL;

So yes, __THROWNL is "__attribute__ ((__nothrow__))".

But they also have __THROW around which is "__attribute__ ((__nothrow__
__LEAF))", so that is just a minor bug in the glibc header, the header
should declare it __THROW if it is no leaf.

If you are concerned about the leaf attribute, it would
be easy to add a builtin for _longjmp, and __sigsetjmp, as
the _ is reserved anyway.  However I considered it an implementation
detail of glibc, that could change, and I did not check newlib on that
either.

Should I add built-in for _longjmp and __sigsetjmp, and check if
that works for newlib too?


Thanks
Bernd.


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