Message ID | 87blwuo3mz.fsf@oldenburg2.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | elf: Support elf/tst-dlopen-aout in more configurations | expand |
On 8/12/19 10:32 AM, Florian Weimer wrote: > dlopen can no longer open PIE binaries, so it is not necessary > to link the executable as non-PIE to trigger a dlopen failure. That fixes one case of the problem, which is a nice cleanup. > If we hard-code the path to the real executable, we can run the test > with and without hard-coded paths because the dlopen path will not > be recognized as the main program in both cases. (With an explict > loader invocation, the loader currently adds argv[0] to l_libname > for the main map and the dlopen call suceeds as a result; it does > not do that in standard mode.) > OK for master if you remove the wrong comment in tst-dlopen-aout.c. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > 2019-08-12 Florian Weimer <fweimer@redhat.com> > > * elf/Makefile (tests): Unconditionally add tst-dlopen-aout. > [$(build-hardcoded-path-in-tests)] (tst-dlopen-aout-no-pie): Do > not set. > * elf/tst-dlopen-aout.c: Do not included <assert.h>. > (do_test): Open the executable using an absolute path. Print > error message to standard output. > > diff --git a/elf/Makefile b/elf/Makefile > index a3eefd1b1f..e8c3458963 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -192,7 +192,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ > tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \ > tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \ > tst-unwind-ctor tst-unwind-main tst-audit13 \ > - tst-sonamemove-link tst-sonamemove-dlopen > + tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout OK. It's great to see this added back. > # reldep9 > tests-internal += loadtest unload unload2 circleload1 \ > neededtest neededtest2 neededtest3 neededtest4 \ > @@ -200,10 +200,6 @@ tests-internal += loadtest unload unload2 circleload1 \ > tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \ > tst-create_format1 > tests-container += tst-pldd > -ifeq ($(build-hardcoded-path-in-tests),yes) > -tests += tst-dlopen-aout > -tst-dlopen-aout-no-pie = yes > -endif OK. > test-srcs = tst-pathopt > selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null) > ifneq ($(selinux-enabled),1) > diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-aout.c > index deedd11ad0..4b3e3f111a 100644 > --- a/elf/tst-dlopen-aout.c > +++ b/elf/tst-dlopen-aout.c > @@ -23,10 +23,11 @@ > Note: this test currently only fails when glibc is configured with > --enable-hardcoded-path-in-tests. */ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This comment needs removing now, not to mention it's wrong. > > -#include <assert.h> > #include <dlfcn.h> > -#include <stdio.h> > #include <pthread.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <support/support.h> OK. > #include <support/xthread.h> > > __thread int x; > @@ -42,15 +43,21 @@ do_test (int argc, char *argv[]) > { > int j; > > + /* Use the full path so that the dynamic loader does not recognize > + the main program as already loaded (even with an explicit ld.so > + invocation). */ > + char *path = xasprintf ("%s/%s", support_objdir_root, "tst-dlopen-aout"); > + printf ("info: dlopen object: %s\n", path); OK. > + > for (j = 0; j < 100; ++j) > { > pthread_t thr; > void *p; > > - p = dlopen (argv[0], RTLD_LAZY); > + p = dlopen (path, RTLD_LAZY); OK. > if (p != NULL) > { > - fprintf (stderr, "dlopen unexpectedly succeeded\n"); > + puts ("error: dlopen succeeded unexpectedly"); OK. > return 1; > } > /* We create threads to force TLS allocation, which triggers > @@ -60,6 +67,7 @@ do_test (int argc, char *argv[]) > xpthread_join (thr); > } > > + free (path); > return 0; > } > >
* Florian Weimer: > + /* Use the full path so that the dynamic loader does not recognize > + the main program as already loaded (even with an explicit ld.so > + invocation). */ > + char *path = xasprintf ("%s/%s", support_objdir_root, "tst-dlopen-aout"); The computed path is not correct, the elf/ component is missing. So this test does not the right thing anymore. I'm working on fixing the explict loader invocation difference, then this won't matter. Thanks, Florian
diff --git a/elf/Makefile b/elf/Makefile index a3eefd1b1f..e8c3458963 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -192,7 +192,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \ tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \ tst-unwind-ctor tst-unwind-main tst-audit13 \ - tst-sonamemove-link tst-sonamemove-dlopen + tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout # reldep9 tests-internal += loadtest unload unload2 circleload1 \ neededtest neededtest2 neededtest3 neededtest4 \ @@ -200,10 +200,6 @@ tests-internal += loadtest unload unload2 circleload1 \ tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \ tst-create_format1 tests-container += tst-pldd -ifeq ($(build-hardcoded-path-in-tests),yes) -tests += tst-dlopen-aout -tst-dlopen-aout-no-pie = yes -endif test-srcs = tst-pathopt selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null) ifneq ($(selinux-enabled),1) diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-aout.c index deedd11ad0..4b3e3f111a 100644 --- a/elf/tst-dlopen-aout.c +++ b/elf/tst-dlopen-aout.c @@ -23,10 +23,11 @@ Note: this test currently only fails when glibc is configured with --enable-hardcoded-path-in-tests. */ -#include <assert.h> #include <dlfcn.h> -#include <stdio.h> #include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <support/support.h> #include <support/xthread.h> __thread int x; @@ -42,15 +43,21 @@ do_test (int argc, char *argv[]) { int j; + /* Use the full path so that the dynamic loader does not recognize + the main program as already loaded (even with an explicit ld.so + invocation). */ + char *path = xasprintf ("%s/%s", support_objdir_root, "tst-dlopen-aout"); + printf ("info: dlopen object: %s\n", path); + for (j = 0; j < 100; ++j) { pthread_t thr; void *p; - p = dlopen (argv[0], RTLD_LAZY); + p = dlopen (path, RTLD_LAZY); if (p != NULL) { - fprintf (stderr, "dlopen unexpectedly succeeded\n"); + puts ("error: dlopen succeeded unexpectedly"); return 1; } /* We create threads to force TLS allocation, which triggers @@ -60,6 +67,7 @@ do_test (int argc, char *argv[]) xpthread_join (thr); } + free (path); return 0; }