[BZ,#23668] Default to the new format for ld.so.cache
Commit Message
glibc has supported this format for 18+ years.
Reorder conditionals to look for the new format first.
[BZ #23668]
* elf/ldconfig.c: Default to the new format for ld.so.cache. glibc has
supported this format for 18+ years.
* elf/dl-cache.c (_dl_load_cache_lookup): Reorder conditionals to look
for the new format first.
---
NEWS | 3 +++
elf/dl-cache.c | 16 ++++++++--------
elf/ldconfig.c | 4 ++--
3 files changed, 13 insertions(+), 10 deletions(-)
Comments
Josh Triplett <josh@joshtriplett.org> writes:
> glibc has supported this format for 18+ years.
>
> Reorder conditionals to look for the new format first.
>
> [BZ #23668]
> * elf/ldconfig.c: Default to the new format for ld.so.cache. glibc has
> supported this format for 18+ years.
> * elf/dl-cache.c (_dl_load_cache_lookup): Reorder conditionals to look
> for the new format first.
Josh, do you know if any distribution has been using this format by
default?
Thanks,
Florian
On Wed, Sep 19, 2018 at 09:35:57PM +0200, Florian Weimer wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
>
> > glibc has supported this format for 18+ years.
> >
> > Reorder conditionals to look for the new format first.
> >
> > [BZ #23668]
> > * elf/ldconfig.c: Default to the new format for ld.so.cache. glibc has
> > supported this format for 18+ years.
> > * elf/dl-cache.c (_dl_load_cache_lookup): Reorder conditionals to look
> > for the new format first.
>
> Josh, do you know if any distribution has been using this format by
> default?
Clear is using it by default, and I'm currently working on getting
Debian doing so.
* 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?
There's a missing consistency check on the new-format path (both with
and without your patch), but we can add that separately.
I think we can remove support for the old cache format completely.
> 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?
Thanks,
Florian
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
* 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.
Fair enough. I'm going push it without this change and rework the
optimization so not to regress bug 18093. I don't know if you have
copyright paperwork with the FSF; these additional changes would likely
exceed the limit of what we can accept without such paperwork.
>> 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?
Make sense. Note that this is not about the ability of skipping over an
old-format header, we have to keep this for fifteen or so more years.
Thanks,
Florian
@@ -37,6 +37,9 @@ Deprecated and removed features, and other changes affecting compatibility:
used by the Linux kernel. This affects the size and layout of those
structures.
+* ldconfig now defaults to the new format for ld.so.cache. glibc has supported
+ this format for 18+ years.
+
Changes to build and runtime requirements:
[Add changes to build and runtime requirements here]
@@ -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)
@@ -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 }
};