]> sourceware.org Git - glibc.git/commitdiff
Promote do_lookup_x:check_match to a full function.
authorCarlos O'Donell <carlos@redhat.com>
Fri, 28 Feb 2014 23:11:06 +0000 (18:11 -0500)
committerCarlos O'Donell <carlos@redhat.com>
Fri, 28 Feb 2014 23:15:10 +0000 (18:15 -0500)
While it may be argued that nested functions make the resulting
code easier to read, or worse to read the following two bugs
make it difficult to debug:

Bug 8300 - no local symbol information within nested or nesting
procedures
https://sourceware.org/bugzilla/show_bug.cgi?id=8300

Bug 53927 - wrong value for DW_AT_static_link
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53927

Until these are fixed I've made check_match a full function.
After they are fixed we can resume arguing about the merits
of nested functions on readability and maintenance.

ChangeLog
elf/dl-lookup.c

index a5b1de26d0b4932cdff1310d5f7602fd17223530..60d63dbf8817e856740438d8f6b5e0330c72b056 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2014-02-28  Carlos O'Donell  <carlos@redhat.com>
+
+       * elf/dl-lookup.c (check_match): New function.
+       (ELF_MACHINE_SYM_NO_MATCH): Adjust comment.
+       (do_lookup_x): Remove nested function check_match. Use non-nested
+       function check_match.
+
 2014-02-28  Roland McGrath  <roland@hack.frob.com>
 
        * csu/Makefile (generated, before-compile): Use += rather than =.
index 0b43db8d9b731aa8701ab7c351d2a60886a05006..d1b8efc5d79eaf2271435b2bca8ecfdca836fe46 100644 (file)
@@ -31,9 +31,8 @@
 
 #include <assert.h>
 
-/* Return nonzero if do_lookup_x:check_match should consider SYM to
-   fail to match a symbol reference for some machine-specific
-   reason.  */
+/* Return nonzero if check_match should consider SYM to fail to match a
+   symbol reference for some machine-specific reason.  */
 #ifndef ELF_MACHINE_SYM_NO_MATCH
 # define ELF_MACHINE_SYM_NO_MATCH(sym) 0
 #endif
@@ -75,6 +74,118 @@ struct sym_val
 # define bump_num_relocations() ((void) 0)
 #endif
 
+/* Utility function for do_lookup_x. The caller is called with undef_name,
+   ref, version, flags and type_class, and those are passed as the first
+   five arguments. The caller then computes sym, symidx, strtab, and map
+   and passes them as the next four arguments. Lastly the caller passes in
+   versioned_sym and num_versions which are modified by check_match during
+   the checking process.  */
+static const ElfW(Sym) *
+check_match (const char *const undef_name,
+            const ElfW(Sym) *const ref,
+            const struct r_found_version *const version,
+            const int flags,
+            const int type_class,
+            const ElfW(Sym) *const sym,
+            const Elf_Symndx symidx,
+            const char *const strtab,
+            const struct link_map *const map,
+            const ElfW(Sym) **const versioned_sym,
+            int *const num_versions)
+{
+  unsigned int stt = ELFW(ST_TYPE) (sym->st_info);
+  assert (ELF_RTYPE_CLASS_PLT == 1);
+  if (__builtin_expect ((sym->st_value == 0 /* No value.  */
+                        && stt != STT_TLS)
+                       || ELF_MACHINE_SYM_NO_MATCH (sym)
+                       || (type_class & (sym->st_shndx == SHN_UNDEF)),
+                       0))
+    return NULL;
+
+  /* Ignore all but STT_NOTYPE, STT_OBJECT, STT_FUNC,
+     STT_COMMON, STT_TLS, and STT_GNU_IFUNC since these are no
+     code/data definitions.  */
+#define ALLOWED_STT \
+  ((1 << STT_NOTYPE) | (1 << STT_OBJECT) | (1 << STT_FUNC) \
+   | (1 << STT_COMMON) | (1 << STT_TLS) | (1 << STT_GNU_IFUNC))
+  if (__glibc_unlikely (((1 << stt) & ALLOWED_STT) == 0))
+    return NULL;
+
+  if (sym != ref && strcmp (strtab + sym->st_name, undef_name))
+    /* Not the symbol we are looking for.  */
+    return NULL;
+
+  const ElfW(Half) *verstab = map->l_versyms;
+  if (version != NULL)
+    {
+      if (__glibc_unlikely (verstab == NULL))
+       {
+         /* We need a versioned symbol but haven't found any.  If
+            this is the object which is referenced in the verneed
+            entry it is a bug in the library since a symbol must
+            not simply disappear.
+
+            It would also be a bug in the object since it means that
+            the list of required versions is incomplete and so the
+            tests in dl-version.c haven't found a problem.*/
+         assert (version->filename == NULL
+                 || ! _dl_name_match_p (version->filename, map));
+
+         /* Otherwise we accept the symbol.  */
+       }
+      else
+       {
+         /* We can match the version information or use the
+            default one if it is not hidden.  */
+         ElfW(Half) ndx = verstab[symidx] & 0x7fff;
+         if ((map->l_versions[ndx].hash != version->hash
+              || strcmp (map->l_versions[ndx].name, version->name))
+             && (version->hidden || map->l_versions[ndx].hash
+                 || (verstab[symidx] & 0x8000)))
+           /* It's not the version we want.  */
+           return NULL;
+       }
+    }
+  else
+    {
+      /* No specific version is selected.  There are two ways we
+        can got here:
+
+        - a binary which does not include versioning information
+        is loaded
+
+        - dlsym() instead of dlvsym() is used to get a symbol which
+        might exist in more than one form
+
+        If the library does not provide symbol version information
+        there is no problem at all: we simply use the symbol if it
+        is defined.
+
+        These two lookups need to be handled differently if the
+        library defines versions.  In the case of the old
+        unversioned application the oldest (default) version
+        should be used.  In case of a dlsym() call the latest and
+        public interface should be returned.  */
+      if (verstab != NULL)
+       {
+         if ((verstab[symidx] & 0x7fff)
+             >= ((flags & DL_LOOKUP_RETURN_NEWEST) ? 2 : 3))
+           {
+             /* Don't accept hidden symbols.  */
+             if ((verstab[symidx] & 0x8000) == 0
+                 && (*num_versions)++ == 0)
+               /* No version so far.  */
+               *versioned_sym = sym;
+
+             return NULL;
+           }
+       }
+    }
+
+  /* There cannot be another entry for this symbol so stop here.  */
+  return sym;
+}
+
 
 /* Inner part of the lookup functions.  We return a value > 0 if we
    found the symbol, the value 0 if nothing is found and < 0 if
@@ -130,105 +241,6 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash,
       const ElfW(Sym) *symtab = (const void *) D_PTR (map, l_info[DT_SYMTAB]);
       const char *strtab = (const void *) D_PTR (map, l_info[DT_STRTAB]);
 
-
-      /* Nested routine to check whether the symbol matches.  */
-      const ElfW(Sym) *
-      __attribute_noinline__
-      check_match (const ElfW(Sym) *sym)
-      {
-       unsigned int stt = ELFW(ST_TYPE) (sym->st_info);
-       assert (ELF_RTYPE_CLASS_PLT == 1);
-       if (__builtin_expect ((sym->st_value == 0 /* No value.  */
-                              && stt != STT_TLS)
-                             || ELF_MACHINE_SYM_NO_MATCH (sym)
-                             || (type_class & (sym->st_shndx == SHN_UNDEF)),
-                             0))
-         return NULL;
-
-       /* Ignore all but STT_NOTYPE, STT_OBJECT, STT_FUNC,
-          STT_COMMON, STT_TLS, and STT_GNU_IFUNC since these are no
-          code/data definitions.  */
-#define ALLOWED_STT \
-       ((1 << STT_NOTYPE) | (1 << STT_OBJECT) | (1 << STT_FUNC) \
-        | (1 << STT_COMMON) | (1 << STT_TLS) | (1 << STT_GNU_IFUNC))
-       if (__glibc_unlikely (((1 << stt) & ALLOWED_STT) == 0))
-         return NULL;
-
-       if (sym != ref && strcmp (strtab + sym->st_name, undef_name))
-         /* Not the symbol we are looking for.  */
-         return NULL;
-
-       const ElfW(Half) *verstab = map->l_versyms;
-       if (version != NULL)
-         {
-           if (__glibc_unlikely (verstab == NULL))
-             {
-               /* We need a versioned symbol but haven't found any.  If
-                  this is the object which is referenced in the verneed
-                  entry it is a bug in the library since a symbol must
-                  not simply disappear.
-
-                  It would also be a bug in the object since it means that
-                  the list of required versions is incomplete and so the
-                  tests in dl-version.c haven't found a problem.*/
-               assert (version->filename == NULL
-                       || ! _dl_name_match_p (version->filename, map));
-
-               /* Otherwise we accept the symbol.  */
-             }
-           else
-             {
-               /* We can match the version information or use the
-                  default one if it is not hidden.  */
-               ElfW(Half) ndx = verstab[symidx] & 0x7fff;
-               if ((map->l_versions[ndx].hash != version->hash
-                    || strcmp (map->l_versions[ndx].name, version->name))
-                   && (version->hidden || map->l_versions[ndx].hash
-                       || (verstab[symidx] & 0x8000)))
-                 /* It's not the version we want.  */
-                 return NULL;
-             }
-         }
-       else
-         {
-           /* No specific version is selected.  There are two ways we
-              can got here:
-
-              - a binary which does not include versioning information
-              is loaded
-
-              - dlsym() instead of dlvsym() is used to get a symbol which
-              might exist in more than one form
-
-              If the library does not provide symbol version information
-              there is no problem at all: we simply use the symbol if it
-              is defined.
-
-              These two lookups need to be handled differently if the
-              library defines versions.  In the case of the old
-              unversioned application the oldest (default) version
-              should be used.  In case of a dlsym() call the latest and
-              public interface should be returned.  */
-           if (verstab != NULL)
-             {
-               if ((verstab[symidx] & 0x7fff)
-                   >= ((flags & DL_LOOKUP_RETURN_NEWEST) ? 2 : 3))
-                 {
-                   /* Don't accept hidden symbols.  */
-                   if ((verstab[symidx] & 0x8000) == 0
-                       && num_versions++ == 0)
-                     /* No version so far.  */
-                     versioned_sym = sym;
-
-                   return NULL;
-                 }
-             }
-         }
-
-       /* There cannot be another entry for this symbol so stop here.  */
-       return sym;
-      }
-
       const ElfW(Sym) *sym;
       const ElfW(Addr) *bitmask = map->l_gnu_bitmask;
       if (__glibc_likely (bitmask != NULL))
@@ -254,7 +266,10 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash,
                    if (((*hasharr ^ new_hash) >> 1) == 0)
                      {
                        symidx = hasharr - map->l_gnu_chain_zero;
-                       sym = check_match (&symtab[symidx]);
+                       sym = check_match (undef_name, ref, version, flags,
+                                          type_class, &symtab[symidx], symidx,
+                                          strtab, map, &versioned_sym,
+                                          &num_versions);
                        if (sym != NULL)
                          goto found_it;
                      }
@@ -276,7 +291,10 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash,
               symidx != STN_UNDEF;
               symidx = map->l_chain[symidx])
            {
-             sym = check_match (&symtab[symidx]);
+             sym = check_match (undef_name, ref, version, flags,
+                                type_class, &symtab[symidx], symidx,
+                                strtab, map, &versioned_sym,
+                                &num_versions);
              if (sym != NULL)
                goto found_it;
            }
This page took 0.128312 seconds and 5 git commands to generate.