Don't use nested function in test-ffs

There is no real need to use a nested function in that test, so break
it out so that it can build with clang too.
---
 string/test-ffs.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

The 11/12/2020 13:27, Siddhesh Poyarekar via Libc-alpha wrote:
> There is no real need to use a nested function in that test, so break
> it out so that it can build with clang too.
> ---
>  string/test-ffs.c | 37 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/string/test-ffs.c b/string/test-ffs.c
> index 0df488fa2b..a2a606522a 100644
> --- a/string/test-ffs.c
> +++ b/string/test-ffs.c
> @@ -21,34 +21,33 @@
>  #include <stdio.h>
>  #include <string.h>
>  
> +void try (const char *name, long long int param, int value, int expected,
> +	  int *failuresp)
> +{
> +  if (value != expected)
> +    {
> +      printf ("%s(%#llx) expected %d got %d\n",
> +	      name, param, expected, value);
> +      *failuresp += 1;
> +    }
> +  else
> +    printf ("%s(%#llx) as expected %d\n", name, param, value);
> +}
> +
>  int
>  do_test (void)
>  {
>    int failures = 0;
>    int i;

this works, but i think you can just make failures global.
(then a bit less changes are needed)

>  
> -  auto void try (const char *name, long long int param, int value,
> -		 int expected);
> -
> -  void try (const char *name, long long int param, int value, int expected)
> -    {
> -      if (value != expected)
> -	{
> -	  printf ("%s(%#llx) expected %d got %d\n",
> -		  name, param, expected, value);
> -	  ++failures;
> -	}
> -      else
> -	printf ("%s(%#llx) as expected %d\n", name, param, value);
> -    }
> -
>  #define TEST(fct, type) \
> -  try (#fct, 0, fct ((type) 0), 0);					      \
> +  try (#fct, 0, fct ((type) 0), 0, &failures);				      \
>    for (i=0 ; i < 8 * sizeof (type); i++)				      \
> -    try (#fct, 1ll << i, fct (((type) 1) << i), i + 1);			      \
> +    try (#fct, 1ll << i, fct (((type) 1) << i), i + 1, &failures);	      \
>    for (i=0 ; i < 8 * sizeof (type) ; i++)				      \
> -    try (#fct, (~((type) 0) >> i) << i, fct ((~((type) 0) >> i) << i), i + 1);\
> -  try (#fct, 0x80008000, fct ((type) 0x80008000), 16)
> +    try (#fct, (~((type) 0) >> i) << i, fct ((~((type) 0) >> i) << i), i + 1, \
> +	 &failures);							      \
> +  try (#fct, 0x80008000, fct ((type) 0x80008000), 16, &failures)
>  
>    TEST (ffs, int);
>    TEST (ffsl, long int);
> -- 
> 2.26.2
>

The 11/12/2020 13:27, Siddhesh Poyarekar via Libc-alpha wrote:
> There is no real need to use a nested function in that test, so break
> it out so that it can build with clang too.

btw last time i looked at clang, mostly to build musl, the
main issues were incompatible inline asm handling on some
targets breaking inline syscalls and lack of fenv support
breaking libm.

and in glibc i'd expect the nested functions in the dynamic
linker to be challenging to fix: that will affect generic
and target code too.

* Szabolcs Nagy via Libc-alpha:

> The 11/12/2020 13:27, Siddhesh Poyarekar via Libc-alpha wrote:
>> There is no real need to use a nested function in that test, so break
>> it out so that it can build with clang too.
>
> btw last time i looked at clang, mostly to build musl, the
> main issues were incompatible inline asm handling on some
> targets breaking inline syscalls and lack of fenv support
> breaking libm.

I think there's also an issue where assembler redirects of standard C
functions do not affect builtins.

For example, GCC (on targets where the copy is not expanded inline) will
generate a call to memcpy_redirect for this:

void *memcpy (void *, const void *, __typeof__ (sizeof (0)))
  __asm__ ("memcpy_redirect");

struct S { int a[128]; };

void
copy (const struct S *from, struct S *to)
{
  *to = *from;
}

I believe Clang calls memcpy instead.

> and in glibc i'd expect the nested functions in the dynamic
> linker to be challenging to fix: that will affect generic
> and target code too.

Yes, but that #include at the top level and in the middle of a function
definition I would like to see fixed *anyway*. 8-)

My understanding is that Siddhesh only wants to run the test suite with
Clang, which I think is a worthwhile goal even if we do not intend to
build glibc with Clang.

Thanks,
Florian

* Szabolcs Nagy via Libc-alpha:

> this works, but i think you can just make failures global.
> (then a bit less changes are needed)

Or use the failure counter maintained by <support/check.h>

Thanks,
Florian

On 11/12/20 1:54 PM, Szabolcs Nagy via Libc-alpha wrote:
>>   int
>>   do_test (void)
>>   {
>>     int failures = 0;
>>     int i;
> 
> this works, but i think you can just make failures global.
> (then a bit less changes are needed)

Of course, I'll fix that up and commit.

 > btw last time i looked at clang, mostly to build musl, the
 > main issues were incompatible inline asm handling on some
 > targets breaking inline syscalls and lack of fenv support
 > breaking libm.
 >
 > and in glibc i'd expect the nested functions in the dynamic
 > linker to be challenging to fix: that will affect generic
 > and target code too.

Right now I'm only interested in getting the testsuite (at least the 
parts I need at the moment) clean enough that it doesn't depend on 
gcc-only GNU extensions.  That will hopefully help glibc get better 
ABI/API testing coverage across compilers.

Siddhesh