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:   ");
> 
>