[PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify inheritance across fork

[PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify inheritance across fork

[Paul Pluzhnikov]

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

Greetings,

Attached patch implements one of the TODOs in stdlib/tst-atexit-common.c


2017-08-28  Paul Pluzhnikov  <ppluzhnikov@google.com>

        * stdlib/tst-atexit-common.c (do_test): Test handler inheritance
        by child.

-- 
Paul Pluzhnikov

[-- Attachment #2: glibc-exit-fork-20170828.txt --]
[-- Type: text/plain, Size: 2263 bytes --]

diff --git a/stdlib/tst-atexit-common.c b/stdlib/tst-atexit-common.c
index 262235a478..99b00bf3aa 100644
--- a/stdlib/tst-atexit-common.c
+++ b/stdlib/tst-atexit-common.c
@@ -21,11 +21,20 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <sys/wait.h>
 
 #define MAX_ATEXIT 20  /* Large enough for current set of invocations.  */
 static char crumbs[MAX_ATEXIT];
 static int next_slot = 0;
 
+/* Helper: flush stdout and _exit.  */
+static void
+_exit_with_flush (int code)
+{
+  fflush (stdout);
+  _exit (code);
+}
+
 static void
 fn0 (void)
 {
@@ -60,11 +69,11 @@ fn_final (void)
   const char expected[] = "3021121130211";
 
   if (strcmp (crumbs, expected) == 0)
-    _exit (0);
+    _exit_with_flush (0);
 
   printf ("crumbs:   %s\n", crumbs);
   printf ("expected: %s\n", expected);
-  _exit (1);
+  _exit_with_flush (1);
 }
 
 /* This is currently just a basic test to verify that exit handlers execute
@@ -72,8 +81,7 @@ fn_final (void)
 
    TODO: Additional tests that we should do:
    1. POSIX says we need to support at least ATEXIT_MAX
-   2. Verify that fork'd child inherit the registrations of the parent.
-   3. ...  */
+   2. ...  */
 
 static int
 do_test (void)
@@ -88,6 +96,40 @@ do_test (void)
   ATEXIT (fn1);
   ATEXIT (fn3);
 
+  /* Verify that handlers registered above are inherited across fork.  */
+  const pid_t child = fork ();
+  switch (child)
+    {
+    case -1:
+      printf ("fork: %m\n");
+      _exit_with_flush (3);
+    case 0:  /* Child.  */
+      break;
+    default:
+      {
+	int status;
+	const pid_t exited = waitpid (child, &status, 0);
+	if (child != exited)
+	  {
+	    printf ("unexpected child: %d, expected %d\n", exited, child);
+	    _exit_with_flush (4);
+	  }
+	if (status != 0)
+	  {
+	    if (WIFEXITED (status))
+	      printf ("unexpected exit status %d from child %d\n",
+		      WEXITSTATUS (status), child);
+	    else if (WIFSIGNALED (status))
+	      printf ("unexpected signal %d from child %d\n",
+		      WTERMSIG (status), child);
+	    else
+	      printf ("unexpected status %d from child %d\n", status, child);
+	    _exit_with_flush (5);
+	  }
+      }
+      break;
+    }
+
   EXIT (2);  /* If we see this exit code, fn_final must have not worked.  */
 }
 

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify inheritance across fork

[Carlos O'Donell]

On 08/28/2017 01:16 PM, Paul Pluzhnikov wrote:
> Greetings,
> 
> Attached patch implements one of the TODOs in stdlib/tst-atexit-common.c
> 
> 
> 2017-08-28  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
>         * stdlib/tst-atexit-common.c (do_test): Test handler inheritance
>         by child.
> 

This looks good to me.

Thanks for adding the child inheritance test.

Checking for ATEXIT_MAX shouldn't be too much harder... ;-)

Cheers,
Carlos.

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify inheritance across fork

[Florian Weimer]

On 08/28/2017 09:47 PM, Carlos O'Donell wrote:

> Checking for ATEXIT_MAX shouldn't be too much harder... ;-)

POSIX does not allow us to define _SC_ATEXIT_MAX because there is no
actual limit (beyond available memory).  The INT_MAX return value is a bug.

I filed a bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22020

Thanks,
Florian

Re: [PATCH] Extend tst-{atexit,at_quick_exit,cxa_atexit,onexit} to verify inheritance across fork

[Carlos O'Donell]

On 08/28/2017 04:01 PM, Florian Weimer wrote:
> On 08/28/2017 09:47 PM, Carlos O'Donell wrote:
> 
>> Checking for ATEXIT_MAX shouldn't be too much harder... ;-)
> 
> POSIX does not allow us to define _SC_ATEXIT_MAX because there is no
> actual limit (beyond available memory).  The INT_MAX return value is a bug.

My apologies, I should have been clearer.

The test case *must* test that you can at least register 32 handlers without
error since POSIX Issue 7 mandates this is the minimum number that must be
supported. This way the test case checks for the minimum.

We don't need to check the maximum. Checking for the maximum is more expensive
than just making sure you can register and call 32 handlers.

> I filed a bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22020

Now you've added the need for another test case which should verify that
_SC_ATEXIT_MAX returns -1?

Cheers,
Carlos.