| Message ID | 87woh7t7f7.fsf@oldenburg2.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | ld.so: Support moving versioned symbols between sonames [BZ #24741] | expand |
On Thu, Jun 27, 2019 at 10:44 AM Florian Weimer <fweimer@redhat.com> wrote: > > This change should be fully backwards-compatible because the old > code aborted the load if a soname mismatch was encountered > (instead of searching further for a matching symbol). This means > that no different symbols are found. I am not familiar enough with the guts of the dynamic linker to review your code changes, but I endorse the idea. This should make it possible to eliminate most of the double definitions of functions in libc.so and libpthread.so without needing to leave forwarding stubs behind in libpthread. zw
* Zack Weinberg: > On Thu, Jun 27, 2019 at 10:44 AM Florian Weimer <fweimer@redhat.com> wrote: >> >> This change should be fully backwards-compatible because the old >> code aborted the load if a soname mismatch was encountered >> (instead of searching further for a matching symbol). This means >> that no different symbols are found. > > I am not familiar enough with the guts of the dynamic linker to review > your code changes, but I endorse the idea. This should make it > possible to eliminate most of the double definitions of functions in > libc.so and libpthread.so without needing to leave forwarding stubs > behind in libpthread. Right, I want to use this to fix bug 20188, among other things. It's also a prerequisite for weak symbol version support. (In my interpretation of Ulrich's design, a weak symbol version reference should be generated by the link editor if all symbols that reference this version are weak.) ld.so is expected to support those already, but any use of them currently runs into that error: <https://sourceware.org/bugzilla/show_bug.cgi?id=24718#c16> Thanks, Florian
Hi, Le jeudi 27 juin 2019 à 16:43 +0200, Florian Weimer a écrit : > This change should be fully backwards-compatible because the old > code aborted the load if a soname mismatch was encountered > (instead of searching further for a matching symbol). This means > that no different symbols are found. > > The soname check was explicitly disabled for the skip_map != NULL > case. However, this only happens with dl(v)sym and RTLD_NEXT, > and those lookups do not come with a verneed entry that could be used > for the check. > > The error check was already explicitly disabled for the skip_map != > NULL case, that is, when dl(v)sym was called with RTLD_NEXT. But > _dl_vsym always sets filename in the struct r_found_version argument > to NULL, so the check was not active anyway. This means that > symbol lookup results for the skip_map != NULL case do not change, > either. > > 2019-06-27 Florian Weimer <fweimer@redhat.com> > > [BZ #24741] > * elf/dl-lookup.c (do_lookup_x): Do not fail if there is a soname > mismatch in a versioned symbol reference. > (_dl_lookup_symbol_x): Do not report soname mismatch failures. > * elf/Makefile [$(build-shared)] (tests): Add tst-sonamemove, > tst-sonamemove-dlopen. > (module-names): Add tst-sonamemove-linkmod1, > tst-sonamemove-runmod1, tst-sonamemove-runmod2. > (LDFLAGS-tst-sonamemove-linkmod1.so): Set. > (LDFLAGS-tst-sonamemove-runmod1.so): Likewise. > (LDFLAGS-tst-sonamemove-runmod2.so): Likewise. > (tst-sonamemove-runmod1.so): Link against > tst-sonamemove-runmod2.so. > (tst-sonamemove): Link against tst-sonamemove-linkmod1.so. > (tst-sonamemove.out): Depend on tst-sonamemove-runmod1.so, > tst-sonamemove-runmod2.so. > (tst-sonamemove-dlopen.out): Likewise. > * elf/tst-sonamemove.c: New file. > * elf/tst-sonamemove-dlopen.c: Likewise. > * elf/tst-sonamemove-linkmod1.c: Likewise. > * elf/tst-sonamemove-linkmod1.map: Likewise. > * elf/tst-sonamemove-runmod1.c: Likewise. > * elf/tst-sonamemove-runmod1.map: Likewise. > * elf/tst-sonamemove-runmod2.c: Likewise. > * elf/tst-sonamemove-runmod2.map: Likewise. > * support/xdlfcn.h (xdlvsym): Declare function. > * support/xdlfcn.c (xdlvsym): Define funciton. > > diff --git a/NEWS b/NEWS > index 8a2fecef47..8cea9f5825 100644 > --- a/NEWS > +++ b/NEWS > @@ -34,6 +34,12 @@ Major new features: > pointer subtraction within the allocated object, where results might > overflow the ptrdiff_t type. > > +* The dynamic linker no longer refuses to load objects which reference > + versioned symbols whose implementation has moved to a different soname > + since the object has been linked. The old error message, symbol > + FUNCTION-NAME, version SYMBOL-VERSION not defined in file DSO-NAME with > + link time reference, is gone. > + > Deprecated and removed features, and other changes affecting compatibility: > > * The functions clock_gettime, clock_getres, clock_settime, > diff --git a/elf/Makefile b/elf/Makefile > index 27a2fa8c14..76b0565054 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -191,7 +191,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ > tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \ > 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-unwind-ctor tst-unwind-main tst-audit13 \ > + tst-sonamemove tst-sonamemove-dlopen tst-sonamemove could be name tst-sonamemove-link to make obvious the difference with -dlopen. > # reldep9 > tests-internal += loadtest unload unload2 circleload1 \ > neededtest neededtest2 neededtest3 neededtest4 \ > @@ -281,7 +282,8 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \ > tst-nodelete-dlclose-dso tst-nodelete-dlclose-plugin \ > tst-main1mod tst-libc_dlvsym-dso tst-absolute-sym-lib \ > tst-absolute-zero-lib tst-big-note-lib tst-unwind-ctor-lib \ > - tst-audit13mod1 > + tst-audit13mod1 tst-sonamemove-linkmod1 \ > + tst-sonamemove-runmod1 tst-sonamemove-runmod2 > # Most modules build with _ISOMAC defined, but those filtered out > # depend on internal headers. > modules-names-tests = $(filter-out ifuncmod% tst-libc_dlvsym-dso tst-tlsmod%,\ > @@ -1410,6 +1412,28 @@ $(objpfx)tst-audit13.out: $(objpfx)tst-audit13mod1.so > LDFLAGS-tst-audit13mod1.so = -Wl,-z,lazy > tst-audit13-ENV = LD_AUDIT=$(objpfx)tst-audit13mod1.so > > +# tst-sonamemove links against an older implementation of the library. > +LDFLAGS-tst-sonamemove-linkmod1.so = \ > + -Wl,--version-script=tst-sonamemove-linkmod1.map \ > + -Wl,-soname,tst-sonamemove-runmod1.so > +LDFLAGS-tst-sonamemove-runmod1.so = -Wl,--no-as-needed \ > + -Wl,--version-script=tst-sonamemove-runmod1.map \ > + -Wl,-soname,tst-sonamemove-runmod1.so > +LDFLAGS-tst-sonamemove-runmod2.so = \ > + -Wl,--version-script=tst-sonamemove-runmod2.map \ > + -Wl,-soname,tst-sonamemove-runmod2.so > +$(objpfx)tst-sonamemove-runmod1.so: $(objpfx)tst-sonamemove-runmod2.so > +# Link against the link module, but depend on the run-time modules > +# for execution. > +$(objpfx)tst-sonamemove: $(objpfx)tst-sonamemove-linkmod1.so > +$(objpfx)tst-sonamemove.out: \ > + $(objpfx)tst-sonamemove-runmod1.so \ > + $(objpfx)tst-sonamemove-runmod2.so > +$(objpfx)tst-sonamemove-dlopen: $(libdl) > +$(objpfx)tst-sonamemove.out: \ > + $(objpfx)tst-sonamemove-runmod1.so \ > + $(objpfx)tst-sonamemove-runmod2.so > + The last three lines duplicate those above. > # Override -z defs, so that we can reference an undefined symbol. > # Force lazy binding for the same reason. > LDFLAGS-tst-latepthreadmod.so = \ [...] > diff --git a/elf/tst-sonamemove-dlopen.c b/elf/tst-sonamemove-dlopen.c > new file mode 100644 > index 0000000000..c496705044 > --- /dev/null > +++ b/elf/tst-sonamemove-dlopen.c > @@ -0,0 +1,35 @@ > +/* Check that a moved versioned symbol can be found using dlsym, dlvsym. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>;. */ > + > +#include <stddef.h> > +#include <support/check.h> > +#include <support/xdlfcn.h> > + > +static int > +do_test (void) > +{ > + /* tst-sonamemove-runmod1.so does not define moved_function, but it > + depends on tst-sonamemove-runmod2.so, which does. */ > + void *handle = xdlopen ("tst-sonamemove-runmod1.so", RTLD_NOW); > + TEST_VERIFY (xdlsym (handle, "moved_function") != NULL); > + TEST_VERIFY (xdlvsym (handle, "moved_function", "SONAME_MOVE") != NULL); > + > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/elf/tst-sonamemove-linkmod1.c b/elf/tst-sonamemove-linkmod1.c > new file mode 100644 > index 0000000000..b8a354e5e3 > --- /dev/null > +++ b/elf/tst-sonamemove-linkmod1.c > @@ -0,0 +1,25 @@ > +/* Link interface for (lack of) soname matching in versioned symbol refs. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>;. */ > + > +/* This function moved from tst-sonamemove-runmod1.so. This module is > + intended for linking only, to simulate an old application which was > + linked against an older version of the library. */ > +void > +moved_function (void) > +{ > +} > diff --git a/elf/tst-sonamemove-linkmod1.map b/elf/tst-sonamemove-linkmod1.map > new file mode 100644 > index 0000000000..8fe5904018 > --- /dev/null > +++ b/elf/tst-sonamemove-linkmod1.map > @@ -0,0 +1,3 @@ > +SONAME_MOVE { > + global: moved_function; > +}; > diff --git a/elf/tst-sonamemove-runmod1.c b/elf/tst-sonamemove-runmod1.c > new file mode 100644 > index 0000000000..5c409e2289 > --- /dev/null > +++ b/elf/tst-sonamemove-runmod1.c > @@ -0,0 +1,23 @@ > +/* Run-time module whose moved_function moved to a library dependency. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>;. */ > + > +/* Dummy function to add the required symbol version. */ > +void > +other_function (void) > +{ > +} > diff --git a/elf/tst-sonamemove-runmod1.map b/elf/tst-sonamemove-runmod1.map > new file mode 100644 > index 0000000000..2ea81c6e6f > --- /dev/null > +++ b/elf/tst-sonamemove-runmod1.map > @@ -0,0 +1,3 @@ > +SONAME_MOVE { > + global: other_function; > +}; > diff --git a/elf/tst-sonamemove-runmod2.c b/elf/tst-sonamemove-runmod2.c > new file mode 100644 > index 0000000000..b5e482eff5 > --- /dev/null > +++ b/elf/tst-sonamemove-runmod2.c > @@ -0,0 +1,24 @@ > +/* Run-time module with the actual implementation of moved_function. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>;. */ > + > +/* In the test scenario, this function was originally in > + tst-sonamemove-runmod1.so. */ > +void > +moved_function (void) > +{ > +} > diff --git a/elf/tst-sonamemove-runmod2.map b/elf/tst-sonamemove-runmod2.map > new file mode 100644 > index 0000000000..8fe5904018 > --- /dev/null > +++ b/elf/tst-sonamemove-runmod2.map > @@ -0,0 +1,3 @@ > +SONAME_MOVE { > + global: moved_function; > +}; > diff --git a/elf/tst-sonamemove.c b/elf/tst-sonamemove.c > new file mode 100644 > index 0000000000..a80ebcb36f > --- /dev/null > +++ b/elf/tst-sonamemove.c > @@ -0,0 +1,30 @@ > +/* Check that a versioned symbol can move from one library to another. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>;. */ > + > +/* moved_function is linked against tst-sonamemove-runmod1.so, but the > + actual implementation is in tst-sonamemove-runmod1.so. */ I found this comment unclear. tst-sonamemove is linked against tst- sonamemove-linkmod1.so, which has tst-sonamemove-runmod1.so soname. When tst-sonamemove is run, tst-sonamemove-runmod1.so is loaded, which imply loading tst-sonamemove-runmod2.so as a dependency. tst- sonamemove-runmod2.so is where move_function's implementation is. > +void moved_function (void); > + > +static int > +do_test (void) > +{ > + moved_function (); > + return 0; > +} > + > +#include <support/test-driver.c> Regards.
* Yann Droneaud: >> diff --git a/elf/Makefile b/elf/Makefile >> index 27a2fa8c14..76b0565054 100644 >> --- a/elf/Makefile >> +++ b/elf/Makefile >> @@ -191,7 +191,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ >> tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \ >> 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-unwind-ctor tst-unwind-main tst-audit13 \ >> + tst-sonamemove tst-sonamemove-dlopen > > tst-sonamemove could be name tst-sonamemove-link to make obvious the > difference with -dlopen. Good idea. >> +$(objpfx)tst-sonamemove-dlopen: $(libdl) >> +$(objpfx)tst-sonamemove.out: \ >> + $(objpfx)tst-sonamemove-runmod1.so \ >> + $(objpfx)tst-sonamemove-runmod2.so >> + > > The last three lines duplicate those above. Fixed. >> +/* moved_function is linked against tst-sonamemove-runmod1.so, but the >> + actual implementation is in tst-sonamemove-runmod1.so. */ > > I found this comment unclear. tst-sonamemove is linked against tst- > sonamemove-linkmod1.so, which has tst-sonamemove-runmod1.so soname. > When tst-sonamemove is run, tst-sonamemove-runmod1.so is loaded, which > imply loading tst-sonamemove-runmod2.so as a dependency. tst- > sonamemove-runmod2.so is where move_function's implementation is. I tried to explain this more explictly: /* At link time, moved_function is bound to the symbol version SONAME_MOVE in tst-sonamemove-runmod1.so, using the tst-sonamemove-linkmod1.so stub object. At run time, the process loads the real tst-sonamemove-runmod1.so, which depends on tst-sonamemove-runmod2.so. tst-sonamemove-runmod1.so does not define moved_function, but tst-sonamemove-runmod2.so does. The net effect is that the versioned symbol moved_function@SONAME_MOVE moved from the soname tst-sonamemove-linkmod1.so at link time to the soname tst-sonamemove-linkmod2.so at run time. */ Thanks, Florian ld.so: Support moving versioned symbols between sonames [BZ #24741] This change should be fully backwards-compatible because the old code aborted the load if a soname mismatch was encountered (instead of searching further for a matching symbol). This means that no different symbols are found. The soname check was explicitly disabled for the skip_map != NULL case. However, this only happens with dl(v)sym and RTLD_NEXT, and those lookups do not come with a verneed entry that could be used for the check. The error check was already explicitly disabled for the skip_map != NULL case, that is, when dl(v)sym was called with RTLD_NEXT. But _dl_vsym always sets filename in the struct r_found_version argument to NULL, so the check was not active anyway. This means that symbol lookup results for the skip_map != NULL case do not change, either. 2019-06-27 Florian Weimer <fweimer@redhat.com> [BZ #24741] * elf/dl-lookup.c (do_lookup_x): Do not fail if there is a soname mismatch in a versioned symbol reference. (_dl_lookup_symbol_x): Do not report soname mismatch failures. * elf/Makefile [$(build-shared)] (tests): Add tst-sonamemove-link, tst-sonamemove-dlopen. (module-names): Add tst-sonamemove-linkmod1, tst-sonamemove-runmod1, tst-sonamemove-runmod2. (LDFLAGS-tst-sonamemove-linkmod1.so): Set. (LDFLAGS-tst-sonamemove-runmod1.so): Likewise. (LDFLAGS-tst-sonamemove-runmod2.so): Likewise. (tst-sonamemove-runmod1.so): Link against tst-sonamemove-runmod2.so. (tst-sonamemove-link): Link against tst-sonamemove-linkmod1.so. (tst-sonamemove-link.out): Depend on tst-sonamemove-runmod1.so, tst-sonamemove-runmod2.so. (tst-sonamemove-dlopen): Link with -ldl. (tst-sonamemove-dlopen.out): Likewise. * elf/tst-sonamemove-link.c: New file. * elf/tst-sonamemove-dlopen.c: Likewise. * elf/tst-sonamemove-linkmod1.c: Likewise. * elf/tst-sonamemove-linkmod1.map: Likewise. * elf/tst-sonamemove-runmod1.c: Likewise. * elf/tst-sonamemove-runmod1.map: Likewise. * elf/tst-sonamemove-runmod2.c: Likewise. * elf/tst-sonamemove-runmod2.map: Likewise. * support/xdlfcn.h (xdlvsym): Declare function. * support/xdlfcn.c (xdlvsym): Define funciton. diff --git a/NEWS b/NEWS index 8a2fecef47..8cea9f5825 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,12 @@ Major new features: pointer subtraction within the allocated object, where results might overflow the ptrdiff_t type. +* The dynamic linker no longer refuses to load objects which reference + versioned symbols whose implementation has moved to a different soname + since the object has been linked. The old error message, symbol + FUNCTION-NAME, version SYMBOL-VERSION not defined in file DSO-NAME with + link time reference, is gone. + Deprecated and removed features, and other changes affecting compatibility: * The functions clock_gettime, clock_getres, clock_settime, diff --git a/elf/Makefile b/elf/Makefile index 27a2fa8c14..a3eefd1b1f 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -191,7 +191,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \ 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-unwind-ctor tst-unwind-main tst-audit13 \ + tst-sonamemove-link tst-sonamemove-dlopen # reldep9 tests-internal += loadtest unload unload2 circleload1 \ neededtest neededtest2 neededtest3 neededtest4 \ @@ -281,7 +282,8 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \ tst-nodelete-dlclose-dso tst-nodelete-dlclose-plugin \ tst-main1mod tst-libc_dlvsym-dso tst-absolute-sym-lib \ tst-absolute-zero-lib tst-big-note-lib tst-unwind-ctor-lib \ - tst-audit13mod1 + tst-audit13mod1 tst-sonamemove-linkmod1 \ + tst-sonamemove-runmod1 tst-sonamemove-runmod2 # Most modules build with _ISOMAC defined, but those filtered out # depend on internal headers. modules-names-tests = $(filter-out ifuncmod% tst-libc_dlvsym-dso tst-tlsmod%,\ @@ -1410,6 +1412,28 @@ $(objpfx)tst-audit13.out: $(objpfx)tst-audit13mod1.so LDFLAGS-tst-audit13mod1.so = -Wl,-z,lazy tst-audit13-ENV = LD_AUDIT=$(objpfx)tst-audit13mod1.so +# tst-sonamemove links against an older implementation of the library. +LDFLAGS-tst-sonamemove-linkmod1.so = \ + -Wl,--version-script=tst-sonamemove-linkmod1.map \ + -Wl,-soname,tst-sonamemove-runmod1.so +LDFLAGS-tst-sonamemove-runmod1.so = -Wl,--no-as-needed \ + -Wl,--version-script=tst-sonamemove-runmod1.map \ + -Wl,-soname,tst-sonamemove-runmod1.so +LDFLAGS-tst-sonamemove-runmod2.so = \ + -Wl,--version-script=tst-sonamemove-runmod2.map \ + -Wl,-soname,tst-sonamemove-runmod2.so +$(objpfx)tst-sonamemove-runmod1.so: $(objpfx)tst-sonamemove-runmod2.so +# Link against the link module, but depend on the run-time modules +# for execution. +$(objpfx)tst-sonamemove-link: $(objpfx)tst-sonamemove-linkmod1.so +$(objpfx)tst-sonamemove-link.out: \ + $(objpfx)tst-sonamemove-runmod1.so \ + $(objpfx)tst-sonamemove-runmod2.so +$(objpfx)tst-sonamemove-dlopen: $(libdl) +$(objpfx)tst-sonamemove-dlopen.out: \ + $(objpfx)tst-sonamemove-runmod1.so \ + $(objpfx)tst-sonamemove-runmod2.so + # Override -z defs, so that we can reference an undefined symbol. # Force lazy binding for the same reason. LDFLAGS-tst-latepthreadmod.so = \ diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c index e3f42a1efb..eb23cca4e3 100644 --- a/elf/dl-lookup.c +++ b/elf/dl-lookup.c @@ -536,11 +536,7 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash, } skip: - /* If this current map is the one mentioned in the verneed entry - and we have not found a weak entry, it is a bug. */ - if (symidx == STN_UNDEF && version != NULL && version->filename != NULL - && __glibc_unlikely (_dl_name_match_p (version->filename, map))) - return -1; + ; } while (++i < n); @@ -810,34 +806,10 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map, /* Search the relevant loaded objects for a definition. */ for (size_t start = i; *scope != NULL; start = 0, ++scope) - { - int res = do_lookup_x (undef_name, new_hash, &old_hash, *ref, - ¤t_value, *scope, start, version, flags, - skip_map, type_class, undef_map); - if (res > 0) - break; - - if (__glibc_unlikely (res < 0) && skip_map == NULL) - { - /* Oh, oh. The file named in the relocation entry does not - contain the needed symbol. This code is never reached - for unversioned lookups. */ - assert (version != NULL); - const char *reference_name = undef_map ? undef_map->l_name : ""; - struct dl_exception exception; - /* XXX We cannot translate the message. */ - _dl_exception_create_format - (&exception, DSO_FILENAME (reference_name), - "symbol %s version %s not defined in file %s" - " with link time reference%s", - undef_name, version->name, version->filename, - res == -2 ? " (no version symbols)" : ""); - _dl_signal_cexception (0, &exception, N_("relocation error")); - _dl_exception_free (&exception); - *ref = NULL; - return 0; - } - } + if (do_lookup_x (undef_name, new_hash, &old_hash, *ref, + ¤t_value, *scope, start, version, flags, + skip_map, type_class, undef_map) != 0) + break; if (__glibc_unlikely (current_value.s == NULL)) { diff --git a/elf/tst-sonamemove-dlopen.c b/elf/tst-sonamemove-dlopen.c new file mode 100644 index 0000000000..c496705044 --- /dev/null +++ b/elf/tst-sonamemove-dlopen.c @@ -0,0 +1,35 @@ +/* Check that a moved versioned symbol can be found using dlsym, dlvsym. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stddef.h> +#include <support/check.h> +#include <support/xdlfcn.h> + +static int +do_test (void) +{ + /* tst-sonamemove-runmod1.so does not define moved_function, but it + depends on tst-sonamemove-runmod2.so, which does. */ + void *handle = xdlopen ("tst-sonamemove-runmod1.so", RTLD_NOW); + TEST_VERIFY (xdlsym (handle, "moved_function") != NULL); + TEST_VERIFY (xdlvsym (handle, "moved_function", "SONAME_MOVE") != NULL); + + return 0; +} + +#include <support/test-driver.c> diff --git a/elf/tst-sonamemove-link.c b/elf/tst-sonamemove-link.c new file mode 100644 index 0000000000..4bc3bf32f8 --- /dev/null +++ b/elf/tst-sonamemove-link.c @@ -0,0 +1,41 @@ +/* Check that a versioned symbol can move from one library to another. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* At link time, moved_function is bound to the symbol version + SONAME_MOVE in tst-sonamemove-runmod1.so, using the + tst-sonamemove-linkmod1.so stub object. + + At run time, the process loads the real tst-sonamemove-runmod1.so, + which depends on tst-sonamemove-runmod2.so. + tst-sonamemove-runmod1.so does not define moved_function, but + tst-sonamemove-runmod2.so does. + + The net effect is that the versioned symbol + moved_function@SONAME_MOVE moved from the soname + tst-sonamemove-linkmod1.so at link time to the soname + tst-sonamemove-linkmod2.so at run time. */ +void moved_function (void); + +static int +do_test (void) +{ + moved_function (); + return 0; +} + +#include <support/test-driver.c> diff --git a/elf/tst-sonamemove-linkmod1.c b/elf/tst-sonamemove-linkmod1.c new file mode 100644 index 0000000000..b8a354e5e3 --- /dev/null +++ b/elf/tst-sonamemove-linkmod1.c @@ -0,0 +1,25 @@ +/* Link interface for (lack of) soname matching in versioned symbol refs. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* This function moved from tst-sonamemove-runmod1.so. This module is + intended for linking only, to simulate an old application which was + linked against an older version of the library. */ +void +moved_function (void) +{ +} diff --git a/elf/tst-sonamemove-linkmod1.map b/elf/tst-sonamemove-linkmod1.map new file mode 100644 index 0000000000..8fe5904018 --- /dev/null +++ b/elf/tst-sonamemove-linkmod1.map @@ -0,0 +1,3 @@ +SONAME_MOVE { + global: moved_function; +}; diff --git a/elf/tst-sonamemove-runmod1.c b/elf/tst-sonamemove-runmod1.c new file mode 100644 index 0000000000..5c409e2289 --- /dev/null +++ b/elf/tst-sonamemove-runmod1.c @@ -0,0 +1,23 @@ +/* Run-time module whose moved_function moved to a library dependency. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* Dummy function to add the required symbol version. */ +void +other_function (void) +{ +} diff --git a/elf/tst-sonamemove-runmod1.map b/elf/tst-sonamemove-runmod1.map new file mode 100644 index 0000000000..2ea81c6e6f --- /dev/null +++ b/elf/tst-sonamemove-runmod1.map @@ -0,0 +1,3 @@ +SONAME_MOVE { + global: other_function; +}; diff --git a/elf/tst-sonamemove-runmod2.c b/elf/tst-sonamemove-runmod2.c new file mode 100644 index 0000000000..b5e482eff5 --- /dev/null +++ b/elf/tst-sonamemove-runmod2.c @@ -0,0 +1,24 @@ +/* Run-time module with the actual implementation of moved_function. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* In the test scenario, this function was originally in + tst-sonamemove-runmod1.so. */ +void +moved_function (void) +{ +} diff --git a/elf/tst-sonamemove-runmod2.map b/elf/tst-sonamemove-runmod2.map new file mode 100644 index 0000000000..8fe5904018 --- /dev/null +++ b/elf/tst-sonamemove-runmod2.map @@ -0,0 +1,3 @@ +SONAME_MOVE { + global: moved_function; +}; diff --git a/support/xdlfcn.c b/support/xdlfcn.c index b2e5c21134..11fe4896d1 100644 --- a/support/xdlfcn.c +++ b/support/xdlfcn.c @@ -51,6 +51,26 @@ xdlsym (void *handle, const char *symbol) return sym; } +void * +xdlvsym (void *handle, const char *symbol, const char *version) +{ + /* Clear any pending errors. */ + dlerror (); + + void *sym = dlvsym (handle, symbol, version); + + if (sym == NULL) + { + const char *error = dlerror (); + if (error != NULL) + FAIL_EXIT1 ("error: dlvsym: %s\n", error); + /* If there was no error, we found a NULL symbol. Return the + NULL value in this case. */ + } + + return sym; +} + void xdlclose (void *handle) { diff --git a/support/xdlfcn.h b/support/xdlfcn.h index c9cd810a8a..7f8d4930fc 100644 --- a/support/xdlfcn.h +++ b/support/xdlfcn.h @@ -27,6 +27,7 @@ __BEGIN_DECLS void *xdlopen (const char *filename, int flags); void *xdlmopen (Lmid_t lmid, const char *filename, int flags); void *xdlsym (void *handle, const char *symbol); +void *xdlvsym (void *handle, const char *symbol, const char *version); void xdlclose (void *handle); __END_DECLS
Hi, Le jeudi 27 juin 2019 à 19:13 +0200, Florian Weimer a écrit : > * Yann Droneaud: > > > > diff --git a/elf/Makefile b/elf/Makefile > > > index 27a2fa8c14..76b0565054 100644 > > > --- a/elf/Makefile > > > +++ b/elf/Makefile > > > @@ -191,7 +191,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ > > > tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \ > > > 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-unwind-ctor tst-unwind-main tst-audit13 \ > > > + tst-sonamemove tst-sonamemove-dlopen > > > > tst-sonamemove could be name tst-sonamemove-link to make obvious the > > difference with -dlopen. > > Good idea. > > > > +$(objpfx)tst-sonamemove-dlopen: $(libdl) > > > +$(objpfx)tst-sonamemove.out: \ > > > + $(objpfx)tst-sonamemove-runmod1.so \ > > > + $(objpfx)tst-sonamemove-runmod2.so > > > + > > > > The last three lines duplicate those above. > > Fixed. > > > > +/* moved_function is linked against tst-sonamemove-runmod1.so, but the > > > + actual implementation is in tst-sonamemove-runmod1.so. */ > > > > I found this comment unclear. tst-sonamemove is linked against tst- > > sonamemove-linkmod1.so, which has tst-sonamemove-runmod1.so soname. > > When tst-sonamemove is run, tst-sonamemove-runmod1.so is loaded, which > > imply loading tst-sonamemove-runmod2.so as a dependency. tst- > > sonamemove-runmod2.so is where move_function's implementation is. > > I tried to explain this more explictly: > > /* At link time, moved_function is bound to the symbol version > SONAME_MOVE in tst-sonamemove-runmod1.so, using the > tst-sonamemove-linkmod1.so stub object. > > At run time, the process loads the real tst-sonamemove-runmod1.so, > which depends on tst-sonamemove-runmod2.so. > tst-sonamemove-runmod1.so does not define moved_function, but > tst-sonamemove-runmod2.so does. > > The net effect is that the versioned symbol > moved_function@SONAME_MOVE moved from the soname > tst-sonamemove-linkmod1.so at link time to the soname > tst-sonamemove-linkmod2.so at run time. */ > Thanks. LGTM. Reviewed-by: Yann Droneaud <ydroneaud@opteya.com> Regards.
On 6/27/19 1:13 PM, Florian Weimer wrote: > * Yann Droneaud: > >>> diff --git a/elf/Makefile b/elf/Makefile >>> index 27a2fa8c14..76b0565054 100644 >>> --- a/elf/Makefile >>> +++ b/elf/Makefile >>> @@ -191,7 +191,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ >>> tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \ >>> 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-unwind-ctor tst-unwind-main tst-audit13 \ >>> + tst-sonamemove tst-sonamemove-dlopen >> >> tst-sonamemove could be name tst-sonamemove-link to make obvious the >> difference with -dlopen. > > Good idea. > >>> +$(objpfx)tst-sonamemove-dlopen: $(libdl) >>> +$(objpfx)tst-sonamemove.out: \ >>> + $(objpfx)tst-sonamemove-runmod1.so \ >>> + $(objpfx)tst-sonamemove-runmod2.so >>> + >> >> The last three lines duplicate those above. > > Fixed. > >>> +/* moved_function is linked against tst-sonamemove-runmod1.so, but the >>> + actual implementation is in tst-sonamemove-runmod1.so. */ >> >> I found this comment unclear. tst-sonamemove is linked against tst- >> sonamemove-linkmod1.so, which has tst-sonamemove-runmod1.so soname. >> When tst-sonamemove is run, tst-sonamemove-runmod1.so is loaded, which >> imply loading tst-sonamemove-runmod2.so as a dependency. tst- >> sonamemove-runmod2.so is where move_function's implementation is. > > I tried to explain this more explictly: > > /* At link time, moved_function is bound to the symbol version > SONAME_MOVE in tst-sonamemove-runmod1.so, using the > tst-sonamemove-linkmod1.so stub object. > > At run time, the process loads the real tst-sonamemove-runmod1.so, > which depends on tst-sonamemove-runmod2.so. > tst-sonamemove-runmod1.so does not define moved_function, but > tst-sonamemove-runmod2.so does. > > The net effect is that the versioned symbol > moved_function@SONAME_MOVE moved from the soname > tst-sonamemove-linkmod1.so at link time to the soname > tst-sonamemove-linkmod2.so at run time. */ > > Thanks, > Florian > > ld.so: Support moving versioned symbols between sonames [BZ #24741] > > This change should be fully backwards-compatible because the old > code aborted the load if a soname mismatch was encountered > (instead of searching further for a matching symbol). This means > that no different symbols are found. > > The soname check was explicitly disabled for the skip_map != NULL > case. However, this only happens with dl(v)sym and RTLD_NEXT, > and those lookups do not come with a verneed entry that could be used > for the check. > > The error check was already explicitly disabled for the skip_map != > NULL case, that is, when dl(v)sym was called with RTLD_NEXT. But > _dl_vsym always sets filename in the struct r_found_version argument > to NULL, so the check was not active anyway. This means that > symbol lookup results for the skip_map != NULL case do not change, > either. I am strong proponent of this change. I think it goes in the right direction. While symbol binding to a version is important, that version may need to be moved to another shared library and that flexibility is important during shuffling of the implementation in glibc or any other project. It could eventually allow us to split glibc into *further* smaller libraries, or unify it into a single hyper-optimized library/loader, whichever we pick, it helps. It leaves the open question: Why was the binding so specific? I don't have a good answer for that, and I think we should clarify the language of the ELF symbol versioning specification for this change. I have gone through the specification again and I see nothing that would require this behaviour. I have dropped Ulrich's text here in our wiki: https://sourceware.org/glibc/wiki/ELFSymbolVersioning There is a hint here: ~~~ These tests have to be recursively performed for all objects and their dependencies. This way it is possible to recognize libraries which are too old and don't contain all the symbols or contain incompatible implementations. Without this kind of test one could end up with runtime errors which don't provide helpful information. ~~~ Notice the wording "don't contain all the symbols", which is actually the case here when you *move* a symbol from one DSO to another, you no longer get an error that the library may be incomplete. It seems overly restrictive to me, and doesn't match the semantics of symbol interposition where you can get part of the implementation in a preloaded library and the rest in the original library. Functionally interposition supports the behaviour we want which is to have another library provide those symbols, and this change simply makes it possible to do this without LD_PRELOAD or changes in search ordering. Can we update our wiki page to reflect what glibc actually implements for symbol versioning? We could put it in git, or work with HJ to get it upstream into the ABI docs, or just edit the wiki (for now). > 2019-06-27 Florian Weimer <fweimer@redhat.com> > > [BZ #24741] > * elf/dl-lookup.c (do_lookup_x): Do not fail if there is a soname > mismatch in a versioned symbol reference. > (_dl_lookup_symbol_x): Do not report soname mismatch failures. > * elf/Makefile [$(build-shared)] (tests): Add tst-sonamemove-link, > tst-sonamemove-dlopen. > (module-names): Add tst-sonamemove-linkmod1, > tst-sonamemove-runmod1, tst-sonamemove-runmod2. > (LDFLAGS-tst-sonamemove-linkmod1.so): Set. > (LDFLAGS-tst-sonamemove-runmod1.so): Likewise. > (LDFLAGS-tst-sonamemove-runmod2.so): Likewise. > (tst-sonamemove-runmod1.so): Link against > tst-sonamemove-runmod2.so. > (tst-sonamemove-link): Link against tst-sonamemove-linkmod1.so. > (tst-sonamemove-link.out): Depend on tst-sonamemove-runmod1.so, > tst-sonamemove-runmod2.so. > (tst-sonamemove-dlopen): Link with -ldl. > (tst-sonamemove-dlopen.out): Likewise. > * elf/tst-sonamemove-link.c: New file. > * elf/tst-sonamemove-dlopen.c: Likewise. > * elf/tst-sonamemove-linkmod1.c: Likewise. > * elf/tst-sonamemove-linkmod1.map: Likewise. > * elf/tst-sonamemove-runmod1.c: Likewise. > * elf/tst-sonamemove-runmod1.map: Likewise. > * elf/tst-sonamemove-runmod2.c: Likewise. > * elf/tst-sonamemove-runmod2.map: Likewise. > * support/xdlfcn.h (xdlvsym): Declare function. > * support/xdlfcn.c (xdlvsym): Define funciton. > OK for master if: - we document the change in our copy of the standard describing symbol versioning. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > diff --git a/NEWS b/NEWS > index 8a2fecef47..8cea9f5825 100644 > --- a/NEWS > +++ b/NEWS > @@ -34,6 +34,12 @@ Major new features: > pointer subtraction within the allocated object, where results might > overflow the ptrdiff_t type. > > +* The dynamic linker no longer refuses to load objects which reference > + versioned symbols whose implementation has moved to a different soname > + since the object has been linked. The old error message, symbol > + FUNCTION-NAME, version SYMBOL-VERSION not defined in file DSO-NAME with > + link time reference, is gone. OK. I actually like that the binding information tells you *where* the original linkage was from, since that information may be important some day. > + > Deprecated and removed features, and other changes affecting compatibility: > > * The functions clock_gettime, clock_getres, clock_settime, > diff --git a/elf/Makefile b/elf/Makefile > index 27a2fa8c14..a3eefd1b1f 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -191,7 +191,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ > tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \ > 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-unwind-ctor tst-unwind-main tst-audit13 \ > + tst-sonamemove-link tst-sonamemove-dlopen OK. Two new tests. > # reldep9 > tests-internal += loadtest unload unload2 circleload1 \ > neededtest neededtest2 neededtest3 neededtest4 \ > @@ -281,7 +282,8 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \ > tst-nodelete-dlclose-dso tst-nodelete-dlclose-plugin \ > tst-main1mod tst-libc_dlvsym-dso tst-absolute-sym-lib \ > tst-absolute-zero-lib tst-big-note-lib tst-unwind-ctor-lib \ > - tst-audit13mod1 > + tst-audit13mod1 tst-sonamemove-linkmod1 \ > + tst-sonamemove-runmod1 tst-sonamemove-runmod2 OK. Three new DSOs. > # Most modules build with _ISOMAC defined, but those filtered out > # depend on internal headers. > modules-names-tests = $(filter-out ifuncmod% tst-libc_dlvsym-dso tst-tlsmod%,\ > @@ -1410,6 +1412,28 @@ $(objpfx)tst-audit13.out: $(objpfx)tst-audit13mod1.so > LDFLAGS-tst-audit13mod1.so = -Wl,-z,lazy > tst-audit13-ENV = LD_AUDIT=$(objpfx)tst-audit13mod1.so > > +# tst-sonamemove links against an older implementation of the library. > +LDFLAGS-tst-sonamemove-linkmod1.so = \ > + -Wl,--version-script=tst-sonamemove-linkmod1.map \ > + -Wl,-soname,tst-sonamemove-runmod1.so OK. Same soname. > +LDFLAGS-tst-sonamemove-runmod1.so = -Wl,--no-as-needed \ > + -Wl,--version-script=tst-sonamemove-runmod1.map \ > + -Wl,-soname,tst-sonamemove-runmod1.so OK. Same soname. > +LDFLAGS-tst-sonamemove-runmod2.so = \ > + -Wl,--version-script=tst-sonamemove-runmod2.map \ > + -Wl,-soname,tst-sonamemove-runmod2.so OK. Different soname. > +$(objpfx)tst-sonamemove-runmod1.so: $(objpfx)tst-sonamemove-runmod2.so > +# Link against the link module, but depend on the run-time modules > +# for execution. > +$(objpfx)tst-sonamemove-link: $(objpfx)tst-sonamemove-linkmod1.so > +$(objpfx)tst-sonamemove-link.out: \ > + $(objpfx)tst-sonamemove-runmod1.so \ > + $(objpfx)tst-sonamemove-runmod2.so OK, Build the dsos before running the test. > +$(objpfx)tst-sonamemove-dlopen: $(libdl) > +$(objpfx)tst-sonamemove-dlopen.out: \ > + $(objpfx)tst-sonamemove-runmod1.so \ > + $(objpfx)tst-sonamemove-runmod2.so OK. likewise. > + > # Override -z defs, so that we can reference an undefined symbol. > # Force lazy binding for the same reason. > LDFLAGS-tst-latepthreadmod.so = \ > diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c > index e3f42a1efb..eb23cca4e3 100644 > --- a/elf/dl-lookup.c > +++ b/elf/dl-lookup.c > @@ -536,11 +536,7 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash, > } > > skip: > - /* If this current map is the one mentioned in the verneed entry > - and we have not found a weak entry, it is a bug. */ > - if (symidx == STN_UNDEF && version != NULL && version->filename != NULL > - && __glibc_unlikely (_dl_name_match_p (version->filename, map))) > - return -1; > + ; OK. An audit of _dl_name_match_p() which is predominantly used for this purpose shows callers in dl-load.c, dl-lookup.c, dl-misc.c, dl-version.c, and rtld.c. I found one questionable assert in dl-lookup.c 99 if (version != NULL) 100 { 101 if (__glibc_unlikely (verstab == NULL)) 102 { 103 /* We need a versioned symbol but haven't found any. If 104 this is the object which is referenced in the verneed 105 entry it is a bug in the library since a symbol must 106 not simply disappear. 107 108 It would also be a bug in the object since it means that 109 the list of required versions is incomplete and so the 110 tests in dl-version.c haven't found a problem.*/ 111 assert (version->filename == NULL 112 || ! _dl_name_match_p (version->filename, map)); 113 114 /* Otherwise we accept the symbol. */ Why would we assert here? Can we reasonably handle this error? If it is the object referenced in verneed then version->filename != NULL and so the assert triggers? That seems overly restrictive, and I expect we don't get here because earlier dl-version.c checks fail. I'm not asking you to cleanup this code, but it does beg the next question. What happens if you move the last versioned symbol out of a library? Are you allowed to stop using the version script in the library that has zero versioned symbols? I think the answer is "No." Because we still check that all libraries provide the version sets that they had at link time? Is that still OK? Everything else looks OK. > } > while (++i < n); > > @@ -810,34 +806,10 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map, > > /* Search the relevant loaded objects for a definition. */ > for (size_t start = i; *scope != NULL; start = 0, ++scope) > - { > - int res = do_lookup_x (undef_name, new_hash, &old_hash, *ref, > - ¤t_value, *scope, start, version, flags, > - skip_map, type_class, undef_map); > - if (res > 0) > - break; > - > - if (__glibc_unlikely (res < 0) && skip_map == NULL) > - { > - /* Oh, oh. The file named in the relocation entry does not > - contain the needed symbol. This code is never reached > - for unversioned lookups. */ > - assert (version != NULL); > - const char *reference_name = undef_map ? undef_map->l_name : ""; > - struct dl_exception exception; > - /* XXX We cannot translate the message. */ > - _dl_exception_create_format > - (&exception, DSO_FILENAME (reference_name), > - "symbol %s version %s not defined in file %s" > - " with link time reference%s", > - undef_name, version->name, version->filename, > - res == -2 ? " (no version symbols)" : ""); > - _dl_signal_cexception (0, &exception, N_("relocation error")); > - _dl_exception_free (&exception); > - *ref = NULL; > - return 0; > - } > - } > + if (do_lookup_x (undef_name, new_hash, &old_hash, *ref, > + ¤t_value, *scope, start, version, flags, > + skip_map, type_class, undef_map) != 0) > + break; OK. This removes the well known code that does this check from _dl_lookup_symbol_x. I didn't know about the case in do_lookup_x. > > if (__glibc_unlikely (current_value.s == NULL)) > { > diff --git a/elf/tst-sonamemove-dlopen.c b/elf/tst-sonamemove-dlopen.c > new file mode 100644 > index 0000000000..c496705044 > --- /dev/null > +++ b/elf/tst-sonamemove-dlopen.c > @@ -0,0 +1,35 @@ > +/* Check that a moved versioned symbol can be found using dlsym, dlvsym. OK. > + Copyright (C) 2019 Free Software Foundation, Inc. OK. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <stddef.h> > +#include <support/check.h> > +#include <support/xdlfcn.h> > + > +static int > +do_test (void) > +{ > + /* tst-sonamemove-runmod1.so does not define moved_function, but it > + depends on tst-sonamemove-runmod2.so, which does. */ OK. > + void *handle = xdlopen ("tst-sonamemove-runmod1.so", RTLD_NOW); > + TEST_VERIFY (xdlsym (handle, "moved_function") != NULL); > + TEST_VERIFY (xdlvsym (handle, "moved_function", "SONAME_MOVE") != NULL); OK. > + > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/elf/tst-sonamemove-link.c b/elf/tst-sonamemove-link.c > new file mode 100644 > index 0000000000..4bc3bf32f8 > --- /dev/null > +++ b/elf/tst-sonamemove-link.c > @@ -0,0 +1,41 @@ > +/* Check that a versioned symbol can move from one library to another. OK. > + Copyright (C) 2019 Free Software Foundation, Inc. OK. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +/* At link time, moved_function is bound to the symbol version > + SONAME_MOVE in tst-sonamemove-runmod1.so, using the > + tst-sonamemove-linkmod1.so stub object. OK. > + > + At run time, the process loads the real tst-sonamemove-runmod1.so, > + which depends on tst-sonamemove-runmod2.so. > + tst-sonamemove-runmod1.so does not define moved_function, but > + tst-sonamemove-runmod2.so does. OK. > + > + The net effect is that the versioned symbol > + moved_function@SONAME_MOVE moved from the soname > + tst-sonamemove-linkmod1.so at link time to the soname > + tst-sonamemove-linkmod2.so at run time. */ OK. > +void moved_function (void); > + > +static int > +do_test (void) > +{ > + moved_function (); > + return 0; OK. > +} > + > +#include <support/test-driver.c> > diff --git a/elf/tst-sonamemove-linkmod1.c b/elf/tst-sonamemove-linkmod1.c > new file mode 100644 > index 0000000000..b8a354e5e3 > --- /dev/null > +++ b/elf/tst-sonamemove-linkmod1.c > @@ -0,0 +1,25 @@ > +/* Link interface for (lack of) soname matching in versioned symbol refs. > + Copyright (C) 2019 Free Software Foundation, Inc. OK. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +/* This function moved from tst-sonamemove-runmod1.so. This module is > + intended for linking only, to simulate an old application which was > + linked against an older version of the library. */ > +void > +moved_function (void) > +{ > +} OK. > diff --git a/elf/tst-sonamemove-linkmod1.map b/elf/tst-sonamemove-linkmod1.map > new file mode 100644 > index 0000000000..8fe5904018 > --- /dev/null > +++ b/elf/tst-sonamemove-linkmod1.map > @@ -0,0 +1,3 @@ > +SONAME_MOVE { > + global: moved_function; > +}; OK. > diff --git a/elf/tst-sonamemove-runmod1.c b/elf/tst-sonamemove-runmod1.c > new file mode 100644 > index 0000000000..5c409e2289 > --- /dev/null > +++ b/elf/tst-sonamemove-runmod1.c > @@ -0,0 +1,23 @@ > +/* Run-time module whose moved_function moved to a library dependency. > + Copyright (C) 2019 Free Software Foundation, Inc. OK. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +/* Dummy function to add the required symbol version. */ > +void > +other_function (void) > +{ > +} OK. Ah, so you do need a dummy function to preserve the required version set or we still get another error? I expect the answer is "Yes" and that we're not entirely willing to remove this requirement. We're happy to move symbols, but what about keeping the original symbol set intact? Would we relax that in a subsequent patch? > diff --git a/elf/tst-sonamemove-runmod1.map b/elf/tst-sonamemove-runmod1.map > new file mode 100644 > index 0000000000..2ea81c6e6f > --- /dev/null > +++ b/elf/tst-sonamemove-runmod1.map > @@ -0,0 +1,3 @@ > +SONAME_MOVE { > + global: other_function; > +}; OK. > diff --git a/elf/tst-sonamemove-runmod2.c b/elf/tst-sonamemove-runmod2.c > new file mode 100644 > index 0000000000..b5e482eff5 > --- /dev/null > +++ b/elf/tst-sonamemove-runmod2.c > @@ -0,0 +1,24 @@ > +/* Run-time module with the actual implementation of moved_function. > + Copyright (C) 2019 Free Software Foundation, Inc. OK. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +/* In the test scenario, this function was originally in > + tst-sonamemove-runmod1.so. */ > +void > +moved_function (void) > +{ > +} OK. > diff --git a/elf/tst-sonamemove-runmod2.map b/elf/tst-sonamemove-runmod2.map > new file mode 100644 > index 0000000000..8fe5904018 > --- /dev/null > +++ b/elf/tst-sonamemove-runmod2.map > @@ -0,0 +1,3 @@ > +SONAME_MOVE { > + global: moved_function; > +}; OK. > diff --git a/support/xdlfcn.c b/support/xdlfcn.c > index b2e5c21134..11fe4896d1 100644 > --- a/support/xdlfcn.c > +++ b/support/xdlfcn.c > @@ -51,6 +51,26 @@ xdlsym (void *handle, const char *symbol) > return sym; > } > > +void * > +xdlvsym (void *handle, const char *symbol, const char *version) > +{ > + /* Clear any pending errors. */ > + dlerror (); > + > + void *sym = dlvsym (handle, symbol, version); > + > + if (sym == NULL) > + { > + const char *error = dlerror (); > + if (error != NULL) > + FAIL_EXIT1 ("error: dlvsym: %s\n", error); > + /* If there was no error, we found a NULL symbol. Return the > + NULL value in this case. */ > + } > + > + return sym; > +} OK. > + > void > xdlclose (void *handle) > { > diff --git a/support/xdlfcn.h b/support/xdlfcn.h > index c9cd810a8a..7f8d4930fc 100644 > --- a/support/xdlfcn.h > +++ b/support/xdlfcn.h > @@ -27,6 +27,7 @@ __BEGIN_DECLS > void *xdlopen (const char *filename, int flags); > void *xdlmopen (Lmid_t lmid, const char *filename, int flags); > void *xdlsym (void *handle, const char *symbol); > +void *xdlvsym (void *handle, const char *symbol, const char *version); > void xdlclose (void *handle); > > __END_DECLS > OK.
* Carlos O'Donell: > I have dropped Ulrich's text here in our wiki: Uhm, I don't see a permission notice in the original document. Did you ask Ulrich? I don't think we should block this patch on the outcome of that discussion. (I would definitely prefer a text file in Git, by the way.) > An audit of _dl_name_match_p() which is predominantly used for this purpose > shows callers in dl-load.c, dl-lookup.c, dl-misc.c, dl-version.c, and rtld.c. > > I found one questionable assert in dl-lookup.c > > 99 if (version != NULL) > 100 { > 101 if (__glibc_unlikely (verstab == NULL)) > 102 { > 103 /* We need a versioned symbol but haven't found any. If > 104 this is the object which is referenced in the verneed > 105 entry it is a bug in the library since a symbol must > 106 not simply disappear. > 107 > 108 It would also be a bug in the object since it means that > 109 the list of required versions is incomplete and so the > 110 tests in dl-version.c haven't found a problem.*/ > 111 assert (version->filename == NULL > 112 || ! _dl_name_match_p (version->filename, map)); > 113 > 114 /* Otherwise we accept the symbol. */ > > Why would we assert here? I think it's a corrupt object, where the symbol uses an index that's outside the symbol table range. For example, this code in _dl_fixup doesn't do any range checks: if (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL) { const ElfW(Half) *vernum = (const void *) D_PTR (l, l_info[VERSYMIDX (DT_VERSYM)]); ElfW(Half) ndx = vernum[ELFW(R_SYM) (reloc->r_info)] & 0x7fff; version = &l->l_versions[ndx]; if (version->hash == 0) version = NULL; } That can of course segfault, or give you bad data, leading to segfaults or asserts later. I believe the assert you quoted would fire if by accident we got data that looked like a genuine version reference, thus implying that the soname needs to have version information. (dl-version.c should verify that l_versions references are actually matched by the listed objects, so I don't see how you can get the assert without some out-of-bounds lookup.) I don't think we can write a valid test that hits that assert. > Can we reasonably handle this error? > If it is the object referenced in verneed then version->filename != NULL > and so the assert triggers? That seems overly restrictive, and I expect > we don't get here because earlier dl-version.c checks fail. I'm not asking > you to cleanup this code, but it does beg the next question. > What happens if you move the last versioned symbol out of a library? You need to ensure that the symbol has the required version definition records. I think with BFD ld, you may need to keep an arbitrarily named versioned symbol under that particular version to retain it under a non-weak version. > Are you allowed to stop using the version script in the library that > has zero versioned symbols? I think the answer is "No." Because we > still check that all libraries provide the version sets that they > had at link time? Is that still OK? Yes, I think that's acceptable. We might want to fix ld eventually to avoid the need for dummy symbol definitions, but I don't think that's immediatelly necessary. >> + This file is part of the GNU C Library. >> + >> + The GNU C Library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.1 of the License, or (at your option) any later version. >> + >> + The GNU C Library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with the GNU C Library; if not, see >> + <http://www.gnu.org/licenses/>. */ >> + >> +/* Dummy function to add the required symbol version. */ >> +void >> +other_function (void) >> +{ >> +} > > OK. Ah, so you do need a dummy function to preserve the required version > set or we still get another error? I didn't even think about it and just added the symbol. I tried now, and BFD ld starts producing weak symbol definitions, altering behavior rather drastically, if you don't do that. We definitely don't want to do that for glibc because we do not have the link module/run-time module separation there, and I think it's valuable to align the test to that (i.e. not have weak version definitions in the run-time module). > I expect the answer is "Yes" and that we're not entirely willing to > remove this requirement. We're happy to move symbols, but what about > keeping the original symbol set intact? Would we relax that in a > subsequent patch? No, I think we should keep that check by default. It's perhaps less important now that many binaries use BIND_NOW, but it's the check that prevents people from running programs with older glibc versions that do not provide the required symbol set. We should really add an option to disable the symbol matching in dl-version.c, so that you can LD_PRELOAD completely new symbol versions, but that has quite different applications. Carlos, this is yet another patch review where you write Reviewed-by: Carlos O'Donell <carlos@redhat.com> with a condition that is difficult to fulfill. (Sometimes, you also request non-trivial changes which would potentially invalidate the review.) I think it's easier for patch submitters if you delay approval until you are satisfied with the patch submissions and all your preconditions have been met.
On 6/27/19 5:12 PM, Florian Weimer wrote: > * Carlos O'Donell: > >> I have dropped Ulrich's text here in our wiki: > > Uhm, I don't see a permission notice in the original document. > Did you ask Ulrich? I have asked Ulrich for permission to use this text under a license which would allow us to carry it as documentation for how GNU symbol versioning is implemented. I don't have a reply yet. > I don't think we should block this patch on the outcome of that > discussion. I can agree to that if we agree to the need for documentation and that we'll update it to follow the changes we make. Agreed? > (I would definitely prefer a text file in Git, by the way.) Perfect, let's do that instead then. I'm removing the wiki page then. >> An audit of _dl_name_match_p() which is predominantly used for this purpose >> shows callers in dl-load.c, dl-lookup.c, dl-misc.c, dl-version.c, and rtld.c. >> >> I found one questionable assert in dl-lookup.c >> >> 99 if (version != NULL) >> 100 { >> 101 if (__glibc_unlikely (verstab == NULL)) >> 102 { >> 103 /* We need a versioned symbol but haven't found any. If >> 104 this is the object which is referenced in the verneed >> 105 entry it is a bug in the library since a symbol must >> 106 not simply disappear. >> 107 >> 108 It would also be a bug in the object since it means that >> 109 the list of required versions is incomplete and so the >> 110 tests in dl-version.c haven't found a problem.*/ >> 111 assert (version->filename == NULL >> 112 || ! _dl_name_match_p (version->filename, map)); >> 113 >> 114 /* Otherwise we accept the symbol. */ >> >> Why would we assert here? > > I think it's a corrupt object, where the symbol uses an index that's > outside the symbol table range. For example, this code in _dl_fixup > doesn't do any range checks: > > if (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > { > const ElfW(Half) *vernum = > (const void *) D_PTR (l, l_info[VERSYMIDX (DT_VERSYM)]); > ElfW(Half) ndx = vernum[ELFW(R_SYM) (reloc->r_info)] & 0x7fff; > version = &l->l_versions[ndx]; > if (version->hash == 0) > version = NULL; > } > > That can of course segfault, or give you bad data, leading to > segfaults or asserts later. I believe the assert you quoted would > fire if by accident we got data that looked like a genuine version > reference, thus implying that the soname needs to have version > information. (dl-version.c should verify that l_versions references > are actually matched by the listed objects, so I don't see how you can > get the assert without some out-of-bounds lookup.) > > I don't think we can write a valid test that hits that assert. No worries then. >> Can we reasonably handle this error? >> If it is the object referenced in verneed then version->filename != NULL >> and so the assert triggers? That seems overly restrictive, and I expect >> we don't get here because earlier dl-version.c checks fail. I'm not asking >> you to cleanup this code, but it does beg the next question. > >> What happens if you move the last versioned symbol out of a library? > > You need to ensure that the symbol has the required version definition > records. I think with BFD ld, you may need to keep an arbitrarily > named versioned symbol under that particular version to retain it > under a non-weak version. That's what I thought. >> Are you allowed to stop using the version script in the library that >> has zero versioned symbols? I think the answer is "No." Because we >> still check that all libraries provide the version sets that they >> had at link time? Is that still OK? > > Yes, I think that's acceptable. We might want to fix ld eventually to > avoid the need for dummy symbol definitions, but I don't think that's > immediatelly necessary. Agreed. >>> + This file is part of the GNU C Library. >>> + >>> + The GNU C Library is free software; you can redistribute it and/or >>> + modify it under the terms of the GNU Lesser General Public >>> + License as published by the Free Software Foundation; either >>> + version 2.1 of the License, or (at your option) any later version. >>> + >>> + The GNU C Library is distributed in the hope that it will be useful, >>> + but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>> + Lesser General Public License for more details. >>> + >>> + You should have received a copy of the GNU Lesser General Public >>> + License along with the GNU C Library; if not, see >>> + <http://www.gnu.org/licenses/>. */ >>> + >>> +/* Dummy function to add the required symbol version. */ >>> +void >>> +other_function (void) >>> +{ >>> +} >> >> OK. Ah, so you do need a dummy function to preserve the required version >> set or we still get another error? > > I didn't even think about it and just added the symbol. I tried now, > and BFD ld starts producing weak symbol definitions, altering behavior > rather drastically, if you don't do that. We definitely don't want to > do that for glibc because we do not have the link module/run-time > module separation there, and I think it's valuable to align the test > to that (i.e. not have weak version definitions in the run-time > module). Agreed. What you have is OK. >> I expect the answer is "Yes" and that we're not entirely willing to >> remove this requirement. We're happy to move symbols, but what about >> keeping the original symbol set intact? Would we relax that in a >> subsequent patch? > > No, I think we should keep that check by default. It's perhaps less > important now that many binaries use BIND_NOW, but it's the check that > prevents people from running programs with older glibc versions that > do not provide the required symbol set. Right. > We should really add an option to disable the symbol matching in > dl-version.c, so that you can LD_PRELOAD completely new symbol > versions, but that has quite different applications. OK. > Carlos, this is yet another patch review where you write > > Reviewed-by: Carlos O'Donell <carlos@redhat.com> > > with a condition that is difficult to fulfill. (Sometimes, you also > request non-trivial changes which would potentially invalidate the > review.) I think it's easier for patch submitters if you delay > approval until you are satisfied with the patch submissions and all > your preconditions have been met. I'm trying to avoid a full-day delay due to review and TZ, and get the patch through more quickly, therefore I try to provide my preconditions for accepting the patch. Your patch is OK for master as-is, and I won't block it on updating documentation. I expect to update the docs when we have them ready for committing to git. Reviewed-by: Carlos O'Donell <carlos@redhat.com>
diff --git a/NEWS b/NEWS index 8a2fecef47..8cea9f5825 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,12 @@ Major new features: pointer subtraction within the allocated object, where results might overflow the ptrdiff_t type. +* The dynamic linker no longer refuses to load objects which reference + versioned symbols whose implementation has moved to a different soname + since the object has been linked. The old error message, symbol + FUNCTION-NAME, version SYMBOL-VERSION not defined in file DSO-NAME with + link time reference, is gone. + Deprecated and removed features, and other changes affecting compatibility: * The functions clock_gettime, clock_getres, clock_settime, diff --git a/elf/Makefile b/elf/Makefile index 27a2fa8c14..76b0565054 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -191,7 +191,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \ 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-unwind-ctor tst-unwind-main tst-audit13 \ + tst-sonamemove tst-sonamemove-dlopen # reldep9 tests-internal += loadtest unload unload2 circleload1 \ neededtest neededtest2 neededtest3 neededtest4 \ @@ -281,7 +282,8 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \ tst-nodelete-dlclose-dso tst-nodelete-dlclose-plugin \ tst-main1mod tst-libc_dlvsym-dso tst-absolute-sym-lib \ tst-absolute-zero-lib tst-big-note-lib tst-unwind-ctor-lib \ - tst-audit13mod1 + tst-audit13mod1 tst-sonamemove-linkmod1 \ + tst-sonamemove-runmod1 tst-sonamemove-runmod2 # Most modules build with _ISOMAC defined, but those filtered out # depend on internal headers. modules-names-tests = $(filter-out ifuncmod% tst-libc_dlvsym-dso tst-tlsmod%,\ @@ -1410,6 +1412,28 @@ $(objpfx)tst-audit13.out: $(objpfx)tst-audit13mod1.so LDFLAGS-tst-audit13mod1.so = -Wl,-z,lazy tst-audit13-ENV = LD_AUDIT=$(objpfx)tst-audit13mod1.so +# tst-sonamemove links against an older implementation of the library. +LDFLAGS-tst-sonamemove-linkmod1.so = \ + -Wl,--version-script=tst-sonamemove-linkmod1.map \ + -Wl,-soname,tst-sonamemove-runmod1.so +LDFLAGS-tst-sonamemove-runmod1.so = -Wl,--no-as-needed \ + -Wl,--version-script=tst-sonamemove-runmod1.map \ + -Wl,-soname,tst-sonamemove-runmod1.so +LDFLAGS-tst-sonamemove-runmod2.so = \ + -Wl,--version-script=tst-sonamemove-runmod2.map \ + -Wl,-soname,tst-sonamemove-runmod2.so +$(objpfx)tst-sonamemove-runmod1.so: $(objpfx)tst-sonamemove-runmod2.so +# Link against the link module, but depend on the run-time modules +# for execution. +$(objpfx)tst-sonamemove: $(objpfx)tst-sonamemove-linkmod1.so +$(objpfx)tst-sonamemove.out: \ + $(objpfx)tst-sonamemove-runmod1.so \ + $(objpfx)tst-sonamemove-runmod2.so +$(objpfx)tst-sonamemove-dlopen: $(libdl) +$(objpfx)tst-sonamemove.out: \ + $(objpfx)tst-sonamemove-runmod1.so \ + $(objpfx)tst-sonamemove-runmod2.so + # Override -z defs, so that we can reference an undefined symbol. # Force lazy binding for the same reason. LDFLAGS-tst-latepthreadmod.so = \ diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c index e3f42a1efb..eb23cca4e3 100644 --- a/elf/dl-lookup.c +++ b/elf/dl-lookup.c @@ -536,11 +536,7 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash, } skip: - /* If this current map is the one mentioned in the verneed entry - and we have not found a weak entry, it is a bug. */ - if (symidx == STN_UNDEF && version != NULL && version->filename != NULL - && __glibc_unlikely (_dl_name_match_p (version->filename, map))) - return -1; + ; } while (++i < n); @@ -810,34 +806,10 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map, /* Search the relevant loaded objects for a definition. */ for (size_t start = i; *scope != NULL; start = 0, ++scope) - { - int res = do_lookup_x (undef_name, new_hash, &old_hash, *ref, - ¤t_value, *scope, start, version, flags, - skip_map, type_class, undef_map); - if (res > 0) - break; - - if (__glibc_unlikely (res < 0) && skip_map == NULL) - { - /* Oh, oh. The file named in the relocation entry does not - contain the needed symbol. This code is never reached - for unversioned lookups. */ - assert (version != NULL); - const char *reference_name = undef_map ? undef_map->l_name : ""; - struct dl_exception exception; - /* XXX We cannot translate the message. */ - _dl_exception_create_format - (&exception, DSO_FILENAME (reference_name), - "symbol %s version %s not defined in file %s" - " with link time reference%s", - undef_name, version->name, version->filename, - res == -2 ? " (no version symbols)" : ""); - _dl_signal_cexception (0, &exception, N_("relocation error")); - _dl_exception_free (&exception); - *ref = NULL; - return 0; - } - } + if (do_lookup_x (undef_name, new_hash, &old_hash, *ref, + ¤t_value, *scope, start, version, flags, + skip_map, type_class, undef_map) != 0) + break; if (__glibc_unlikely (current_value.s == NULL)) { diff --git a/elf/tst-sonamemove-dlopen.c b/elf/tst-sonamemove-dlopen.c new file mode 100644 index 0000000000..c496705044 --- /dev/null +++ b/elf/tst-sonamemove-dlopen.c @@ -0,0 +1,35 @@ +/* Check that a moved versioned symbol can be found using dlsym, dlvsym. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stddef.h> +#include <support/check.h> +#include <support/xdlfcn.h> + +static int +do_test (void) +{ + /* tst-sonamemove-runmod1.so does not define moved_function, but it + depends on tst-sonamemove-runmod2.so, which does. */ + void *handle = xdlopen ("tst-sonamemove-runmod1.so", RTLD_NOW); + TEST_VERIFY (xdlsym (handle, "moved_function") != NULL); + TEST_VERIFY (xdlvsym (handle, "moved_function", "SONAME_MOVE") != NULL); + + return 0; +} + +#include <support/test-driver.c> diff --git a/elf/tst-sonamemove-linkmod1.c b/elf/tst-sonamemove-linkmod1.c new file mode 100644 index 0000000000..b8a354e5e3 --- /dev/null +++ b/elf/tst-sonamemove-linkmod1.c @@ -0,0 +1,25 @@ +/* Link interface for (lack of) soname matching in versioned symbol refs. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* This function moved from tst-sonamemove-runmod1.so. This module is + intended for linking only, to simulate an old application which was + linked against an older version of the library. */ +void +moved_function (void) +{ +} diff --git a/elf/tst-sonamemove-linkmod1.map b/elf/tst-sonamemove-linkmod1.map new file mode 100644 index 0000000000..8fe5904018 --- /dev/null +++ b/elf/tst-sonamemove-linkmod1.map @@ -0,0 +1,3 @@ +SONAME_MOVE { + global: moved_function; +}; diff --git a/elf/tst-sonamemove-runmod1.c b/elf/tst-sonamemove-runmod1.c new file mode 100644 index 0000000000..5c409e2289 --- /dev/null +++ b/elf/tst-sonamemove-runmod1.c @@ -0,0 +1,23 @@ +/* Run-time module whose moved_function moved to a library dependency. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* Dummy function to add the required symbol version. */ +void +other_function (void) +{ +} diff --git a/elf/tst-sonamemove-runmod1.map b/elf/tst-sonamemove-runmod1.map new file mode 100644 index 0000000000..2ea81c6e6f --- /dev/null +++ b/elf/tst-sonamemove-runmod1.map @@ -0,0 +1,3 @@ +SONAME_MOVE { + global: other_function; +}; diff --git a/elf/tst-sonamemove-runmod2.c b/elf/tst-sonamemove-runmod2.c new file mode 100644 index 0000000000..b5e482eff5 --- /dev/null +++ b/elf/tst-sonamemove-runmod2.c @@ -0,0 +1,24 @@ +/* Run-time module with the actual implementation of moved_function. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* In the test scenario, this function was originally in + tst-sonamemove-runmod1.so. */ +void +moved_function (void) +{ +} diff --git a/elf/tst-sonamemove-runmod2.map b/elf/tst-sonamemove-runmod2.map new file mode 100644 index 0000000000..8fe5904018 --- /dev/null +++ b/elf/tst-sonamemove-runmod2.map @@ -0,0 +1,3 @@ +SONAME_MOVE { + global: moved_function; +}; diff --git a/elf/tst-sonamemove.c b/elf/tst-sonamemove.c new file mode 100644 index 0000000000..a80ebcb36f --- /dev/null +++ b/elf/tst-sonamemove.c @@ -0,0 +1,30 @@ +/* Check that a versioned symbol can move from one library to another. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* moved_function is linked against tst-sonamemove-runmod1.so, but the + actual implementation is in tst-sonamemove-runmod1.so. */ +void moved_function (void); + +static int +do_test (void) +{ + moved_function (); + return 0; +} + +#include <support/test-driver.c> diff --git a/support/xdlfcn.c b/support/xdlfcn.c index b2e5c21134..11fe4896d1 100644 --- a/support/xdlfcn.c +++ b/support/xdlfcn.c @@ -51,6 +51,26 @@ xdlsym (void *handle, const char *symbol) return sym; } +void * +xdlvsym (void *handle, const char *symbol, const char *version) +{ + /* Clear any pending errors. */ + dlerror (); + + void *sym = dlvsym (handle, symbol, version); + + if (sym == NULL) + { + const char *error = dlerror (); + if (error != NULL) + FAIL_EXIT1 ("error: dlvsym: %s\n", error); + /* If there was no error, we found a NULL symbol. Return the + NULL value in this case. */ + } + + return sym; +} + void xdlclose (void *handle) { diff --git a/support/xdlfcn.h b/support/xdlfcn.h index c9cd810a8a..7f8d4930fc 100644 --- a/support/xdlfcn.h +++ b/support/xdlfcn.h @@ -27,6 +27,7 @@ __BEGIN_DECLS void *xdlopen (const char *filename, int flags); void *xdlmopen (Lmid_t lmid, const char *filename, int flags); void *xdlsym (void *handle, const char *symbol); +void *xdlvsym (void *handle, const char *symbol, const char *version); void xdlclose (void *handle); __END_DECLS