[14/28] elf: Enhance ld.so --help to print HWCAP subdirectories

  ---
 elf/dl-usage.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)
  

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote:
> ---
>  elf/dl-usage.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/elf/dl-usage.c b/elf/dl-usage.c
> index 20aa715cb1..9765d1b5c1 100644
> --- a/elf/dl-usage.c
> +++ b/elf/dl-usage.c
> @@ -22,6 +22,8 @@
>  #include <unistd.h>
>  #include "version.h"
>  
> +#include <dl-hwcaps.h>
> +
>  void
>  _dl_usage (const char *argv0, const char *wrong_option)
>  {
> @@ -101,6 +103,65 @@ print_search_path_for_help (struct dl_main_state *state)
>    print_search_path_for_help_1 (__rtld_search_dirs.dirs);
>  }
>  
> +/* Helper function for printing flags associated with a HWCAP name.  */
> +static void
> +print_hwcap_1 (bool *first, bool active, const char *label)
> +{
> +  if (active)
> +    {
> +      if (*first)
> +        {
> +          _dl_printf (" (");
> +          *first = false;
> +        }
> +      else
> +        _dl_printf (", ");
> +      _dl_printf ("%s", label);
> +    }
> +}
> +
> +/* Called after a series of print_hwcap_1 calls to emit the line
> +   terminator.  */
> +static void
> +print_hwcap_1_finish (bool *first)
> +{
> +  if (*first)
> +    _dl_printf ("\n");
> +  else
> +    _dl_printf (")\n");
> +}
> +
> +/* Write a list of hwcap subdirectories to standard output.  See
> + _dl_important_hwcaps in dl-hwcaps.c.  */
> +static void
> +print_legacy_hwcap_directories (void)
> +{
> +  _dl_printf ("\n\
> +Legacy HWCAP subdirectories under library search path directories:\n");
> +
> +  const char *platform = GLRO (dl_platform);
> +  if (platform != NULL)
> +    _dl_printf ("  %s (AT_PLATFORM)\n", platform);
> +
> +  _dl_printf ("  tls (supported, searched)\n");
> +
> +  uint64_t hwcap_mask = GET_HWCAP_MASK();
> +  uint64_t searched = GLRO (dl_hwcap) & hwcap_mask;
> +  for (int n = 63; n >= 0; --n)
> +    {
> +      uint64_t bit = 1ULL << n;

UINT64_C(1) maybe?

> +      if (HWCAP_IMPORTANT & bit)
> +        {
> +          _dl_printf ("  %s", _dl_hwcap_string (n));
> +          bool first = true;
> +          print_hwcap_1 (&first, GLRO (dl_hwcap) & bit, "supported");
> +          print_hwcap_1 (&first, !(hwcap_mask & bit), "masked");
> +          print_hwcap_1 (&first, searched & bit, "searched");
> +          print_hwcap_1_finish (&first);
> +        }
> +    }
> +}
> +

Ok.

>  void
>  _dl_help (const char *argv0, struct dl_main_state *state)
>  {
> @@ -136,5 +197,6 @@ This program interpreter self-identifies as: " RTLD "\n\
>  ",
>                argv0);
>    print_search_path_for_help (state);
> +  print_legacy_hwcap_directories ();
>    _exit (0);
>  }
> 

Ok.
  

* Adhemerval Zanella via Libc-alpha:

> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

Thanks.

>> +  const char *platform = GLRO (dl_platform);
>> +  if (platform != NULL)
>> +    _dl_printf ("  %s (AT_PLATFORM)\n", platform);

I'm going to change this to (AT_PLATFORM; supported, searched) for
consistency with the other lines.

>> +
>> +  _dl_printf ("  tls (supported, searched)\n");
>> +
>> +  uint64_t hwcap_mask = GET_HWCAP_MASK();
>> +  uint64_t searched = GLRO (dl_hwcap) & hwcap_mask;
>> +  for (int n = 63; n >= 0; --n)
>> +    {
>> +      uint64_t bit = 1ULL << n;
>
> UINT64_C(1) maybe?

unsigned long long needs to be at least 64 bits in our environment, so I
think this should be fine.

Florian
  

Hi Florian,

After this patch was merged I started seeing the following error when
building with --disable-tunables:

l-usage.c: In function ‘print_legacy_hwcap_directories’:
dl-usage.c:153:11: error: ‘HWCAP_IMPORTANT’ undeclared (first use in
this function)
       if (HWCAP_IMPORTANT & bit)
           ^~~~~~~~~~~~~~~
dl-usage.c:153:11: note: each undeclared identifier is reported only
once for each function it appears in
dl-usage.c:155:31: error: implicit declaration of function
‘_dl_hwcap_string’; did you mean ‘_dl_fatal_printf’?
[-Werror=implicit-function-declaration]
           _dl_printf ("  %s", _dl_hwcap_string (n));
                               ^~~~~~~~~~~~~~~~
                               _dl_fatal_printf
dl-usage.c:155:27: error: format ‘%s’ expects argument of type ‘char *’,
but argument 2 has type ‘int’ [-Werror=format=]
           _dl_printf ("  %s", _dl_hwcap_string (n));
                          ~^   ~~~~~~~~~~~~~~~~~~~~
                          %d
cc1: all warnings being treated as errors
make[2]: *** [../o-iterator.mk:9:
/home/tcbot/bot-worker/glibc-ppc64le-power8-notunables/vpath/elf/dl-usage.os]
Error 1


I could reproduce this on both ppc64le and x84_64 with
--disable-tunables. Are we missing an `#include <dl-procinfo.h>`?

diff --git a/elf/dl-usage.c b/elf/dl-usage.c
index c07f43835b..796ad38b43 100644
--- a/elf/dl-usage.c
+++ b/elf/dl-usage.c
@@ -22,6 +22,7 @@
 #include <unistd.h>
 #include "version.h"

+#include <dl-procinfo.h>
 #include <dl-hwcaps.h>

 void


--
Matheus Castanho
  

* Matheus Castanho:

> I could reproduce this on both ppc64le and x84_64 with
> --disable-tunables. Are we missing an `#include <dl-procinfo.h>`?
>
> diff --git a/elf/dl-usage.c b/elf/dl-usage.c
> index c07f43835b..796ad38b43 100644
> --- a/elf/dl-usage.c
> +++ b/elf/dl-usage.c
> @@ -22,6 +22,7 @@
>  #include <unistd.h>
>  #include "version.h"
>
> +#include <dl-procinfo.h>
>  #include <dl-hwcaps.h>

Right, I think that's what's missing.  It's an oversight on my part.  I
tried build-many-glibcs.py on the whole series, but it doesn't have a
configuration without tunables.

Would you please post your patch to the list?  You can also push it at
the same time if you want, I think missing #includes are considered
trivial under the check-in policy.

Thanks,
Florian
  

* Florian Weimer via Libc-alpha:

> * Matheus Castanho:
>
>> I could reproduce this on both ppc64le and x84_64 with
>> --disable-tunables. Are we missing an `#include <dl-procinfo.h>`?
>>
>> diff --git a/elf/dl-usage.c b/elf/dl-usage.c
>> index c07f43835b..796ad38b43 100644
>> --- a/elf/dl-usage.c
>> +++ b/elf/dl-usage.c
>> @@ -22,6 +22,7 @@
>>  #include <unistd.h>
>>  #include "version.h"
>>
>> +#include <dl-procinfo.h>
>>  #include <dl-hwcaps.h>
>
> Right, I think that's what's missing.  It's an oversight on my part.  I
> tried build-many-glibcs.py on the whole series, but it doesn't have a
> configuration without tunables.
>
> Would you please post your patch to the list?  You can also push it at
> the same time if you want, I think missing #includes are considered
> trivial under the check-in policy.

Eh, clearly you have posted this to the list already.  It's been a long
day. 8-/

Please check this in at your convenience.  Thanks.

Florian
  

On 10/9/20 2:12 PM, Florian Weimer wrote:
> * Florian Weimer via Libc-alpha:
> 
>> * Matheus Castanho:
>>
>>> I could reproduce this on both ppc64le and x84_64 with
>>> --disable-tunables. Are we missing an `#include <dl-procinfo.h>`?
>>>
>>> diff --git a/elf/dl-usage.c b/elf/dl-usage.c
>>> index c07f43835b..796ad38b43 100644
>>> --- a/elf/dl-usage.c
>>> +++ b/elf/dl-usage.c
>>> @@ -22,6 +22,7 @@
>>>  #include <unistd.h>
>>>  #include "version.h"
>>>
>>> +#include <dl-procinfo.h>
>>>  #include <dl-hwcaps.h>
>>
>> Right, I think that's what's missing.  It's an oversight on my part.  I
>> tried build-many-glibcs.py on the whole series, but it doesn't have a
>> configuration without tunables.
>>
>> Would you please post your patch to the list?  You can also push it at
>> the same time if you want, I think missing #includes are considered
>> trivial under the check-in policy.
> 
> Eh, clearly you have posted this to the list already.  It's been a long
> day. 8-/
> 
> Please check this in at your convenience.  Thanks.
> 
> Florian
> 

Ok. I'll ask someone to push it for me then. Thanks!

--
Matheus Castanho
  

* Matheus Castanho via Libc-alpha:

> On 10/9/20 2:12 PM, Florian Weimer wrote:
>> * Florian Weimer via Libc-alpha:
>> 
>>> * Matheus Castanho:
>>>
>>>> I could reproduce this on both ppc64le and x84_64 with
>>>> --disable-tunables. Are we missing an `#include <dl-procinfo.h>`?
>>>>
>>>> diff --git a/elf/dl-usage.c b/elf/dl-usage.c
>>>> index c07f43835b..796ad38b43 100644
>>>> --- a/elf/dl-usage.c
>>>> +++ b/elf/dl-usage.c
>>>> @@ -22,6 +22,7 @@
>>>>  #include <unistd.h>
>>>>  #include "version.h"
>>>>
>>>> +#include <dl-procinfo.h>
>>>>  #include <dl-hwcaps.h>
>>>
>>> Right, I think that's what's missing.  It's an oversight on my part.  I
>>> tried build-many-glibcs.py on the whole series, but it doesn't have a
>>> configuration without tunables.
>>>
>>> Would you please post your patch to the list?  You can also push it at
>>> the same time if you want, I think missing #includes are considered
>>> trivial under the check-in policy.
>> 
>> Eh, clearly you have posted this to the list already.  It's been a long
>> day. 8-/
>> 
>> Please check this in at your convenience.  Thanks.
>> 
>> Florian
>> 
>
> Ok. I'll ask someone to push it for me then. Thanks!

I've pushed it for you.

Thanks,
Florian