| Message ID | CALoOobMeBycaWz2E7_YYQFeg38936j3xde5k3+zk1sgrLy+J0Q@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers | expand |
On 08/29/2017 01:11 PM, Paul Pluzhnikov wrote: > Greetings, > > This patch implements the other TODO in stdlib/tst-atexit-common.c > > 2017-08-29 Paul Pluzhnikov <ppluzhnikov@google.com> > > * stdlib/tst-atexit-common.c (do_test): Test support for at least > 32 atexit handlers. This looks good to me. Thank you for being so thorough.
On Tue, Aug 29, 2017 at 11:11 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > Greetings, > > This patch implements the other TODO in stdlib/tst-atexit-common.c > > 2017-08-29 Paul Pluzhnikov <ppluzhnikov@google.com> > > * stdlib/tst-atexit-common.c (do_test): Test support for at least > 32 atexit handlers. > > It failed on Linux/i686: FAIL: stdlib/tst-at_quick_exit FAIL: stdlib/tst-atexit FAIL: stdlib/tst-cxa_atexit FAIL: stdlib/tst-on_exit [hjl@gnu-6 build-i686-linux]$ ./stdlib/tst-at_quick_exit crumbs: 00000000000000000000000003021121130211 expected: 00000000000000000000000003021121130211 unexpected exit status 1 from child 16101 [hjl@gnu-6 build-i686-linux]$
On Fri, Sep 1, 2017 at 11:03 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> It failed on Linux/i686:
The "crumbs" buffer was not sized correctly, and I had global buffer overflow:
==71453==ERROR: AddressSanitizer: global-buffer-overflow on address
0x0000006023c0 at pc 0x000000400e84 bp 0x7ffe059dc370 sp
0x7ffe059dc368
WRITE of size 1 at 0x0000006023c0 thread T0
#0 0x400e83 in fn1 ../stdlib/tst-atexit-common.c:53
#1 0x7ff8af2da1a8 (/lib/x86_64-linux-gnu/libc.so.6+0x3c1a8)
#2 0x7ff8af2da1f4 in exit (/lib/x86_64-linux-gnu/libc.so.6+0x3c1f4)
#3 0x400bf5 in do_test ../stdlib/tst-atexit-common.c:140
#4 0x400bf5 in main ../stdlib/tst-atexit-common.c:143
#5 0x7ff8af2bff44 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
#6 0x400d39 (/glibc-git/stdlib/a.out+0x400d39)
0x0000006023c0 is located 0 bytes to the right of global variable
'crumbs' defined in '../stdlib/tst-atexit-common.c:33:13' (0x6023a0)
of size 32
Sorry about that. Committed attached fix.
On Fri, Sep 1, 2017 at 11:37 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > On Fri, Sep 1, 2017 at 11:03 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > >> It failed on Linux/i686: > > The "crumbs" buffer was not sized correctly, and I had global buffer overflow: > > ==71453==ERROR: AddressSanitizer: global-buffer-overflow on address > 0x0000006023c0 at pc 0x000000400e84 bp 0x7ffe059dc370 sp > 0x7ffe059dc368 > WRITE of size 1 at 0x0000006023c0 thread T0 > #0 0x400e83 in fn1 ../stdlib/tst-atexit-common.c:53 > #1 0x7ff8af2da1a8 (/lib/x86_64-linux-gnu/libc.so.6+0x3c1a8) > #2 0x7ff8af2da1f4 in exit (/lib/x86_64-linux-gnu/libc.so.6+0x3c1f4) > #3 0x400bf5 in do_test ../stdlib/tst-atexit-common.c:140 > #4 0x400bf5 in main ../stdlib/tst-atexit-common.c:143 > #5 0x7ff8af2bff44 in __libc_start_main > (/lib/x86_64-linux-gnu/libc.so.6+0x21f44) > #6 0x400d39 (/glibc-git/stdlib/a.out+0x400d39) > > 0x0000006023c0 is located 0 bytes to the right of global variable > 'crumbs' defined in '../stdlib/tst-atexit-common.c:33:13' (0x6023a0) > of size 32 > > Sorry about that. Committed attached fix. > I still got [hjl@gnu-6 build-i686-linux]$ ./stdlib/tst-tls-atexit tst-tls-atexit: allocatestack.c:530: allocate_stack: Assertion `size != 0' failed. Didn't expect signal from child: got `Aborted' [hjl@gnu-6 build-i686-linux]$
On Fri, Sep 1, 2017 at 11:51 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > I still got > > [hjl@gnu-6 build-i686-linux]$ ./stdlib/tst-tls-atexit > tst-tls-atexit: allocatestack.c:530: allocate_stack: Assertion `size != 0' failed. > Didn't expect signal from child: got `Aborted' This doesn't look like something that would be caused by my patch. It also doesn't reproduce for me. Are you sure you don't have an inconsistent build or other local changes? Thanks,
On Fri, Sep 1, 2017 at 12:09 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > On Fri, Sep 1, 2017 at 11:51 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > >> I still got >> >> [hjl@gnu-6 build-i686-linux]$ ./stdlib/tst-tls-atexit >> tst-tls-atexit: allocatestack.c:530: allocate_stack: Assertion `size != 0' failed. >> Didn't expect signal from child: got `Aborted' > > This doesn't look like something that would be caused by my patch. It > also doesn't reproduce for me. > > Are you sure you don't have an inconsistent build or other local changes? False alarm. They passed after I did a clean build. Thanks.
diff --git a/stdlib/tst-atexit-common.c b/stdlib/tst-atexit-common.c index 99b00bf3aa..d6dcf08cdd 100644 --- a/stdlib/tst-atexit-common.c +++ b/stdlib/tst-atexit-common.c @@ -23,7 +23,13 @@ #include <unistd.h> #include <sys/wait.h> -#define MAX_ATEXIT 20 /* Large enough for current set of invocations. */ +/* http://pubs.opengroup.org/onlinepubs/000095399/functions/atexit.html + requires that we support at least 32 atexit handlers. + + The number we actually support is limited by memory. Here we simply + check that we support at least the minimum required. */ +#define MAX_ATEXIT 32 + static char crumbs[MAX_ATEXIT]; static int next_slot = 0; @@ -66,7 +72,7 @@ static void fn_final (void) { /* Arbitrary sequence matching current registrations. */ - const char expected[] = "3021121130211"; + const char expected[] = "00000000000000000000000003021121130211"; if (strcmp (crumbs, expected) == 0) _exit_with_flush (0); @@ -76,25 +82,26 @@ fn_final (void) _exit_with_flush (1); } -/* This is currently just a basic test to verify that exit handlers execute - in LIFO order, even when the handlers register additional new handlers. - - TODO: Additional tests that we should do: - 1. POSIX says we need to support at least ATEXIT_MAX - 2. ... */ - static int do_test (void) { + int slots_remaining = MAX_ATEXIT; + /* Register this first so it can verify expected order of the rest. */ - ATEXIT (fn_final); + ATEXIT (fn_final); --slots_remaining; - ATEXIT (fn1); - ATEXIT (fn3); - ATEXIT (fn1); - ATEXIT (fn2); - ATEXIT (fn1); - ATEXIT (fn3); + ATEXIT (fn1); --slots_remaining; + ATEXIT (fn3); --slots_remaining; + ATEXIT (fn1); --slots_remaining; + ATEXIT (fn2); --slots_remaining; + ATEXIT (fn1); --slots_remaining; + ATEXIT (fn3); --slots_remaining; + + /* Fill the rest of available slots with fn0. */ + while (slots_remaining > 0) + { + ATEXIT (fn0); --slots_remaining; + } /* Verify that handlers registered above are inherited across fork. */ const pid_t child = fork ();
Greetings, This patch implements the other TODO in stdlib/tst-atexit-common.c 2017-08-29 Paul Pluzhnikov <ppluzhnikov@google.com> * stdlib/tst-atexit-common.c (do_test): Test support for at least 32 atexit handlers.