[ping][PATCH v3] Remove Linuxism from tst-tls-atexit

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

Ping!

On Wed, Jul 15, 2015 at 11:53:13AM +0530, Siddhesh Poyarekar wrote:
> v3: I renamed some functions to make it clearer what they were doing. 
> I also added foo_var to test reachability of the DSO address space
> without side-effects.
> 
> The tst-tls-atexit test case searches for its module in /proc/PID/maps
> to verify that it is unloaded, which is a Linux-specific test.  This
> patch makes the test generic by trying to call foo (the symbol
> obtained from dlsym) and traps SIGSEGV momentarily to see if the crash
> occurred.
> 
> Verified that the test continues to succeed on x86_64.  There is a bug
> in the test case where it calls dlclose once again, which is actually
> incorrect but still manages to unload the DSO thanks to an existing
> bug in __tls_call_dtors.  This will be fixed in a later patch which
> also fixes up the __cxa_thread_atexit_impl implementation.  I have
> added a FIXME comment to that call momentarily, which I will remove
> when I fix the problem.
> 
> 	* stdlib/tst-tls-atexit-lib.c (foo_var): New variable.
> 	(do_foo): Rename to reg_dtor.
> 	* stdlib/tst-tls-atexit.c: (segv_handler): New function.
> 	(load): Rename to reg_dtor_and_close.  Move dlopen to...
> 	(do_test): ... here.  Use FOO_VAR from the DSO to test its
> 	availability.  Expect SIGSEGV after the DSO is closed.
> ---
>  stdlib/tst-tls-atexit-lib.c |  3 +-
>  stdlib/tst-tls-atexit.c     | 96 ++++++++++++++++++++++++++-------------------
>  2 files changed, 58 insertions(+), 41 deletions(-)
> 
> diff --git a/stdlib/tst-tls-atexit-lib.c b/stdlib/tst-tls-atexit-lib.c
> index 2945379..7e6e935 100644
> --- a/stdlib/tst-tls-atexit-lib.c
> +++ b/stdlib/tst-tls-atexit-lib.c
> @@ -19,6 +19,7 @@
>  #include <stdlib.h>
>  
>  extern void *__dso_handle;
> +int foo_var = 42;
>  
>  typedef struct
>  {
> @@ -31,7 +32,7 @@ void A_dtor (void *obj)
>    ((A *)obj)->val = obj;
>  }
>  
> -void do_foo (void)
> +void reg_dtor (void)
>  {
>    static __thread A b;
>    __cxa_thread_atexit_impl (A_dtor, &b, __dso_handle);
> diff --git a/stdlib/tst-tls-atexit.c b/stdlib/tst-tls-atexit.c
> index 0c6c499..0a9e920 100644
> --- a/stdlib/tst-tls-atexit.c
> +++ b/stdlib/tst-tls-atexit.c
> @@ -16,10 +16,11 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -/* There are two tests in this test case.  The first is implicit where it is
> +/* There are two parts in this test case.  The first is implicit where it is
>     assumed that the destructor call on exit of the LOAD function does not
>     segfault.  The other is a verification that after the thread has exited, a
> -   dlclose will unload the DSO.  */
> +   dlclose will unload the DSO, which is done by calling into the DSO and
> +   expecting that call to segfault.  */
>  
>  #include <dlfcn.h>
>  #include <pthread.h>
> @@ -28,31 +29,30 @@
>  #include <string.h>
>  #include <errno.h>
>  
> -void *handle;
> -
> -void *
> -load (void *u)
> +static void
> +segv_handler (int sig)
>  {
> -  handle = dlopen ("$ORIGIN/tst-tls-atexit-lib.so", RTLD_LAZY);
> -  if (handle == NULL)
> -    {
> -      printf ("Unable to load DSO: %s\n", dlerror ());
> -      return (void *) (uintptr_t) 1;
> -    }
> +  /* All good.  */
> +  _exit (0);
> +}
>  
> -  void (*foo) (void) = (void (*) (void)) dlsym (handle, "do_foo");
> +/* Accept a valid handle returned by DLOPEN, load the reg_dtor symbol to
> +   register a destructor and then call dlclose on the handle.  The dlclose
> +   should not unload the DSO since the destructor has not been called yet.  */
> +static void *
> +reg_dtor_and_close (void *h)
> +{
> +  void (*reg_dtor) (void) = (void (*) (void)) dlsym (h, "reg_dtor");
>  
> -  if (foo == NULL)
> +  if (reg_dtor == NULL)
>      {
>        printf ("Unable to find symbol: %s\n", dlerror ());
> -      exit (1);
> +      return (void *) (uintptr_t) 1;
>      }
>  
> -  foo ();
> +  reg_dtor ();
>  
> -  /* This should not unload the DSO.  If it does, then the thread exit will
> -     result in a segfault.  */
> -  dlclose (handle);
> +  dlclose (h);
>  
>    return NULL;
>  }
> @@ -64,7 +64,27 @@ do_test (void)
>    int ret;
>    void *thr_ret;
>  
> -  if ((ret = pthread_create (&t, NULL, load, NULL)) != 0)
> +  /* Load the DSO.  */
> +  void *handle = dlopen ("$ORIGIN/tst-tls-atexit-lib.so", RTLD_LAZY);
> +  if (handle == NULL)
> +    {
> +      printf ("Unable to load DSO: %s\n", dlerror ());
> +      return 1;
> +    }
> +
> +  /* FOO is our variable to test if the DSO is unloaded.  */
> +  int *foo = dlsym (handle, "foo_var");
> +
> +  if (foo == NULL)
> +    {
> +      printf ("Unable to find symbol do_foo: %s\n", dlerror ());
> +      return 1;
> +    }
> +
> +  /* Print FOO to be sure that it is working OK.  */
> +  printf ("%d\n", *foo);
> +
> +  if ((ret = pthread_create (&t, NULL, reg_dtor_and_close, handle)) != 0)
>      {
>        printf ("pthread_create failed: %s\n", strerror (ret));
>        return 1;
> @@ -79,32 +99,28 @@ do_test (void)
>    if (thr_ret != NULL)
>      return 1;
>  
> -  /* Now this should unload the DSO.  */
> +  /* Now this should unload the DSO.  FIXME: This is a bug, calling dlclose
> +     like this is actually wrong, but it works because cxa_thread_atexit_impl
> +     has a bug which results in dlclose allowing this to work.  */
>    dlclose (handle);
>  
> -  /* Run through our maps and ensure that the DSO is unloaded.  */
> -  FILE *f = fopen ("/proc/self/maps", "r");
> -
> -  if (f == NULL)
> +  /* Handle SIGSEGV.  We don't use the EXPECTED_SIGNAL macro because we want to
> +     detect any other SIGSEGVs as failures.  */
> +  struct sigaction sa, old_sa;
> +  sa.sa_handler = segv_handler;
> +  sigemptyset (&sa.sa_mask);
> +  sa.sa_flags = 0;
> +  if (sigaction (SIGSEGV, &sa, &old_sa) < 0)
>      {
> -      perror ("Failed to open /proc/self/maps");
> -      fprintf (stderr, "Skipping verification of DSO unload\n");
> -      return 0;
> +      printf ("Failed to set SIGSEGV handler: %m\n");
> +      return 1;
>      }
>  
> -  char *line = NULL;
> -  size_t s = 0;
> -  while (getline (&line, &s, f) > 0)
> -    {
> -      if (strstr (line, "tst-tls-atexit-lib.so"))
> -        {
> -	  printf ("DSO not unloaded yet:\n%s", line);
> -	  return 1;
> -	}
> -    }
> -  free (line);
> +  /* This should crash...  */
> +  printf ("%d\n", *foo);
>  
> -  return 0;
> +  /* ... or else we fail.  */
> +  return 1;
>  }
>  
>  #define TEST_FUNCTION do_test ()
> -- 
> 2.4.3
>