Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify minimum number of supported handlers
diff mbox series

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
Related show

Commit Message

Paul Pluzhnikov Aug. 29, 2017, 6:11 p.m. UTC
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.

Comments

Carlos O'Donell Sept. 1, 2017, 12:40 a.m. UTC | #1
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.
H.J. Lu Sept. 1, 2017, 6:03 p.m. UTC | #2
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]$
Paul Pluzhnikov Sept. 1, 2017, 6:37 p.m. UTC | #3
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.
H.J. Lu Sept. 1, 2017, 6:51 p.m. UTC | #4
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]$
Paul Pluzhnikov Sept. 1, 2017, 7:09 p.m. UTC | #5
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,
H.J. Lu Sept. 1, 2017, 7:12 p.m. UTC | #6
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.

Patch
diff mbox series

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