This is the mail archive of the glibc-cvs@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]

GNU C Library master sources branch master updated. glibc-2.19-101-g8e25d1e


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  8e25d1e7721d8078d8925e325799740dd53a5801 (commit)
      from  7b3551e3a8f7278e123757987570c72f1216acc2 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=8e25d1e7721d8078d8925e325799740dd53a5801

commit 8e25d1e7721d8078d8925e325799740dd53a5801
Author: Carlos O'Donell <carlos@redhat.com>
Date:   Fri Feb 28 18:11:06 2014 -0500

    Promote do_lookup_x:check_match to a full function.
    
    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.

diff --git a/ChangeLog b/ChangeLog
index a5b1de2..60d63db 100644
--- 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 =.
diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
index 0b43db8..d1b8efc 100644
--- a/elf/dl-lookup.c
+++ b/elf/dl-lookup.c
@@ -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;
 	    }

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog       |    7 ++
 elf/dl-lookup.c |  226 ++++++++++++++++++++++++++++++-------------------------
 2 files changed, 129 insertions(+), 104 deletions(-)


hooks/post-receive
-- 
GNU C Library master sources


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