| Message ID | 1436941393-20089-1-git-send-email-siddhesh@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 >
On 07/15/2015 02:23 AM, 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. > +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); > +} We could crash for any number of reasons in printf. May I suggest walking _r_debug->r_map and checking l_name to see if the DSO is still loaded? Just like the debugger would do? This doesn't require a SIGSEGV handler at all? Cheers, Carlos.
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 ()