[PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers

[PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 259 bytes --]

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.


-- 
Paul Pluzhnikov

[-- Attachment #2: glibc-atexit-max-20170829.txt --]
[-- Type: text/plain, Size: 2075 bytes --]

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 ();

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers

[Carlos O'Donell]

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.
-- 
Cheers,
Carlos.

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers

[H.J. Lu]

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]$

-- 
H.J.

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

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.




-- 
Paul Pluzhnikov

[-- Attachment #2: glibc-atexit-max-20170901.txt --]
[-- Type: text/plain, Size: 808 bytes --]

diff --git a/stdlib/tst-atexit-common.c b/stdlib/tst-atexit-common.c
index d6dcf08cdd..9ab8c1aea5 100644
--- a/stdlib/tst-atexit-common.c
+++ b/stdlib/tst-atexit-common.c
@@ -30,7 +30,10 @@
    check that we support at least the minimum required.  */
 #define MAX_ATEXIT 32
 
-static char crumbs[MAX_ATEXIT];
+/* Arbitrary sequence matching current registrations.  */
+const char expected[] = "00000000000000000000000003021121130211";
+
+static char crumbs[sizeof (expected)];
 static int next_slot = 0;
 
 /* Helper: flush stdout and _exit.  */
@@ -71,9 +74,6 @@ fn3 (void)
 static void
 fn_final (void)
 {
-  /* Arbitrary sequence matching current registrations.  */
-  const char expected[] = "00000000000000000000000003021121130211";
-
   if (strcmp (crumbs, expected) == 0)
     _exit_with_flush (0);
 

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers

[H.J. Lu]

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]$


-- 
H.J.

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers

[Paul Pluzhnikov]

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,
-- 
Paul Pluzhnikov

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers

[H.J. Lu]

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.


-- 
H.J.