On 07/19/16 19:30, Bernd Edlinger wrote:
> 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.
I have tried to find a test case with setjmp/longjmp where the leaf
attribute on the setjmp makes a difference, and the most aggressive
test case I could think of was this:
cat test1.c
static long env0[16], env1[16];
static int x = 0;
int jmp(void*) __attribute__((returns_twice, nothrow, leaf));
void go(void*, void*, int) __attribute__((nothrow));
void doit()
{
x++;
}
int
test()
{
static int xx;
xx = x;
jmp (env0);
xx += x;
jmp (env1);
go (env0, env1, xx + x);
return xx;
}
gcc -O3 -S test.c && inspect assembler code.
Here jmp is marked leaf and returns_twice, but go is not leaf
and can either call doit, or jump to env0 or env1, or simply
return. It would be wrong to expect "x" not to change between
the jmp calls. gcc-4.8.4 generates invalid code out of this,
and correct code if leaf is not in the function declaration.
However on trunk the code is correct, and the assembler code is
completely identical with or without leaf.
I wondered when that was fixed...
So I googled a bit around "gcc returns_twice leaf", and found
something interesting, where you argumented that it would
be wrong to put the leaf attribute at the setjmp function
in glibc: https://bugzilla.redhat.com/show_bug.cgi?id=752905
"Jakub Jelinek 2011-11-10 14:38:50 EST
setjmp/__sigsetjmp/_setjmp definitely must be __THROWNL rather than __THROW.
Similarly setcontext and swapcontext. They all have side-effects which
can modify module static variables that don't have address taken."
I completely agree with you. But how can it be, that special_function_p
seems to do the completely opposite thing, and add leaf to the setjmp
declaration even if that is not written in the header file?
Thanks
Bernd.
