Re: [PATCH 3/4] ARC/signal: shield sa_restorer from compiler toggle side-effects

On Thursday 26 March 2015 04:49 PM, Bernhard Reutner-Fischer wrote:
> On 26 March 2015 at 09:55, Vineet Gupta <[email protected]> wrote:
>> when building uClibc with -O0 (DODEBUG build) the default sigrestorer
>> had some extra glue code generated for stack manipulation which was
>> messing up resume from signal path.
>>
>> So annotate the function with -Os so that gcc would only generate the
>> bare min 2 instruction TRAP sequence
> .. which is the reason the rt_sigrestorer and default_sigrestorer are
> usually implemented in a separate file (usually asm, IIRC).

Yeah, we used to have it asm which was nice and fine, except that it was a
different source file so PIC references to restorer were not super optimal.

> I think the toplevel impl that is used in x86_64 works fine and has
> the nice benefit to be in one source-file.
> See e.g.: libc/sysdeps/linux/x86_64/sigaction.c
>
> This would have avoided all the sigaction dance in
> 65bc357213f1c08e339757923740f5d68212cf76 and
> f74294bd6768af59e33dc14c6fed41f01d0adc5b it seems, but anyway.

Only the first one, latter is not related to restorer it just avoids a hop in
sigaction path.

>
> Do you think the x86_64 scheme makes sense for you and you can peruse it?

I could, but....

x86 approach solves the multi-src file issue, but we still lack dwarf unwind info,
w/o adding explicit asm CFA ops which AFAIKR our tools still don't support. With C
we get the dwarf info for free.

Thx,
-Vineet


>
> thanks,
>> Reported-and-Debugged-by: Alexey Brodkin <[email protected]>
>> Signed-off-by: Vineet Gupta <[email protected]>
>> ---
>>  libc/sysdeps/linux/arc/sigaction.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libc/sysdeps/linux/arc/sigaction.c b/libc/sysdeps/linux/arc/sigaction.c
>> index 4a4c9e2d0821..3e2f91cd73e8 100644
>> --- a/libc/sysdeps/linux/arc/sigaction.c
>> +++ b/libc/sysdeps/linux/arc/sigaction.c
>> @@ -13,7 +13,8 @@
>>  /*
>>   * Default sigretrun stub if user doesn't specify SA_RESTORER
>>   */
>> -static void __default_rt_sa_restorer(void)
>> +static void __attribute__((noinline, optimize ("-Os")))
>> +__default_rt_sa_restorer(void)
>>  {
>>         INTERNAL_SYSCALL_NCS(__NR_rt_sigreturn, , 0);
>>  }
>> --
>> 1.9.1
>>
>> _______________________________________________
>> uClibc mailing list
>> [email protected]
>> http://lists.busybox.net/mailman/listinfo/uclibc