This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] elf: Support dlvsym within libc.so


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]