This is the mail archive of the libc-alpha@sources.redhat.com 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]

Re: glibc 2.1.94 (ldconfig has a bug)


Here are the fixes etc one by one. They all mean dl-cache.c, for which
my previous tiny patch has been applied. Would they be applied too, it
must be done in the order they are here.


a) We have the next macro:

#define _dl_cache_verify_ptr(ptr) (ptr < cachesize - sizeof *cache)

It doesn't work in case of the new cache format and it doesn't work
even in case of the old format, because the libs[] array is forgotten
here. But it behaves quiet while we give it the good ptr's only (and
it is what we usually do). To cure the situation we can do some
different calculations of a limit for this pointer (which is an offset
really) for the each variant of the cache file contents, but we have a
better way. We always use the macro like this:

       _dl_cache_verify_ptr (cache->libs[middle - 1].key)
       ...
       (_dl_cache_libcmp (name,
                          cache_data
                           + cache->libs[middle - 1].key)

so, we can compute (cache_data + cache->libs[...].key) right away,
look if this goes into the cache file simple by comparing its value
with the end of the cache file's map and use this value in a later
expression. This approach will work for any cache formats, and we will
get some optimization, because the arithmetic becomes simpler. Below
it is a patch, which implements this approach. There we:

 add the "End Of Cache" var - eoc; 

 add the pointer - ptr, which holds the
 (cache_data + cache->libs[...].something)'s
 values;

 change the macro to do the simple (ptr < eoc) check;

 change all the pairs
       _dl_cache_verify_ptr
       ...
       _dl_cache_do_something_with_ptr 
 to work thru the new way.

By the way, after this patch is applied, _dl_load_cache_lookup
compiled by GCC for Pentium becomes roughly 0x100 bytes shorter. So,
the arithmetic indeed becomes simpler.



--- glibc-2.1.95/sysdeps/generic/dl-cache.c	Tue Oct 17 09:17:24 2000
+++ glibc-2.1.95/sysdeps/generic/dl-cache.c	Tue Oct 17 09:50:27 2000
@@ -35,7 +35,7 @@
 static size_t cachesize;
 
 /* 1 if cache_data + PTR points into the cache.  */
-#define _dl_cache_verify_ptr(ptr) (ptr < cachesize - sizeof *cache)
+#define _dl_cache_verify_ptr(ptr) ((ptr) < eoc)
 
 /* This is the cache ID we expect.  Normally it is 3 for glibc linked
    binaries.  */
@@ -57,15 +57,15 @@
       {									  \
 	/* Make sure string table indices are not bogus before using	  \
 	   them.  */							  \
-	if (! _dl_cache_verify_ptr (cache->libs[middle].key))		  \
+	if (! _dl_cache_verify_ptr (ptr = cache_data			  \
+					+ cache->libs[middle].key))	  \
 	  {								  \
 	    cmpres = 1;							  \
 	    break;							  \
 	  }								  \
 									  \
 	/* Actually compare the entry with the key.  */			  \
-	cmpres = _dl_cache_libcmp (name,				  \
-				   cache_data + cache->libs[middle].key); \
+	cmpres = _dl_cache_libcmp (name, ptr);				  \
 	if (cmpres == 0)						  \
 	  /* Found it.  */						  \
 	  break;							  \
@@ -89,12 +89,10 @@
 	while (middle > 0						  \
 	       /* Make sure string table indices are not bogus before	  \
 		  using them.  */					  \
-	       && _dl_cache_verify_ptr (cache->libs[middle - 1].key)	  \
+	       && _dl_cache_verify_ptr (ptr = cache_data		  \
+					    + cache->libs[middle - 1].key)\
 	       /* Actually compare the entry.  */			  \
-	       && (_dl_cache_libcmp (name,				  \
-				     cache_data				  \
-				     + cache->libs[middle - 1].key)	  \
-		   == 0))						  \
+	       && (_dl_cache_libcmp (name, ptr) == 0))			  \
 	  --middle;							  \
 									  \
 	do								  \
@@ -106,21 +104,20 @@
 		/* We haven't seen this string so far.  Test whether the  \
 		   index is ok and whether the name matches.  Otherwise	  \
 		   we are done.  */					  \
-		&& (! _dl_cache_verify_ptr (cache->libs[middle].key)	  \
-		    || (_dl_cache_libcmp (name,				  \
-					  cache_data			  \
-					  + cache->libs[middle].key)	  \
-			!= 0)))						  \
+		&& (! _dl_cache_verify_ptr (ptr = cache_data		  \
+						+ cache->libs[middle].key)\
+		    || (_dl_cache_libcmp (name, ptr) != 0)))		  \
 	      break;							  \
 									  \
 	    flags = cache->libs[middle].flags;				  \
 	    if (_dl_cache_check_flags (flags)				  \
-		&& _dl_cache_verify_ptr (cache->libs[middle].value))	  \
+		&& _dl_cache_verify_ptr (ptr = cache_data		  \
+					     + cache->libs[middle].value))\
 	      {								  \
 		if (best == NULL || flags == _dl_correct_cache_id)	  \
 		  {							  \
 		    HWCAP_CHECK;					  \
-		    best = cache_data + cache->libs[middle].value;	  \
+		    best = ptr;						  \
 									  \
 		    if (flags == _dl_correct_cache_id)			  \
 		      /* We've found an exact match for the shared	  \
@@ -145,8 +142,8 @@
 {
   int left, right, middle;
   int cmpres;
-  const char *cache_data;
-  const char *best;
+  const char *cache_data, *eoc;
+  const char *best, *ptr;
 
   /* Print a message if the loading of libs is traced.  */
   if (_dl_debug_libs)
@@ -205,6 +202,7 @@
     /* Previously looked for the cache file and didn't find it.  */
     return NULL;
 
+  eoc = (void*)cache + cachesize;
   best = NULL;
 
   if (cache_new != (void *) -1)



b) When we check the new cache signature, we look at the magic and the
version separately. We can consolidate this.



--- glibc-2.1.95/sysdeps/generic/dl-cache.c	Tue Oct 17 09:50:27 2000
+++ glibc-2.1.95/sysdeps/generic/dl-cache.c	Tue Oct 17 10:28:14 2000
@@ -134,6 +134,9 @@
 
 
 
+#define CACHE_NEW_SIGN (CACHEMAGIC_NEW CACHE_VERSION)
+
+
 /* Look up NAME in ld.so.cache and return the file name stored there,
    or null if none is found.  */
 
@@ -173,20 +176,14 @@
 
 	  cache_new = (struct cache_file_new *) ((void *)cache + offset);
 	  if (cachesize < (offset + sizeof (struct cache_file_new))
-	      || memcmp (cache_new->magic, CACHEMAGIC_NEW,
-			  sizeof CACHEMAGIC_NEW - 1)
-	      || memcmp (cache_new->version, CACHE_VERSION,
-			 sizeof CACHE_VERSION - 1))
+	      || memcmp (cache_new, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
 	    cache_new = (void *) -1;
 	}
       else if (file && cachesize > sizeof *cache_new)
 	{
 	  cache_new = (struct cache_file_new *)
 	  cache     = file;
-	  if (memcmp (cache_new->magic, CACHEMAGIC_NEW,
-		      sizeof CACHEMAGIC_NEW - 1)
-	      || memcmp (cache_new->version, CACHE_VERSION,
-			 sizeof CACHE_VERSION - 1))
+	  if (memcmp (cache_new, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
 	    cache_new = (void *) -1;
 	}
       else



c) A little error, again. We have:

      else if (...)
        {
          cache_new = (struct cache_file_new *)
          cache     = file;
          if (...)
            cache_new = (void *) -1;
        }

where we check for the new cache version. If the second condition
isn't satisfied, then cache is left with a trustworthy value, while it
must be rewritten with -1, as cache_new is. The next patch fixes that.
And the whole "else if" branch is modified to be more, say, natural.



--- glibc-2.1.95/sysdeps/generic/dl-cache.c	Tue Oct 17 10:28:14 2000
+++ glibc-2.1.95/sysdeps/generic/dl-cache.c	Tue Oct 17 13:38:13 2000
@@ -179,13 +179,10 @@
 	      || memcmp (cache_new, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
 	    cache_new = (void *) -1;
 	}
-      else if (file && cachesize > sizeof *cache_new)
-	{
-	  cache_new = (struct cache_file_new *)
-	  cache     = file;
-	  if (memcmp (cache_new, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
-	    cache_new = (void *) -1;
-	}
+      else if (file && cachesize > sizeof *cache_new &&
+	       !memcmp (file, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
+	cache_new = (struct cache_file_new *)
+	cache     = file;
       else
 	{
 	  if (file)



d) The next patch can be considered as a little combing of the code.
It cures nothing. But it eliminates some superfluous checks for (file
!= NULL) and reduces the usage of a file-level variable cache, so it
can be considered as a little optimization too.



--- glibc-2.1.95/sysdeps/generic/dl-cache.c	Tue Oct 17 13:38:13 2000
+++ glibc-2.1.95/sysdeps/generic/dl-cache.c	Tue Oct 17 14:38:21 2000
@@ -163,33 +163,32 @@
 	 - the old format with the new format in it
 	 - only the new format
 	 The following checks if the cache contains any of these formats.  */
-      if (file && cachesize > sizeof *cache &&
-	  !memcmp (file, CACHEMAGIC, sizeof CACHEMAGIC - 1))
-	{
-	  size_t offset;
-	  /* Looks ok.  */
-	  cache = file;
-
-	  /* Check for new version.  */
-	  offset = ALIGN_CACHE (sizeof (struct cache_file)
-				+ cache->nlibs * sizeof (struct file_entry));
-
-	  cache_new = (struct cache_file_new *) ((void *)cache + offset);
-	  if (cachesize < (offset + sizeof (struct cache_file_new))
-	      || memcmp (cache_new, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
+      if (file)
+	if (cachesize > sizeof *cache &&
+	    !memcmp (file, CACHEMAGIC, sizeof CACHEMAGIC - 1))
+	  {
+	    /* Looks ok.  */
+	    /* Check for new version.  */
+	    size_t offset;
 	    cache_new = (void *) -1;
-	}
-      else if (file && cachesize > sizeof *cache_new &&
-	       !memcmp (file, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
-	cache_new = (struct cache_file_new *)
-	cache     = file;
-      else
-	{
-	  if (file)
+	    offset = ALIGN_CACHE (sizeof *cache + ((struct cache_file*)file)->
+				  nlibs * sizeof (struct file_entry));
+	    if (cachesize > offset + sizeof *cache_new &&
+		!memcmp (file + offset, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
+	      cache_new = file + offset;
+	  }
+	else if (cachesize > sizeof *cache_new &&
+		 !memcmp (file, CACHE_NEW_SIGN, sizeof CACHE_NEW_SIGN - 1))
+	  cache_new = file;
+	else
+	  {
 	    __munmap (file, cachesize);
-	  cache = (void *) -1;
-	  return NULL;
-	}
+	    goto fail;
+	  }
+      else fail:
+	file = (void *) -1;
+
+      cache = file;
     }
 
   if (cache == (void *) -1)



That's all. Again, these patches assume that my first patch is already
applied to dl-cache.c.

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