elf: Support dlvsym within libc.so

Message ID 20171218145711.CF2EB43994576@oldenburg.str.redhat.com
State Superseded
Headers

Commit Message

Florian Weimer Dec. 18, 2017, 2:57 p.m. UTC
  This commit adds a new _dl_open_hook entry for dlvsym and implements the
function using the existing dl_lookup_symbol_x function supplied by the
dynamic loader.

2017-12-18  Florian Weimer  <fweimer@redhat.com>

	Add support for calling dlvsym from libc.so.
	* include/dlfcn.h (__libc_dlvsym): Declare.
	* elf/Makefile (tests-static-internal): Add
	tst-libc_dlvsym-static.
	(tests-internal): Add tst-libc_dlvsym.
	(modules-names): Add tst-libc_dlvsym-dso.
	(tst-libc_dlvsym, tst-libc_dlvsym-static): Link with libdl.
	(tst-libc_dlvsym-dso.so): Link with libdl, libsupport.
	(tst-libc_dlvsym.out, tst-libc_dlvsym-static.out): The shared
	object tst-libc_dlvsym-dso.so needs to be built before running
	these tests.
	(tst-libc_dlvsym-static-ENV): Set LD_LIBRARY_PATH.
	* elf/Versions: Export __libc_dlvsym.
	* elf/dl-libc.c (struct do_dlvsym_args): New.
	(do_dlvsym, __libc_dlvsym): New functions.
	(struct dl_open_hook, _dl_open_hook): Add dlvsym member.
	* elf/tst-libc_dlvsym-dso.c: New file.
	* elf/tst-libc_dlvsym-static.c: Likewise.
	* elf/tst-libc_dlvsym.c: Likewise.
	* elf/tst-libc_dlvsym.h: Likewise.
  

Comments

Carlos O'Donell Dec. 18, 2017, 5:13 p.m. UTC | #1
On 12/18/2017 06:57 AM, Florian Weimer wrote:
> This commit adds a new _dl_open_hook entry for dlvsym and implements the
> function using the existing dl_lookup_symbol_x function supplied by the
> dynamic loader.

This is a very useful feature!

High level:

It is very useful to have dlvsym work from libc.so, since internally libc.so
may wish to lookup specific versions of a symbol and call them depending on
the desired result that should be achieved. I can see this being useful for
developer testing without much impact on the maintenance of the library
since we already implement dlvsym for user applications.

Design:

The design looks good, a wrapper, with the right calls. I don't see anything
particularly wrong here. It looks straight forward, but I bet it took quite
a bit of trial and error to get it all wired up correctly. Thank you for that.

Implementation:

Two comments needs expanding in the test case.

We probably need to test this design a little more widely.

Have you run this with build-many-glibcs.sh to see if this compiles across
all the variants? That would be something we would want to cover before
accepting this. Some of the code, like that in DL_SYMBOL_ADDRESS has enough
variability in other arches that it sometimes breaks in weird ways, just
speaking from experience.

Also, will your test break for new arches? See my comments below about adding
one more fallback version check.

 
> 2017-12-18  Florian Weimer  <fweimer@redhat.com>
> 
> 	Add support for calling dlvsym from libc.so.
> 	* include/dlfcn.h (__libc_dlvsym): Declare.
> 	* elf/Makefile (tests-static-internal): Add
> 	tst-libc_dlvsym-static.
> 	(tests-internal): Add tst-libc_dlvsym.
> 	(modules-names): Add tst-libc_dlvsym-dso.
> 	(tst-libc_dlvsym, tst-libc_dlvsym-static): Link with libdl.
> 	(tst-libc_dlvsym-dso.so): Link with libdl, libsupport.
> 	(tst-libc_dlvsym.out, tst-libc_dlvsym-static.out): The shared
> 	object tst-libc_dlvsym-dso.so needs to be built before running
> 	these tests.
> 	(tst-libc_dlvsym-static-ENV): Set LD_LIBRARY_PATH.
> 	* elf/Versions: Export __libc_dlvsym.
> 	* elf/dl-libc.c (struct do_dlvsym_args): New.
> 	(do_dlvsym, __libc_dlvsym): New functions.
> 	(struct dl_open_hook, _dl_open_hook): Add dlvsym member.
> 	* elf/tst-libc_dlvsym-dso.c: New file.
> 	* elf/tst-libc_dlvsym-static.c: Likewise.
> 	* elf/tst-libc_dlvsym.c: Likewise.
> 	* elf/tst-libc_dlvsym.h: Likewise.
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 47c3d23ed8..b129739a79 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -151,7 +151,7 @@ tests-static-normal := tst-leaks1-static tst-array1-static tst-array5-static \
>  	       tst-linkall-static tst-env-setuid tst-env-setuid-tunables
>  tests-static-internal := tst-tls1-static tst-tls2-static \
>  	       tst-ptrguard1-static tst-stackguard1-static \
> -	       tst-tls1-static-non-pie
> +	       tst-tls1-static-non-pie tst-libc_dlvsym-static

OK.

>  
>  CRT-tst-tls1-static-non-pie := $(csu-objpfx)crt1.o
>  tst-tls1-static-non-pie-no-pie = yes
> @@ -192,7 +192,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
>  tests-internal += loadtest unload unload2 circleload1 \
>  	 neededtest neededtest2 neededtest3 neededtest4 \
>  	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
> -	 tst-ptrguard1 tst-stackguard1
> +	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym

OK.

>  ifeq ($(build-hardcoded-path-in-tests),yes)
>  tests += tst-dlopen-aout
>  LDFLAGS-tst-dlopen-aout = $(no-pie-ldflag)
> @@ -272,7 +272,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
>  		tst-audit12mod1 tst-audit12mod2 tst-audit12mod3 tst-auditmod12 \
>  		tst-latepthreadmod $(tst-tls-many-dynamic-modules) \
>  		tst-nodelete-dlclose-dso tst-nodelete-dlclose-plugin \
> -		tst-main1mod
> +		tst-main1mod tst-libc_dlvsym-dso

OK.

>  ifeq (yes,$(have-mtls-dialect-gnu2))
>  tests += tst-gnu2-tls1
>  modules-names += tst-gnu2-tls1mod
> @@ -1436,3 +1436,13 @@ CRT-tst-main1 := $(csu-objpfx)crt1.o
>  LDFLAGS-tst-main1 = $(no-pie-ldflag)
>  LDLIBS-tst-main1 = $(libsupport)
>  tst-main1mod.so-no-z-defs = yes
> +
> +# Both the main program and the DSO for tst-libc_dlvsym need to link
> +# against libdl.
> +$(objpfx)tst-libc_dlvsym: $(libdl)
> +$(objpfx)tst-libc_dlvsym-dso.so: $(libsupport) $(libdl)
> +$(objpfx)tst-libc_dlvsym.out: $(objpfx)tst-libc_dlvsym-dso.so
> +$(objpfx)tst-libc_dlvsym-static: $(common-objpfx)dlfcn/libdl.a
> +tst-libc_dlvsym-static-ENV = \
> +  LD_LIBRARY_PATH=$(objpfx):$(common-objpfx):$(common-objpfx)dlfcn

OK. Thanks for testing static :-)

> +$(objpfx)tst-libc_dlvsym-static.out: $(objpfx)tst-libc_dlvsym-dso.so

OK.

> diff --git a/elf/Versions b/elf/Versions
> index 79ffaf73d2..90dc6255cd 100644
> --- a/elf/Versions
> +++ b/elf/Versions
> @@ -25,7 +25,7 @@ libc {
>      _dl_addr;
>      _dl_open_hook;
>      _dl_sym; _dl_vsym;
> -    __libc_dlclose; __libc_dlopen_mode; __libc_dlsym;
> +    __libc_dlclose; __libc_dlopen_mode; __libc_dlsym; __libc_dlvsym;

OK.

>  
>      # Internal error handling support.  Interposes the functions in ld.so.
>      _dl_signal_exception; _dl_catch_exception;
> diff --git a/elf/dl-libc.c b/elf/dl-libc.c
> index 7d9a8948f3..01f1860946 100644
> --- a/elf/dl-libc.c
> +++ b/elf/dl-libc.c
> @@ -20,6 +20,7 @@
>  #include <dlfcn.h>
>  #include <stdlib.h>
>  #include <ldsodefs.h>
> +#include <dl-hash.h>
>  
>  extern int __libc_argc attribute_hidden;
>  extern char **__libc_argv attribute_hidden;
> @@ -78,6 +79,15 @@ struct do_dlsym_args
>    const ElfW(Sym) *ref;
>  };
>  
> +struct do_dlvsym_args
> +{
> +  /* dlvsym is like dlsym.  */
> +  struct do_dlsym_args dlsym;
> +
> +  /* But dlvsym needs a version  as well.  */
> +  struct r_found_version version;
> +};

OK.

> +
>  static void
>  do_dlopen (void *ptr)
>  {
> @@ -99,6 +109,18 @@ do_dlsym (void *ptr)
>  }
>  
>  static void
> +do_dlvsym (void *ptr)
> +{
> +  struct do_dlvsym_args *args = ptr;
> +  args->dlsym.ref = NULL;
> +  args->dlsym.loadbase
> +    = GLRO(dl_lookup_symbol_x) (args->dlsym.name, args->dlsym.map,
> +				&args->dlsym.ref,
> +				args->dlsym.map->l_local_scope,
> +				&args->version, 0, 0, NULL);
> +}

OK.

> +
> +static void
>  do_dlclose (void *ptr)
>  {
>    GLRO(dl_close) ((struct link_map *) ptr);
> @@ -112,6 +134,7 @@ struct dl_open_hook
>    void *(*dlopen_mode) (const char *name, int mode);
>    void *(*dlsym) (void *map, const char *name);
>    int (*dlclose) (void *map);
> +  void *(*dlvsym) (void *map, const char *name, const char *version);
>  };

OK.

>  
>  #ifdef SHARED
> @@ -142,7 +165,8 @@ static struct dl_open_hook _dl_open_hook =
>    {
>      .dlopen_mode = __libc_dlopen_mode,
>      .dlsym = __libc_dlsym,
> -    .dlclose = __libc_dlclose
> +    .dlclose = __libc_dlclose,
> +    .dlvsym = __libc_dlvsym,

OK.

>    };
>  #endif
>  
> @@ -211,6 +235,30 @@ __libc_dlsym (void *map, const char *name)
>  }
>  libc_hidden_def (__libc_dlsym)
>  
> +void *
> +__libc_dlvsym (void *map, const char *name, const char *version)
> +{
> +#ifdef SHARED
> +  if (!rtld_active ())
> +    return _dl_open_hook->dlvsym (map, name, version);

OK.

> +#endif
> +
> +  struct do_dlvsym_args args;
> +  args.dlsym.map = map;
> +  args.dlsym.name = name;
> +
> +  /* See _dl_vsym in dl-sym.c.  */
> +  args.version.name = version;
> +  args.version.hidden = 1;
> +  args.version.hash = _dl_elf_hash (version);
> +  args.version.filename = NULL;
> +
> +  return (dlerror_run (do_dlvsym, &args) ? NULL
> +	  : (void *) (DL_SYMBOL_ADDRESS (args.dlsym.loadbase,
> +					 args.dlsym.ref)));

OK.

> +}
> +libc_hidden_def (__libc_dlvsym)
> +
>  int
>  __libc_dlclose (void *map)
>  {
> diff --git a/elf/tst-libc_dlvsym-dso.c b/elf/tst-libc_dlvsym-dso.c
> new file mode 100644
> index 0000000000..d74c1ead61
> --- /dev/null
> +++ b/elf/tst-libc_dlvsym-dso.c
> @@ -0,0 +1,25 @@
> +/* Compare dlvsym and __libc_dlvsym results.  Shared object code.
> +   Copyright (C) 2017 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 "tst-libc_dlvsym.h"
> +
> +void
> +compare_vsyms_global (void)
> +{
> +  compare_vsyms ();
> +}

OK.

> diff --git a/elf/tst-libc_dlvsym-static.c b/elf/tst-libc_dlvsym-static.c
> new file mode 100644
> index 0000000000..955f28d754
> --- /dev/null
> +++ b/elf/tst-libc_dlvsym-static.c
> @@ -0,0 +1,32 @@
> +/* Compare dlvsym and __libc_dlvsym results.  Static version.
> +   Copyright (C) 2017 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 <support/xdlfcn.h>
> +
> +static int
> +do_test (void)
> +{
> +  void *handle = xdlopen ("tst-libc_dlvsym-dso.so", RTLD_LAZY);
> +  void (*compare) (void) = xdlsym (handle, "compare_vsyms_global");
> +  compare ();
> +  xdlclose (handle);
> +
> +  return 0;
> +}

OK.

> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-libc_dlvsym.c b/elf/tst-libc_dlvsym.c
> new file mode 100644
> index 0000000000..864db4ee6a
> --- /dev/null
> +++ b/elf/tst-libc_dlvsym.c
> @@ -0,0 +1,34 @@
> +/* Compare dlvsym and __libc_dlvsym results.  Dynamic version.
> +   Copyright (C) 2017 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 "tst-libc_dlvsym.h"
> +
> +static int
> +do_test (void)
> +{
> +  compare_vsyms ();
> +
> +  void *handle = xdlopen ("tst-libc_dlvsym-dso.so", RTLD_LAZY);
> +  void (*compare) (void) = xdlsym (handle, "compare_vsyms_global");
> +  compare ();
> +  xdlclose (handle);
> +
> +  return 0;
> +}

OK.

> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-libc_dlvsym.h b/elf/tst-libc_dlvsym.h
> new file mode 100644
> index 0000000000..0680140a85
> --- /dev/null
> +++ b/elf/tst-libc_dlvsym.h
> @@ -0,0 +1,104 @@
> +/* Compare dlvsym and __libc_dlvsym results.  Common code.
> +   Copyright (C) 2017 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 <array_length.h>
> +#include <gnu/lib-names.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/xdlfcn.h>
> +

You need a large comment block here explaining at a high level what the
test does and why it expects the results it does.

Why do dlvsym and __libc_dlvsym need to be compared?

Why did you pick certain symbols for comparison?

Will this test fail with new ports and need updating?

Why do you iterate over the given list of irregular versions?
(See note below about expanding that).

Writing up the answers to these questions should give us a verbose
description of what the test is doing and why so future developers
can quickly understand your intent. Explaining intent is very important
for a test.

> +/* Run consistency check for versioned symbol NAME@VERSION.  NB: We
> +   may execute in a shared object, so exit on error for proper error
> +   reporting.  */
> +static void
> +compare_vsyms_0 (void *libc_handle, const char *name, const char *version,
> +                 bool *pfound)
> +{
> +  void *dlvsym_address = dlvsym (libc_handle, name, version);
> +  void *libc_dlvsym_address
> +    = __libc_dlvsym (libc_handle, name, version);

OK. Test calls internal symbol, but that's OK.

> +  if (dlvsym_address != libc_dlvsym_address)
> +    FAIL_EXIT1 ("%s@%s mismatch: %p != %p",
> +                name, version, dlvsym_address, libc_dlvsym_address);
> +  if (dlvsym_address != NULL)
> +    *pfound = true;
> +}
> +
> +
> +/* Run consistency check for versioned symbol NAME at multiple symbol
> +   version.  */
> +static void
> +compare_vsyms_1 (void *libc_handle, const char *name)
> +{
> +  bool found = false;
> +
> +  /* Special irregular version strings.  */

Please expand what you mean by "irregular."

> +  const char *special_versions[] =
> +    {
> +      "GLIBC_2.1.1",
> +      "GLIBC_2.1.2",
> +      "GLIBC_2.1.3",
> +      "GLIBC_2.1.4",
> +      "GLIBC_2.2.1",
> +      "GLIBC_2.2.2",
> +      "GLIBC_2.2.3",
> +      "GLIBC_2.2.4",
> +      "GLIBC_2.2.5",
> +      "GLIBC_2.2.6",
> +      "GLIBC_2.3.2",
> +      "GLIBC_2.3.3",
> +      "GLIBC_2.3.4",
> +    };
> +  for (int i = 0; i < array_length (special_versions); ++i)
> +    compare_vsyms_0 (libc_handle, name, special_versions[i], &found);
> +
> +  /* Iterate to an out-of-range version, to cover some unused symbols
> +     as well.  */
> +  for (int minor_version = 0; minor_version <= __GLIBC_MINOR__ + 2;
> +       ++minor_version)
> +    {
> +      char version[30];
> +      snprintf (version, sizeof (version), "GLIBC_%d.%d",
> +                __GLIBC__, minor_version);
> +      compare_vsyms_0 (libc_handle, name, version, &found);
> +    }
> +
> +  if (!found)
> +    FAIL_EXIT1 ("symbol %s not found at any version", name);
> +}
> +
> +/* Run consistency checks for various symbols which usually have
> +   multiple versions.  */
> +static void
> +compare_vsyms (void)
> +{
> +  /* The minor version loop in compare_vsyms_1 needs updating in case
> +     we ever switch to glibc 3.0.  */
> +  if (__GLIBC__ != 2)
> +    FAIL_EXIT1 ("unexpected glibc major version: %d", __GLIBC__);
> +
> +  /* __libc_dlvsym does not recognize the special RTLD_* handles.  */
> +  void *libc_handle = xdlopen (LIBC_SO, RTLD_LAZY | RTLD_NOLOAD);
> +
> +  compare_vsyms_1 (libc_handle, "_sys_errlist");
> +  compare_vsyms_1 (libc_handle, "_sys_siglist");

Will this fail for a new architecture since they will not have any of these
at old versions, but only at the latest version?

Should your loop above always end in checking the latest version?

> +  compare_vsyms_1 (libc_handle, "quick_exit");
> +
> +  xdlclose (libc_handle);
> +}
> diff --git a/include/dlfcn.h b/include/dlfcn.h
> index 526086f1a0..12ef913e19 100644
> --- a/include/dlfcn.h
> +++ b/include/dlfcn.h
> @@ -35,9 +35,11 @@ extern char **__libc_argv attribute_hidden;
>    __libc_dlopen_mode (name, RTLD_LAZY | __RTLD_DLOPEN)
>  extern void *__libc_dlopen_mode  (const char *__name, int __mode);
>  extern void *__libc_dlsym   (void *__map, const char *__name);
> +extern void *__libc_dlvsym (void *map, const char *name, const char *version);
>  extern int   __libc_dlclose (void *__map);
>  libc_hidden_proto (__libc_dlopen_mode)
>  libc_hidden_proto (__libc_dlsym)
> +libc_hidden_proto (__libc_dlvsym)
>  libc_hidden_proto (__libc_dlclose)
>  
>  /* Locate shared object containing the given address.  */
>
  
Florian Weimer Dec. 18, 2017, 8:43 p.m. UTC | #2
On 12/18/2017 06:13 PM, Carlos O'Donell wrote:

> We probably need to test this design a little more widely.

The new internal interface does not vary across architectures, so I 
don't think this is necessary.

> Have you run this with build-many-glibcs.sh to see if this compiles across
> all the variants? That would be something we would want to cover before
> accepting this. Some of the code, like that in DL_SYMBOL_ADDRESS has enough
> variability in other arches that it sometimes breaks in weird ways, just
> speaking from experience.

We shouldn't make build-many-glibcs.sh runs a requirement.  And this 
change is textually very close to the other uses of DL_SYMBOL_ADDRESS in 
elf/dl-libc.c.

We could turn this macro into an inline function if its correct use 
across architectures is a concern.

> Also, will your test break for new arches? See my comments below about adding
> one more fallback version check.

Why would it?  It will pick up default and non-default symbol versions 
alike, and the test only requires one symbol version for each symbol (in 
which the test does not cover much, of course).  I don't see any 
indications that we plan to turn any of those symbols into compat symbols.

>> +#include <array_length.h>
>> +#include <gnu/lib-names.h>
>> +#include <stdbool.h>
>> +#include <stdio.h>
>> +#include <support/check.h>
>> +#include <support/xdlfcn.h>
>> +
> 
> You need a large comment block here explaining at a high level what the
> test does and why it expects the results it does.
> 
> Why do dlvsym and __libc_dlvsym need to be compared?
> 
> Why did you pick certain symbols for comparison?

What about this explanation?

/* compare_vsyms is the main entry point for these tests.

    Indirectly, It calls __libc_dlvsym (from libc.so; internal
    interface) and dlvsym (from libdl.so; public interface) to compare
    the results for a selected set of symbols in libc.so which
    typically have more than one symbol version.  The two functions are
    implemented by somewhat different code, and this test checks that
    their results are the same.

    The versions are generated to range from GLIBC_2.0 to GLIBC_2.Y,
    with Y being the current __GLIBC_MINOR__ version plus two.
    Comparing the two dlvsym results at versions which do not actually
    exist does not test much, but it will not contribute to false test
    failures, either.  */

And:

   /* Historic versions which do not follow the usual GLIBC_2.Y
      pattern, to increase test coverage.  Not all architectures have
      those, but probing additional versions does not hurt.  */
   char *special_versions[] =

>> +  /* __libc_dlvsym does not recognize the special RTLD_* handles.  */
>> +  void *libc_handle = xdlopen (LIBC_SO, RTLD_LAZY | RTLD_NOLOAD);
>> +
>> +  compare_vsyms_1 (libc_handle, "_sys_errlist");
>> +  compare_vsyms_1 (libc_handle, "_sys_siglist");
> 
> Will this fail for a new architecture since they will not have any of these
> at old versions, but only at the latest version?

I don't see how.  I don't count symbol versions.  As long there is one 
version, the test will be okay.

> Should your loop above always end in checking the latest version?

No, because __GLIBC_MINOR__ is still 26 during glibc 2.27 development.

Thanks,
Florian
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 47c3d23ed8..b129739a79 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -151,7 +151,7 @@  tests-static-normal := tst-leaks1-static tst-array1-static tst-array5-static \
 	       tst-linkall-static tst-env-setuid tst-env-setuid-tunables
 tests-static-internal := tst-tls1-static tst-tls2-static \
 	       tst-ptrguard1-static tst-stackguard1-static \
-	       tst-tls1-static-non-pie
+	       tst-tls1-static-non-pie tst-libc_dlvsym-static
 
 CRT-tst-tls1-static-non-pie := $(csu-objpfx)crt1.o
 tst-tls1-static-non-pie-no-pie = yes
@@ -192,7 +192,7 @@  tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
 	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
-	 tst-ptrguard1 tst-stackguard1
+	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym
 ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-dlopen-aout
 LDFLAGS-tst-dlopen-aout = $(no-pie-ldflag)
@@ -272,7 +272,7 @@  modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-audit12mod1 tst-audit12mod2 tst-audit12mod3 tst-auditmod12 \
 		tst-latepthreadmod $(tst-tls-many-dynamic-modules) \
 		tst-nodelete-dlclose-dso tst-nodelete-dlclose-plugin \
-		tst-main1mod
+		tst-main1mod tst-libc_dlvsym-dso
 ifeq (yes,$(have-mtls-dialect-gnu2))
 tests += tst-gnu2-tls1
 modules-names += tst-gnu2-tls1mod
@@ -1436,3 +1436,13 @@  CRT-tst-main1 := $(csu-objpfx)crt1.o
 LDFLAGS-tst-main1 = $(no-pie-ldflag)
 LDLIBS-tst-main1 = $(libsupport)
 tst-main1mod.so-no-z-defs = yes
+
+# Both the main program and the DSO for tst-libc_dlvsym need to link
+# against libdl.
+$(objpfx)tst-libc_dlvsym: $(libdl)
+$(objpfx)tst-libc_dlvsym-dso.so: $(libsupport) $(libdl)
+$(objpfx)tst-libc_dlvsym.out: $(objpfx)tst-libc_dlvsym-dso.so
+$(objpfx)tst-libc_dlvsym-static: $(common-objpfx)dlfcn/libdl.a
+tst-libc_dlvsym-static-ENV = \
+  LD_LIBRARY_PATH=$(objpfx):$(common-objpfx):$(common-objpfx)dlfcn
+$(objpfx)tst-libc_dlvsym-static.out: $(objpfx)tst-libc_dlvsym-dso.so
diff --git a/elf/Versions b/elf/Versions
index 79ffaf73d2..90dc6255cd 100644
--- a/elf/Versions
+++ b/elf/Versions
@@ -25,7 +25,7 @@  libc {
     _dl_addr;
     _dl_open_hook;
     _dl_sym; _dl_vsym;
-    __libc_dlclose; __libc_dlopen_mode; __libc_dlsym;
+    __libc_dlclose; __libc_dlopen_mode; __libc_dlsym; __libc_dlvsym;
 
     # Internal error handling support.  Interposes the functions in ld.so.
     _dl_signal_exception; _dl_catch_exception;
diff --git a/elf/dl-libc.c b/elf/dl-libc.c
index 7d9a8948f3..01f1860946 100644
--- a/elf/dl-libc.c
+++ b/elf/dl-libc.c
@@ -20,6 +20,7 @@ 
 #include <dlfcn.h>
 #include <stdlib.h>
 #include <ldsodefs.h>
+#include <dl-hash.h>
 
 extern int __libc_argc attribute_hidden;
 extern char **__libc_argv attribute_hidden;
@@ -78,6 +79,15 @@  struct do_dlsym_args
   const ElfW(Sym) *ref;
 };
 
+struct do_dlvsym_args
+{
+  /* dlvsym is like dlsym.  */
+  struct do_dlsym_args dlsym;
+
+  /* But dlvsym needs a version  as well.  */
+  struct r_found_version version;
+};
+
 static void
 do_dlopen (void *ptr)
 {
@@ -99,6 +109,18 @@  do_dlsym (void *ptr)
 }
 
 static void
+do_dlvsym (void *ptr)
+{
+  struct do_dlvsym_args *args = ptr;
+  args->dlsym.ref = NULL;
+  args->dlsym.loadbase
+    = GLRO(dl_lookup_symbol_x) (args->dlsym.name, args->dlsym.map,
+				&args->dlsym.ref,
+				args->dlsym.map->l_local_scope,
+				&args->version, 0, 0, NULL);
+}
+
+static void
 do_dlclose (void *ptr)
 {
   GLRO(dl_close) ((struct link_map *) ptr);
@@ -112,6 +134,7 @@  struct dl_open_hook
   void *(*dlopen_mode) (const char *name, int mode);
   void *(*dlsym) (void *map, const char *name);
   int (*dlclose) (void *map);
+  void *(*dlvsym) (void *map, const char *name, const char *version);
 };
 
 #ifdef SHARED
@@ -142,7 +165,8 @@  static struct dl_open_hook _dl_open_hook =
   {
     .dlopen_mode = __libc_dlopen_mode,
     .dlsym = __libc_dlsym,
-    .dlclose = __libc_dlclose
+    .dlclose = __libc_dlclose,
+    .dlvsym = __libc_dlvsym,
   };
 #endif
 
@@ -211,6 +235,30 @@  __libc_dlsym (void *map, const char *name)
 }
 libc_hidden_def (__libc_dlsym)
 
+void *
+__libc_dlvsym (void *map, const char *name, const char *version)
+{
+#ifdef SHARED
+  if (!rtld_active ())
+    return _dl_open_hook->dlvsym (map, name, version);
+#endif
+
+  struct do_dlvsym_args args;
+  args.dlsym.map = map;
+  args.dlsym.name = name;
+
+  /* See _dl_vsym in dl-sym.c.  */
+  args.version.name = version;
+  args.version.hidden = 1;
+  args.version.hash = _dl_elf_hash (version);
+  args.version.filename = NULL;
+
+  return (dlerror_run (do_dlvsym, &args) ? NULL
+	  : (void *) (DL_SYMBOL_ADDRESS (args.dlsym.loadbase,
+					 args.dlsym.ref)));
+}
+libc_hidden_def (__libc_dlvsym)
+
 int
 __libc_dlclose (void *map)
 {
diff --git a/elf/tst-libc_dlvsym-dso.c b/elf/tst-libc_dlvsym-dso.c
new file mode 100644
index 0000000000..d74c1ead61
--- /dev/null
+++ b/elf/tst-libc_dlvsym-dso.c
@@ -0,0 +1,25 @@ 
+/* Compare dlvsym and __libc_dlvsym results.  Shared object code.
+   Copyright (C) 2017 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 "tst-libc_dlvsym.h"
+
+void
+compare_vsyms_global (void)
+{
+  compare_vsyms ();
+}
diff --git a/elf/tst-libc_dlvsym-static.c b/elf/tst-libc_dlvsym-static.c
new file mode 100644
index 0000000000..955f28d754
--- /dev/null
+++ b/elf/tst-libc_dlvsym-static.c
@@ -0,0 +1,32 @@ 
+/* Compare dlvsym and __libc_dlvsym results.  Static version.
+   Copyright (C) 2017 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 <support/xdlfcn.h>
+
+static int
+do_test (void)
+{
+  void *handle = xdlopen ("tst-libc_dlvsym-dso.so", RTLD_LAZY);
+  void (*compare) (void) = xdlsym (handle, "compare_vsyms_global");
+  compare ();
+  xdlclose (handle);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-libc_dlvsym.c b/elf/tst-libc_dlvsym.c
new file mode 100644
index 0000000000..864db4ee6a
--- /dev/null
+++ b/elf/tst-libc_dlvsym.c
@@ -0,0 +1,34 @@ 
+/* Compare dlvsym and __libc_dlvsym results.  Dynamic version.
+   Copyright (C) 2017 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 "tst-libc_dlvsym.h"
+
+static int
+do_test (void)
+{
+  compare_vsyms ();
+
+  void *handle = xdlopen ("tst-libc_dlvsym-dso.so", RTLD_LAZY);
+  void (*compare) (void) = xdlsym (handle, "compare_vsyms_global");
+  compare ();
+  xdlclose (handle);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-libc_dlvsym.h b/elf/tst-libc_dlvsym.h
new file mode 100644
index 0000000000..0680140a85
--- /dev/null
+++ b/elf/tst-libc_dlvsym.h
@@ -0,0 +1,104 @@ 
+/* Compare dlvsym and __libc_dlvsym results.  Common code.
+   Copyright (C) 2017 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 <array_length.h>
+#include <gnu/lib-names.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/xdlfcn.h>
+
+/* Run consistency check for versioned symbol NAME@VERSION.  NB: We
+   may execute in a shared object, so exit on error for proper error
+   reporting.  */
+static void
+compare_vsyms_0 (void *libc_handle, const char *name, const char *version,
+                 bool *pfound)
+{
+  void *dlvsym_address = dlvsym (libc_handle, name, version);
+  void *libc_dlvsym_address
+    = __libc_dlvsym (libc_handle, name, version);
+  if (dlvsym_address != libc_dlvsym_address)
+    FAIL_EXIT1 ("%s@%s mismatch: %p != %p",
+                name, version, dlvsym_address, libc_dlvsym_address);
+  if (dlvsym_address != NULL)
+    *pfound = true;
+}
+
+
+/* Run consistency check for versioned symbol NAME at multiple symbol
+   version.  */
+static void
+compare_vsyms_1 (void *libc_handle, const char *name)
+{
+  bool found = false;
+
+  /* Special irregular version strings.  */
+  const char *special_versions[] =
+    {
+      "GLIBC_2.1.1",
+      "GLIBC_2.1.2",
+      "GLIBC_2.1.3",
+      "GLIBC_2.1.4",
+      "GLIBC_2.2.1",
+      "GLIBC_2.2.2",
+      "GLIBC_2.2.3",
+      "GLIBC_2.2.4",
+      "GLIBC_2.2.5",
+      "GLIBC_2.2.6",
+      "GLIBC_2.3.2",
+      "GLIBC_2.3.3",
+      "GLIBC_2.3.4",
+    };
+  for (int i = 0; i < array_length (special_versions); ++i)
+    compare_vsyms_0 (libc_handle, name, special_versions[i], &found);
+
+  /* Iterate to an out-of-range version, to cover some unused symbols
+     as well.  */
+  for (int minor_version = 0; minor_version <= __GLIBC_MINOR__ + 2;
+       ++minor_version)
+    {
+      char version[30];
+      snprintf (version, sizeof (version), "GLIBC_%d.%d",
+                __GLIBC__, minor_version);
+      compare_vsyms_0 (libc_handle, name, version, &found);
+    }
+
+  if (!found)
+    FAIL_EXIT1 ("symbol %s not found at any version", name);
+}
+
+/* Run consistency checks for various symbols which usually have
+   multiple versions.  */
+static void
+compare_vsyms (void)
+{
+  /* The minor version loop in compare_vsyms_1 needs updating in case
+     we ever switch to glibc 3.0.  */
+  if (__GLIBC__ != 2)
+    FAIL_EXIT1 ("unexpected glibc major version: %d", __GLIBC__);
+
+  /* __libc_dlvsym does not recognize the special RTLD_* handles.  */
+  void *libc_handle = xdlopen (LIBC_SO, RTLD_LAZY | RTLD_NOLOAD);
+
+  compare_vsyms_1 (libc_handle, "_sys_errlist");
+  compare_vsyms_1 (libc_handle, "_sys_siglist");
+  compare_vsyms_1 (libc_handle, "quick_exit");
+
+  xdlclose (libc_handle);
+}
diff --git a/include/dlfcn.h b/include/dlfcn.h
index 526086f1a0..12ef913e19 100644
--- a/include/dlfcn.h
+++ b/include/dlfcn.h
@@ -35,9 +35,11 @@  extern char **__libc_argv attribute_hidden;
   __libc_dlopen_mode (name, RTLD_LAZY | __RTLD_DLOPEN)
 extern void *__libc_dlopen_mode  (const char *__name, int __mode);
 extern void *__libc_dlsym   (void *__map, const char *__name);
+extern void *__libc_dlvsym (void *map, const char *name, const char *version);
 extern int   __libc_dlclose (void *__map);
 libc_hidden_proto (__libc_dlopen_mode)
 libc_hidden_proto (__libc_dlsym)
+libc_hidden_proto (__libc_dlvsym)
 libc_hidden_proto (__libc_dlclose)
 
 /* Locate shared object containing the given address.  */