[2.29,COMMITTED] Fix output of LD_SHOW_AUXV=1.

Message ID 26473622-44fc-b2ae-6132-e7d475190f95@linux.ibm.com
State New
Headers show
Series
  • [2.29,COMMITTED] Fix output of LD_SHOW_AUXV=1.
Related show

Commit Message

Stefan Liebler March 13, 2019, 9:54 a.m.
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:   ");

> 

>

Patch

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