This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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] Tests for minimal signal handler functionality in MINSIGSTKSZ space.


On 1/18/19 9:53 AM, H.J. Lu wrote:
> On Thu, Jan 17, 2019 at 7:12 PM Zack Weinberg <zackw@panix.com> wrote:
>>
>> On Wed, Jan 16, 2019 at 10:31 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>
>>> The new tests failed on AVX512 machines:
>>>
>>> Program received signal SIGUSR1, User defined signal 1.
>>> __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
>>> 50   return ret;
>>> (gdb) c
>>> Continuing.
>>>
>>> Program received signal SIGSEGV, Segmentation fault.
>>> _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
>>> 93 movq %rax, REGISTER_SAVE_RAX(%rsp)
>>> (gdb) bt
>>> #0  _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
>>> #1  0x0040248d in handler (unused=<optimized out>) at tst-minsigstksz-4.c:44
>>> #2  <signal handler called>
>>> #3  __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
>>> #4  0x004024da in do_test () at tst-minsigstksz-4.c:59
>>> #5  0x00402cd6 in support_test_main (argc=1, argv=0xffffcef8,
>>>     config=config@entry=0xffffcdf0) at support_test_main.c:350
>>> #6  0x00402348 in main (argc=<optimized out>, argv=<optimized out>)
>>>     at ../support/test-driver.c:168
>>> (gdb)
>>>
>>> AVX512 needs 2560 bytes to save processor state.
>>
>> Well, this is the problem that we knew existed and can't fix quickly.
>> If I'm reading http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
>> correctly, we can't introduce new sysconf constants unilaterally
>> (there is no license to define system-specific _SC_* symbols).
>>
>> I wonder if this test passes if you run it with LD_BIND_NOW=t in the
>> environment.  Forcing -z now for these tests might be the best we can
>> do for 2.29.
>>
> 
> This works:
> 
> diff --git a/signal/Makefile b/signal/Makefile
> index 9597287bca..9c65c26887 100644
> --- a/signal/Makefile
> +++ b/signal/Makefile
> @@ -59,3 +59,9 @@ CFLAGS-sigwait.c += -fexceptions -fasynchronous-unwind-tables
>  CFLAGS-sigwaitinfo.c += -fexceptions -fasynchronous-unwind-tables
> 
>  CFLAGS-sigreturn.c += $(no-stack-protector)
> +

OK with comment:

# We don't want to test the lazy resolution stack usage
# just the execution of the handler and the functions
> +LDFLAGS-tst-minsigstksz-1 = -Wl,-z,now
> +LDFLAGS-tst-minsigstksz-2 = -Wl,-z,now
> +LDFLAGS-tst-minsigstksz-3 = -Wl,-z,now
> +LDFLAGS-tst-minsigstksz-3a = -Wl,-z,now
> +LDFLAGS-tst-minsigstksz-4 = -Wl,-z,now
> 

I'd say this is OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

All of this raises an interesting point. Should MINSIGSTKSZ
have included enough space for the lazy resolution to happen?
I think we have to, because you're never going to have already
called abort, quick_exit, or _exit, so they will all go through
lazy binding resolution if you're not BIND_NOW. Which means we
need an average estimate from all arches about the lazy binding
stack usage.

-- 
Cheers,
Carlos.


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