[PATCH BZ #23668] Default to the new format for ld.so.cache

[Josh Triplett]

On Fri, May 08, 2020 at 08:58:40PM +0200, Florian Weimer wrote:
> * Josh Triplett:
> > diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> > index 6ee5153ff9..75bd9d9536 100644
> > --- a/elf/dl-cache.c
> > +++ b/elf/dl-cache.c
> > @@ -203,7 +203,14 @@ _dl_load_cache_lookup (const char *name)
> >  	 - 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 != MAP_FAILED && cachesize > sizeof *cache
> > +      if (file != MAP_FAILED && cachesize > sizeof *cache_new
> > +	       && memcmp (file, CACHEMAGIC_VERSION_NEW,
> > +			  sizeof CACHEMAGIC_VERSION_NEW - 1) == 0)
> > +	{
> > +	  cache_new = file;
> > +	  cache = file;
> > +	}
> > +      else if (file != MAP_FAILED && cachesize > sizeof *cache
> >  	  && memcmp (file, CACHEMAGIC, sizeof CACHEMAGIC - 1) == 0)
> >  	{
> >  	  size_t offset;
> > @@ -220,13 +227,6 @@ _dl_load_cache_lookup (const char *name)
> >  			 sizeof CACHEMAGIC_VERSION_NEW - 1) != 0)
> >  	    cache_new = (void *) -1;
> >  	}
> > -      else if (file != MAP_FAILED && cachesize > sizeof *cache_new
> > -	       && memcmp (file, CACHEMAGIC_VERSION_NEW,
> > -			  sizeof CACHEMAGIC_VERSION_NEW - 1) == 0)
> > -	{
> > -	  cache_new = file;
> > -	  cache = file;
> > -	}
> >        else
> >  	{
> >  	  if (file != MAP_FAILED)
> 
> I do not think this is needed.  It's just a minor performance
> optimization, right?

A minor performance optimization at the start of every single
dynamically linked program run; I think it's worth doing.

> I think we can remove support for the old cache format completely.

I didn't want to make that call, given glibc's strong compatibility
guarantees. It seems reasonable to remove, but perhaps we should
deprecate it first, in case someone's system still has an old-format
cache and they don't realize that?

> > diff --git a/elf/ldconfig.c b/elf/ldconfig.c
> > index fbdd814edf..1ce4a29566 100644
> > --- a/elf/ldconfig.c
> > +++ b/elf/ldconfig.c
> > @@ -95,7 +95,7 @@ int opt_verbose;
> >  
> >  /* Format to support.  */
> >  /* 0: only libc5/glibc2; 1: both; 2: only glibc 2.2.  */
> > -int opt_format = 1;
> > +int opt_format = 2;
> >  
> >  /* Build cache.  */
> >  static int opt_build_cache = 1;
> > @@ -148,7 +148,7 @@ static const struct argp_option options[] =
> >    { NULL, 'f', N_("CONF"), 0, N_("Use CONF as configuration file"), 0},
> >    { NULL, 'n', NULL, 0, N_("Only process directories specified on the command line.  Don't build cache."), 0},
> >    { NULL, 'l', NULL, 0, N_("Manually link individual libraries."), 0},
> > -  { "format", 'c', N_("FORMAT"), 0, N_("Format to use: new, old or compat (default)"), 0},
> > +  { "format", 'c', N_("FORMAT"), 0, N_("Format to use: new (default), old, or compat"), 0},
> >    { "ignore-aux-cache", 'i', NULL, 0, N_("Ignore auxiliary cache file"), 0},
> >    { NULL, 0, NULL, 0, NULL, 0 }
> >  };
> 
> But this patch a good start.  Do you need help installing it?

I'd appreciate it if someone with commit access would commit it, yes. I
don't have such access.

- Josh Triplett