* [PATCH] Fix output of LD_SHOW_AUXV=1.
@ 2019-03-08 12:52 Stefan Liebler
2019-03-08 14:51 ` Carlos O'Donell
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Liebler @ 2019-03-08 12:52 UTC (permalink / raw)
To: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]
Hi,
starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
the output was corrupted on some platforms as _dl_procinfo
was called for every auxv entry and on some architectures like s390
all entries were represented as "AT_HWCAP".
This patch fixes the condition which determines if _dl_procinfo
is called and adjusts _dl_procinfo implementations which assumed
that they are only called for AT_HWCAP or AT_HWCAP2.
A further question is if we should always call _dl_procinfo without a
condition and let it decide if it is able to print an entry or if it
should be printed with help of the auxvars list?
Okay to commit?
Once it is committed I would also backport it to glibc 2.29 release branch.
Bye,
Stefan
ChangeLog:
* elf/dl-sysdep.c (_dl_show_auxv): Fix condition if _dl_procinfo
is called.
* sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
Ignore types other than AT_HWCAP.
* sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
* sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo):
Likewise.
[-- Attachment #2: 20190308_ldshowauxv.patch --]
[-- Type: text/x-patch, Size: 3021 bytes --]
commit 5bbde0719d7c5fa58b0140d6fb1c6fa31a372772
Author: Stefan Liebler <stli@linux.ibm.com>
Date: Fri Mar 8 11:18:53 2019 +0100
Fix output of LD_SHOW_AUXV=1.
Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
the output was corrupted on some platforms as _dl_procinfo
was called for every auxv entry and on some architectures like s390
all entries were represented as "AT_HWCAP".
This patch fixes the condition which determines if _dl_procinfo
is called and adjusts _dl_procinfo implementations which assumed
that they are only called for AT_HWCAP or AT_HWCAP2.
ChangeLog:
* elf/dl-sysdep.c (_dl_show_auxv): Fix condition if _dl_procinfo
is called.
* sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
Ignore types other than AT_HWCAP.
* sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
* sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo): Likewise.
diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index 5f6c679a3f..1588555651 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -329,8 +329,10 @@ _dl_show_auxv (void)
assert (AT_IGNORE == 1);
if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2
- || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY
- || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY)
+ || av->a_type == AT_L1I_CACHEGEOMETRY
+ || av->a_type == AT_L1D_CACHEGEOMETRY
+ || av->a_type == AT_L2_CACHEGEOMETRY
+ || av->a_type == AT_L3_CACHEGEOMETRY)
{
/* These are handled in a special way per platform. */
if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h
index 282b8c5117..cc2687e99b 100644
--- a/sysdeps/sparc/dl-procinfo.h
+++ b/sysdeps/sparc/dl-procinfo.h
@@ -32,7 +32,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
int i;
/* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
index 22b43431bc..3aef14c6c1 100644
--- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
@@ -31,7 +31,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
int i;
/* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
index 19329a335b..16739fd6cd 100644
--- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
@@ -33,7 +33,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
int i;
/* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix output of LD_SHOW_AUXV=1.
2019-03-08 12:52 [PATCH] Fix output of LD_SHOW_AUXV=1 Stefan Liebler
@ 2019-03-08 14:51 ` Carlos O'Donell
2019-03-12 15:06 ` Stefan Liebler
0 siblings, 1 reply; 5+ messages in thread
From: Carlos O'Donell @ 2019-03-08 14:51 UTC (permalink / raw)
To: Stefan Liebler, GNU C Library
On 3/8/19 7:52 AM, Stefan Liebler wrote:
> Hi,
>
> starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
> the output was corrupted on some platforms as _dl_procinfo
> was called for every auxv entry and on some architectures like s390
> all entries were represented as "AT_HWCAP".
>
> This patch fixes the condition which determines if _dl_procinfo
> is called and adjusts _dl_procinfo implementations which assumed
> that they are only called for AT_HWCAP or AT_HWCAP2.
>
> A further question is if we should always call _dl_procinfo without a
> condition and let it decide if it is able to print an entry or if it
> should be printed with help of the auxvars list?
This is a design question:
(a) Call machine-specific routine *after* filtering out generic options.
vs.
(b) Call machine-specific routine for *all* options and let the the
machine-specific routine call a generic routine after it handles
all the options it wants.
We should *always* call _dl_procinfo, and let the architectures decide
if they will handle the entry in a special way.
We need to provide a "base" function helper to allow the arches to print
the information in a generic manner if their own code decides it doesn't
want to handle it.
This would involve a refactoring of the existing code though, so I'm not
going to require it as part of fixing this for s390x.
However, if we implement (b), then the generic code doesn't need to know
what will be handled by the machine-specific routines, so I see that as
a good cleanup.
$0.02.
> Okay to commit?
> Once it is committed I would also backport it to glibc 2.29 release branch.
OK for master.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> Bye,
> Stefan
>
> ChangeLog:
>
> * elf/dl-sysdep.c (_dl_show_auxv): Fix condition if _dl_procinfo
> is called.
> * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
> Ignore types other than AT_HWCAP.
> * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
> * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo):
> Likewise.
>
> 20190308_ldshowauxv.patch
>
> commit 5bbde0719d7c5fa58b0140d6fb1c6fa31a372772
> Author: Stefan Liebler <stli@linux.ibm.com>
> Date: Fri Mar 8 11:18:53 2019 +0100
>
> Fix output of LD_SHOW_AUXV=1.
>
> Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
> the output was corrupted on some platforms as _dl_procinfo
> was called for every auxv entry and on some architectures like s390
> all entries were represented as "AT_HWCAP".
>
> This patch fixes the condition which determines if _dl_procinfo
> is called and adjusts _dl_procinfo implementations which assumed
> that they are only called for AT_HWCAP or AT_HWCAP2.
OK.
> ChangeLog:
>
> * elf/dl-sysdep.c (_dl_show_auxv): Fix condition if _dl_procinfo
> is called.
> * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
> Ignore types other than AT_HWCAP.
> * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
> * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo): Likewise.
>
> diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
> index 5f6c679a3f..1588555651 100644
> --- a/elf/dl-sysdep.c
> +++ b/elf/dl-sysdep.c
> @@ -329,8 +329,10 @@ _dl_show_auxv (void)
> assert (AT_IGNORE == 1);
>
> if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2
> - || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY
> - || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY)
> + || av->a_type == AT_L1I_CACHEGEOMETRY
> + || av->a_type == AT_L1D_CACHEGEOMETRY
> + || av->a_type == AT_L2_CACHEGEOMETRY
> + || av->a_type == AT_L3_CACHEGEOMETRY)
OK. Fixes it so that we call into _dl_procinfo properly for the special cases.
> {
> /* These are handled in a special way per platform. */
> if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
> diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h
> index 282b8c5117..cc2687e99b 100644
> --- a/sysdeps/sparc/dl-procinfo.h
> +++ b/sysdeps/sparc/dl-procinfo.h
> @@ -32,7 +32,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
> int i;
>
> /* Fallback to unknown output mechanism. */
> - if (type == AT_HWCAP2)
> + if (type != AT_HWCAP)
OK.
> return -1;
>
> _dl_printf ("AT_HWCAP: ");
> diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
> index 22b43431bc..3aef14c6c1 100644
> --- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
> +++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
> @@ -31,7 +31,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
> int i;
>
> /* Fallback to unknown output mechanism. */
> - if (type == AT_HWCAP2)
> + if (type != AT_HWCAP)
OK.
> return -1;
>
> _dl_printf ("AT_HWCAP: ");
> diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
> index 19329a335b..16739fd6cd 100644
> --- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
> +++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
> @@ -33,7 +33,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
> int i;
>
> /* Fallback to unknown output mechanism. */
> - if (type == AT_HWCAP2)
> + if (type != AT_HWCAP)
OK.
> return -1;
>
> _dl_printf ("AT_HWCAP: ");
OK, verified sysdeps/unix/sysv/linux/arm/dl-procinfo.h uses a switch
statement that handles this correctly.
OK, verified sysdeps/powerpc/dl-procinfo.h uses a switch statement that
handles this correctly.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix output of LD_SHOW_AUXV=1.
2019-03-08 14:51 ` Carlos O'Donell
@ 2019-03-12 15:06 ` Stefan Liebler
2019-03-12 20:11 ` Carlos O'Donell
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Liebler @ 2019-03-12 15:06 UTC (permalink / raw)
To: Carlos O'Donell, GNU C Library
[-- Attachment #1: Type: text/plain, Size: 6579 bytes --]
On 3/8/19 3:51 PM, Carlos O'Donell wrote:
> On 3/8/19 7:52 AM, Stefan Liebler wrote:
>> Hi,
>>
>> starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
>> the output was corrupted on some platforms as _dl_procinfo
>> was called for every auxv entry and on some architectures like s390
>> all entries were represented as "AT_HWCAP".
>>
>> This patch fixes the condition which determines if _dl_procinfo
>> is called and adjusts _dl_procinfo implementations which assumed
>> that they are only called for AT_HWCAP or AT_HWCAP2.
>>
>> A further question is if we should always call _dl_procinfo without a
>> condition and let it decide if it is able to print an entry or if it
>> should be printed with help of the auxvars list?
>
> This is a design question:
>
> (a) Call machine-specific routine *after* filtering out generic options.
>
> vs.
>
> (b) Call machine-specific routine for *all* options and let the the
> machine-specific routine call a generic routine after it handles
> all the options it wants.
>
> We should *always* call _dl_procinfo, and let the architectures decide
> if they will handle the entry in a special way.
Exactly this is currently done as the condition is always true and e.g.
for x86_64 or powerpc it works correct.
The generic code (in elf/dl-sysdep.c) is printing an entry if the arch
is correctly returning a non-zero-value if it is not able to print it.
This is either done via auxvars information or by just printing "AT_???
..." for unknown entries.
Please have a look at the attached patch.
It does not require a helper function or refactoring. In fact it
simplifies the patch.
Compared to my previous patch only elf/dl-sysdep.c is changed.
Thanks,
Stefan
>
> We need to provide a "base" function helper to allow the arches to print
> the information in a generic manner if their own code decides it doesn't
> want to handle it.
>
> This would involve a refactoring of the existing code though, so I'm not
> going to require it as part of fixing this for s390x.
>
> However, if we implement (b), then the generic code doesn't need to know
> what will be handled by the machine-specific routines, so I see that as
> a good cleanup.
>
> $0.02.
>
>> Okay to commit?
>> Once it is committed I would also backport it to glibc 2.29 release branch.
>
> OK for master.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
>> Bye,
>> Stefan
>>
>> ChangeLog:
>>
>> * elf/dl-sysdep.c (_dl_show_auxv): Fix condition if _dl_procinfo
>> is called.
>> * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
>> Ignore types other than AT_HWCAP.
>> * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
>> * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo):
>> Likewise.
>>
>> 20190308_ldshowauxv.patch
>>
>> commit 5bbde0719d7c5fa58b0140d6fb1c6fa31a372772
>> Author: Stefan Liebler <stli@linux.ibm.com>
>> Date: Fri Mar 8 11:18:53 2019 +0100
>>
>> Fix output of LD_SHOW_AUXV=1.
>>
>> Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
>> the output was corrupted on some platforms as _dl_procinfo
>> was called for every auxv entry and on some architectures like s390
>> all entries were represented as "AT_HWCAP".
>>
>> This patch fixes the condition which determines if _dl_procinfo
>> is called and adjusts _dl_procinfo implementations which assumed
>> that they are only called for AT_HWCAP or AT_HWCAP2.
>
> OK.
>
>> ChangeLog:
>>
>> * elf/dl-sysdep.c (_dl_show_auxv): Fix condition if _dl_procinfo
>> is called.
>> * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
>> Ignore types other than AT_HWCAP.
>> * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
>> * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo): Likewise.
>>
>> diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
>> index 5f6c679a3f..1588555651 100644
>> --- a/elf/dl-sysdep.c
>> +++ b/elf/dl-sysdep.c
>> @@ -329,8 +329,10 @@ _dl_show_auxv (void)
>> assert (AT_IGNORE == 1);
>>
>> if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2
>> - || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY
>> - || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY)
>> + || av->a_type == AT_L1I_CACHEGEOMETRY
>> + || av->a_type == AT_L1D_CACHEGEOMETRY
>> + || av->a_type == AT_L2_CACHEGEOMETRY
>> + || av->a_type == AT_L3_CACHEGEOMETRY)
>
> OK. Fixes it so that we call into _dl_procinfo properly for the special cases.
>
>> {
>> /* These are handled in a special way per platform. */
>> if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
>> diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h
>> index 282b8c5117..cc2687e99b 100644
>> --- a/sysdeps/sparc/dl-procinfo.h
>> +++ b/sysdeps/sparc/dl-procinfo.h
>> @@ -32,7 +32,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
>> int i;
>>
>> /* Fallback to unknown output mechanism. */
>> - if (type == AT_HWCAP2)
>> + if (type != AT_HWCAP)
>
> OK.
>
>> return -1;
>>
>> _dl_printf ("AT_HWCAP: ");
>> diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
>> index 22b43431bc..3aef14c6c1 100644
>> --- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
>> +++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
>> @@ -31,7 +31,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
>> int i;
>>
>> /* Fallback to unknown output mechanism. */
>> - if (type == AT_HWCAP2)
>> + if (type != AT_HWCAP)
>
> OK.
>
>> return -1;
>>
>> _dl_printf ("AT_HWCAP: ");
>> diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
>> index 19329a335b..16739fd6cd 100644
>> --- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
>> +++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
>> @@ -33,7 +33,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
>> int i;
>>
>> /* Fallback to unknown output mechanism. */
>> - if (type == AT_HWCAP2)
>> + if (type != AT_HWCAP)
>
> OK.
>
>> return -1;
>>
>> _dl_printf ("AT_HWCAP: ");
>
> OK, verified sysdeps/unix/sysv/linux/arm/dl-procinfo.h uses a switch
> statement that handles this correctly.
>
> OK, verified sysdeps/powerpc/dl-procinfo.h uses a switch statement that
> handles this correctly.
>
[-- Attachment #2: 20190312_ldshowauxv.patch --]
[-- Type: text/x-patch, Size: 3336 bytes --]
commit 988376a08d6d36ea321203b9c039db4b266609ae
Author: Stefan Liebler <stli@linux.ibm.com>
Date: Fri Mar 8 11:18:53 2019 +0100
Fix output of LD_SHOW_AUXV=1.
Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
the output was corrupted on some platforms as _dl_procinfo
was called for every auxv entry and on some architectures like s390
all entries were represented as "AT_HWCAP".
This patch is removing the condition and let _dl_procinfo decide if
an entry is printed in a platform specific or generic way.
This patch also adjusts all _dl_procinfo implementations which assumed
that they are only called for AT_HWCAP or AT_HWCAP2. They are now just
returning a non-zero-value for entries which are not handled platform
specifc.
ChangeLog:
* elf/dl-sysdep.c (_dl_show_auxv): Remove condition and always
call _dl_procinfo.
* sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
Ignore types other than AT_HWCAP.
* sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
* sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo):
Likewise.
diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index 5f6c679a3f..5d19b100b2 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -328,14 +328,9 @@ _dl_show_auxv (void)
assert (AT_NULL == 0);
assert (AT_IGNORE == 1);
- if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2
- || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY
- || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY)
- {
- /* These are handled in a special way per platform. */
- if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
- continue;
- }
+ /* Some entries are handled in a special way per platform. */
+ if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
+ continue;
if (idx < sizeof (auxvars) / sizeof (auxvars[0])
&& auxvars[idx].form != unknown)
diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h
index 282b8c5117..cc2687e99b 100644
--- a/sysdeps/sparc/dl-procinfo.h
+++ b/sysdeps/sparc/dl-procinfo.h
@@ -32,7 +32,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
int i;
/* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
index 22b43431bc..3aef14c6c1 100644
--- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
@@ -31,7 +31,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
int i;
/* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
index 19329a335b..16739fd6cd 100644
--- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
@@ -33,7 +33,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
int i;
/* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix output of LD_SHOW_AUXV=1.
2019-03-12 15:06 ` Stefan Liebler
@ 2019-03-12 20:11 ` Carlos O'Donell
2019-03-13 9:54 ` [2.29 COMMITTED] " Stefan Liebler
0 siblings, 1 reply; 5+ messages in thread
From: Carlos O'Donell @ 2019-03-12 20:11 UTC (permalink / raw)
To: Stefan Liebler, GNU C Library
On 3/12/19 11:06 AM, Stefan Liebler wrote:
> commit 988376a08d6d36ea321203b9c039db4b266609ae
> Author: Stefan Liebler <stli@linux.ibm.com>
> Date: Fri Mar 8 11:18:53 2019 +0100
>
> Fix output of LD_SHOW_AUXV=1.
>
> Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
> the output was corrupted on some platforms as _dl_procinfo
> was called for every auxv entry and on some architectures like s390
> all entries were represented as "AT_HWCAP".
>
> This patch is removing the condition and let _dl_procinfo decide if
> an entry is printed in a platform specific or generic way.
> This patch also adjusts all _dl_procinfo implementations which assumed
> that they are only called for AT_HWCAP or AT_HWCAP2. They are now just
> returning a non-zero-value for entries which are not handled platform
> specifc.
>
> ChangeLog:
>
> * elf/dl-sysdep.c (_dl_show_auxv): Remove condition and always
> call _dl_procinfo.
> * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
> Ignore types other than AT_HWCAP.
> * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
> * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo):
> Likewise.
Thanks for the cleanup, this makes it really clear how this is intended to work.
OK for master if you...
- Fix ARM and POWERPC backend _dl_procinfo to remove "/* This should not happen. */"
comment in dl-procinfo.c. It *does* happen now because we pass all options to
the machine version first.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
> index 5f6c679a3f..5d19b100b2 100644
> --- a/elf/dl-sysdep.c
> +++ b/elf/dl-sysdep.c
> @@ -328,14 +328,9 @@ _dl_show_auxv (void)
> assert (AT_NULL == 0);
> assert (AT_IGNORE == 1);
>
> - if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2
> - || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY
> - || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY)
> - {
> - /* These are handled in a special way per platform. */
> - if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
> - continue;
> - }
> + /* Some entries are handled in a special way per platform. */
> + if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
> + continue;
OK.
OK. ARM backend returns -1 with default handling.
OK. POWERPC backend return -1 with default handling.
>
> if (idx < sizeof (auxvars) / sizeof (auxvars[0])
> && auxvars[idx].form != unknown)
> diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h
> index 282b8c5117..cc2687e99b 100644
> --- a/sysdeps/sparc/dl-procinfo.h
> +++ b/sysdeps/sparc/dl-procinfo.h
> @@ -32,7 +32,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
> int i;
>
> /* Fallback to unknown output mechanism. */
> - if (type == AT_HWCAP2)
> + if (type != AT_HWCAP)
OK.
> return -1;
>
> _dl_printf ("AT_HWCAP: ");
> diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
> index 22b43431bc..3aef14c6c1 100644
> --- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
> +++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
> @@ -31,7 +31,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
> int i;
>
> /* Fallback to unknown output mechanism. */
> - if (type == AT_HWCAP2)
> + if (type != AT_HWCAP)
OK.
> return -1;
>
> _dl_printf ("AT_HWCAP: ");
> diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
> index 19329a335b..16739fd6cd 100644
> --- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
> +++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
> @@ -33,7 +33,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
> int i;
>
> /* Fallback to unknown output mechanism. */
> - if (type == AT_HWCAP2)
> + if (type != AT_HWCAP)
OK.
> return -1;
>
> _dl_printf ("AT_HWCAP: ");
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [2.29 COMMITTED] Fix output of LD_SHOW_AUXV=1.
2019-03-12 20:11 ` Carlos O'Donell
@ 2019-03-13 9:54 ` Stefan Liebler
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Liebler @ 2019-03-13 9:54 UTC (permalink / raw)
To: GNU C Library, libc-stable; +Cc: Carlos O'Donell
[-- Attachment #1: Type: text/plain, Size: 4411 bytes --]
On 3/12/19 9:11 PM, Carlos O'Donell wrote:
> On 3/12/19 11:06 AM, Stefan Liebler wrote:
>> commit 988376a08d6d36ea321203b9c039db4b266609ae
>> Author: Stefan Liebler <stli@linux.ibm.com>
>> Date: Fri Mar 8 11:18:53 2019 +0100
>>
>> Fix output of LD_SHOW_AUXV=1.
>>
>> Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
>> the output was corrupted on some platforms as _dl_procinfo
>> was called for every auxv entry and on some architectures like s390
>> all entries were represented as "AT_HWCAP".
>>
>> This patch is removing the condition and let _dl_procinfo decide if
>> an entry is printed in a platform specific or generic way.
>> This patch also adjusts all _dl_procinfo implementations which assumed
>> that they are only called for AT_HWCAP or AT_HWCAP2. They are now just
>> returning a non-zero-value for entries which are not handled platform
>> specifc.
>>
>> ChangeLog:
>>
>> * elf/dl-sysdep.c (_dl_show_auxv): Remove condition and always
>> call _dl_procinfo.
>> * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
>> Ignore types other than AT_HWCAP.
>> * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
>> * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo):
>> Likewise.
>
> Thanks for the cleanup, this makes it really clear how this is intended to work.
>
> OK for master if you...
> - Fix ARM and POWERPC backend _dl_procinfo to remove "/* This should not happen. */"
> comment in dl-procinfo.c. It *does* happen now because we pass all options to
> the machine version first.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
Okay. I've adjusted those comments and committed the patch to master and
backported it to glibc release branch 2.29.
Thanks,
Stefan
>> diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
>> index 5f6c679a3f..5d19b100b2 100644
>> --- a/elf/dl-sysdep.c
>> +++ b/elf/dl-sysdep.c
>> @@ -328,14 +328,9 @@ _dl_show_auxv (void)
>> assert (AT_NULL == 0);
>> assert (AT_IGNORE == 1);
>>
>> - if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2
>> - || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY
>> - || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY)
>> - {
>> - /* These are handled in a special way per platform. */
>> - if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
>> - continue;
>> - }
>> + /* Some entries are handled in a special way per platform. */
>> + if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
>> + continue;
>
> OK.
>
> OK. ARM backend returns -1 with default handling.
>
> OK. POWERPC backend return -1 with default handling.
>
>>
>> if (idx < sizeof (auxvars) / sizeof (auxvars[0])
>> && auxvars[idx].form != unknown)
>> diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h
>> index 282b8c5117..cc2687e99b 100644
>> --- a/sysdeps/sparc/dl-procinfo.h
>> +++ b/sysdeps/sparc/dl-procinfo.h
>> @@ -32,7 +32,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
>> int i;
>>
>> /* Fallback to unknown output mechanism. */
>> - if (type == AT_HWCAP2)
>> + if (type != AT_HWCAP)
>
> OK.
>
>> return -1;
>>
>> _dl_printf ("AT_HWCAP: ");
>> diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
>> index 22b43431bc..3aef14c6c1 100644
>> --- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
>> +++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
>> @@ -31,7 +31,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
>> int i;
>>
>> /* Fallback to unknown output mechanism. */
>> - if (type == AT_HWCAP2)
>> + if (type != AT_HWCAP)
>
> OK.
>
>> return -1;
>>
>> _dl_printf ("AT_HWCAP: ");
>> diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
>> index 19329a335b..16739fd6cd 100644
>> --- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
>> +++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
>> @@ -33,7 +33,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
>> int i;
>>
>> /* Fallback to unknown output mechanism. */
>> - if (type == AT_HWCAP2)
>> + if (type != AT_HWCAP)
>
> OK.
>
>> return -1;
>>
>> _dl_printf ("AT_HWCAP: ");
>
>
[-- Attachment #2: 20190313_ldshowauxv.patch --]
[-- Type: text/x-patch, Size: 4692 bytes --]
commit daede01d78492f2f7b640966a2e60ff409be677a
Author: Stefan Liebler <stli@linux.ibm.com>
Date: Fri Mar 8 11:18:53 2019 +0100
Fix output of LD_SHOW_AUXV=1.
Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e
the output was corrupted on some platforms as _dl_procinfo
was called for every auxv entry and on some architectures like s390
all entries were represented as "AT_HWCAP".
This patch is removing the condition and let _dl_procinfo decide if
an entry is printed in a platform specific or generic way.
This patch also adjusts all _dl_procinfo implementations which assumed
that they are only called for AT_HWCAP or AT_HWCAP2. They are now just
returning a non-zero-value for entries which are not handled platform
specifc.
ChangeLog:
* elf/dl-sysdep.c (_dl_show_auxv): Remove condition and always
call _dl_procinfo.
* sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo):
Ignore types other than AT_HWCAP.
* sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise.
* sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo):
Likewise.
* sysdeps/powerpc/dl-procinfo.h (_dl_procinfo): Adjust comment
in the case of falling back to generic output mechanism.
* sysdeps/unix/sysv/linux/arm/dl-procinfo.h (_dl_procinfo):
Likewise.
diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index 5f6c679a3f..5d19b100b2 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -328,14 +328,9 @@ _dl_show_auxv (void)
assert (AT_NULL == 0);
assert (AT_IGNORE == 1);
- if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2
- || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY
- || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY)
- {
- /* These are handled in a special way per platform. */
- if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
- continue;
- }
+ /* Some entries are handled in a special way per platform. */
+ if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0)
+ continue;
if (idx < sizeof (auxvars) / sizeof (auxvars[0])
&& auxvars[idx].form != unknown)
diff --git a/sysdeps/powerpc/dl-procinfo.h b/sysdeps/powerpc/dl-procinfo.h
index f542f7318f..dfc3b33a72 100644
--- a/sysdeps/powerpc/dl-procinfo.h
+++ b/sysdeps/powerpc/dl-procinfo.h
@@ -225,7 +225,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
break;
}
default:
- /* This should not happen. */
+ /* Fallback to generic output mechanism. */
return -1;
}
_dl_printf ("\n");
diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h
index 282b8c5117..64ee267fc7 100644
--- a/sysdeps/sparc/dl-procinfo.h
+++ b/sysdeps/sparc/dl-procinfo.h
@@ -31,8 +31,8 @@ _dl_procinfo (unsigned int type, unsigned long int word)
{
int i;
- /* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ /* Fallback to generic output mechanism. */
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
diff --git a/sysdeps/unix/sysv/linux/arm/dl-procinfo.h b/sysdeps/unix/sysv/linux/arm/dl-procinfo.h
index 66c00297b7..05c62c8687 100644
--- a/sysdeps/unix/sysv/linux/arm/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/arm/dl-procinfo.h
@@ -67,7 +67,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
break;
}
default:
- /* This should not happen. */
+ /* Fallback to generic output mechanism. */
return -1;
}
_dl_printf ("\n");
diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
index 22b43431bc..0585cdaa9c 100644
--- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h
@@ -30,8 +30,8 @@ _dl_procinfo (unsigned int type, unsigned long int word)
in the kernel sources. */
int i;
- /* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ /* Fallback to generic output mechanism. */
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
index 19329a335b..d67fde368f 100644
--- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h
@@ -32,8 +32,8 @@ _dl_procinfo (unsigned int type, unsigned long int word)
in the kernel sources. */
int i;
- /* Fallback to unknown output mechanism. */
- if (type == AT_HWCAP2)
+ /* Fallback to generic output mechanism. */
+ if (type != AT_HWCAP)
return -1;
_dl_printf ("AT_HWCAP: ");
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-13 9:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 12:52 [PATCH] Fix output of LD_SHOW_AUXV=1 Stefan Liebler
2019-03-08 14:51 ` Carlos O'Donell
2019-03-12 15:06 ` Stefan Liebler
2019-03-12 20:11 ` Carlos O'Donell
2019-03-13 9:54 ` [2.29 COMMITTED] " Stefan Liebler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).