llvm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] selftests/vDSO: fix clang build errors and warnings
@ 2024-05-27 21:16 John Hubbard
  2024-05-29  8:05 ` Muhammad Usama Anjum
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: John Hubbard @ 2024-05-27 21:16 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Mark Brown, Vincenzo Frascino, Colin Ian King, Valentin Obst,
	linux-kselftest, LKML, llvm, John Hubbard

When building with clang, via:

    make LLVM=1 -C tools/testing/selftests

...there are several warnings, and an error. This fixes all of those and
allows these tests to run and pass.

1. Fix linker error (undefined reference to memcpy) by providing a local
   version of memcpy.

2. clang complains about using this form:

    if (g = h & 0xf0000000)

...so factor out the assignment into a separate step.

3. The code is passing a signed const char* to elf_hash(), which expects
   a const unsigned char *. There are several callers, so fix this at
   the source by allowing the function to accept a signed argument, and
   then converting to unsigned operations, once inside the function.

4. clang doesn't have __attribute__((externally_visible)) and generates
   a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
   to require that attribute in order to build, run and pass tests here,
   so remove it.

[1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---

Changes since the first version:

1) Rebased onto Linux 6.10-rc1

thanks,
John Hubbard

 tools/testing/selftests/vDSO/parse_vdso.c      | 16 +++++++++++-----
 .../selftests/vDSO/vdso_standalone_test_x86.c  | 18 ++++++++++++++++--
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c
index 413f75620a35..4ae417372e9e 100644
--- a/tools/testing/selftests/vDSO/parse_vdso.c
+++ b/tools/testing/selftests/vDSO/parse_vdso.c
@@ -55,14 +55,20 @@ static struct vdso_info
 	ELF(Verdef) *verdef;
 } vdso_info;
 
-/* Straight from the ELF specification. */
-static unsigned long elf_hash(const unsigned char *name)
+/*
+ * Straight from the ELF specification...and then tweaked slightly, in order to
+ * avoid a few clang warnings.
+ */
+static unsigned long elf_hash(const char *name)
 {
 	unsigned long h = 0, g;
-	while (*name)
+	const unsigned char *uch_name = (const unsigned char *)name;
+
+	while (*uch_name)
 	{
-		h = (h << 4) + *name++;
-		if (g = h & 0xf0000000)
+		h = (h << 4) + *uch_name++;
+		g = h & 0xf0000000;
+		if (g)
 			h ^= g >> 24;
 		h &= ~g;
 	}
diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
index 8a44ff973ee1..27f6fdf11969 100644
--- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
+++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
@@ -18,7 +18,7 @@
 
 #include "parse_vdso.h"
 
-/* We need a libc functions... */
+/* We need some libc functions... */
 int strcmp(const char *a, const char *b)
 {
 	/* This implementation is buggy: it never returns -1. */
@@ -34,6 +34,20 @@ int strcmp(const char *a, const char *b)
 	return 0;
 }
 
+/*
+ * The clang build needs this, although gcc does not.
+ * Stolen from lib/string.c.
+ */
+void *memcpy(void *dest, const void *src, size_t count)
+{
+	char *tmp = dest;
+	const char *s = src;
+
+	while (count--)
+		*tmp++ = *s++;
+	return dest;
+}
+
 /* ...and two syscalls.  This is x86-specific. */
 static inline long x86_syscall3(long nr, long a0, long a1, long a2)
 {
@@ -70,7 +84,7 @@ void to_base10(char *lastdig, time_t n)
 	}
 }
 
-__attribute__((externally_visible)) void c_main(void **stack)
+void c_main(void **stack)
 {
 	/* Parse the stack */
 	long argc = (long)*stack;

base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings
  2024-05-27 21:16 [PATCH v2] selftests/vDSO: fix clang build errors and warnings John Hubbard
@ 2024-05-29  8:05 ` Muhammad Usama Anjum
  2024-05-30 19:24   ` John Hubbard
  2024-06-04 21:18 ` [v2] " Edward Liaw
  2024-06-11 18:30 ` [PATCH v2] " Carlos Llamas
  2 siblings, 1 reply; 8+ messages in thread
From: Muhammad Usama Anjum @ 2024-05-29  8:05 UTC (permalink / raw)
  To: John Hubbard, Shuah Khan
  Cc: Muhammad Usama Anjum, Mark Brown, Vincenzo Frascino,
	Colin Ian King, Valentin Obst, linux-kselftest, LKML, llvm

On 5/28/24 2:16 AM, John Hubbard wrote:
> When building with clang, via:
> 
>     make LLVM=1 -C tools/testing/selftests
> 
> ...there are several warnings, and an error. This fixes all of those and
> allows these tests to run and pass.
> 
> 1. Fix linker error (undefined reference to memcpy) by providing a local
>    version of memcpy.
> 
> 2. clang complains about using this form:
> 
>     if (g = h & 0xf0000000)
> 
> ...so factor out the assignment into a separate step.
> 
> 3. The code is passing a signed const char* to elf_hash(), which expects
>    a const unsigned char *. There are several callers, so fix this at
>    the source by allowing the function to accept a signed argument, and
>    then converting to unsigned operations, once inside the function.
> 
> 4. clang doesn't have __attribute__((externally_visible)) and generates
>    a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
>    to require that attribute in order to build, run and pass tests here,
>    so remove it.
Just checked with GCC 5.1, it builds fine without any errors.

> 
> [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/
> 
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
LGTM
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> ---
> 
> Changes since the first version:
> 
> 1) Rebased onto Linux 6.10-rc1
> 
> thanks,
> John Hubbard
> 
>  tools/testing/selftests/vDSO/parse_vdso.c      | 16 +++++++++++-----
>  .../selftests/vDSO/vdso_standalone_test_x86.c  | 18 ++++++++++++++++--
>  2 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c
> index 413f75620a35..4ae417372e9e 100644
> --- a/tools/testing/selftests/vDSO/parse_vdso.c
> +++ b/tools/testing/selftests/vDSO/parse_vdso.c
> @@ -55,14 +55,20 @@ static struct vdso_info
>  	ELF(Verdef) *verdef;
>  } vdso_info;
>  
> -/* Straight from the ELF specification. */
> -static unsigned long elf_hash(const unsigned char *name)
> +/*
> + * Straight from the ELF specification...and then tweaked slightly, in order to
> + * avoid a few clang warnings.
> + */
> +static unsigned long elf_hash(const char *name)
>  {
>  	unsigned long h = 0, g;
> -	while (*name)
> +	const unsigned char *uch_name = (const unsigned char *)name;
> +
> +	while (*uch_name)
>  	{
> -		h = (h << 4) + *name++;
> -		if (g = h & 0xf0000000)
> +		h = (h << 4) + *uch_name++;
> +		g = h & 0xf0000000;
> +		if (g)
>  			h ^= g >> 24;
>  		h &= ~g;
>  	}
> diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> index 8a44ff973ee1..27f6fdf11969 100644
> --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> +++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> @@ -18,7 +18,7 @@
>  
>  #include "parse_vdso.h"
>  
> -/* We need a libc functions... */
> +/* We need some libc functions... */
>  int strcmp(const char *a, const char *b)
>  {
>  	/* This implementation is buggy: it never returns -1. */
> @@ -34,6 +34,20 @@ int strcmp(const char *a, const char *b)
>  	return 0;
>  }
>  
> +/*
> + * The clang build needs this, although gcc does not.
> + * Stolen from lib/string.c.
> + */
> +void *memcpy(void *dest, const void *src, size_t count)
> +{
> +	char *tmp = dest;
> +	const char *s = src;
> +
> +	while (count--)
> +		*tmp++ = *s++;
> +	return dest;
> +}
> +
>  /* ...and two syscalls.  This is x86-specific. */
>  static inline long x86_syscall3(long nr, long a0, long a1, long a2)
>  {
> @@ -70,7 +84,7 @@ void to_base10(char *lastdig, time_t n)
>  	}
>  }
>  
> -__attribute__((externally_visible)) void c_main(void **stack)
> +void c_main(void **stack)
>  {
>  	/* Parse the stack */
>  	long argc = (long)*stack;
> 
> base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b

-- 
BR,
Muhammad Usama Anjum

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings
  2024-05-29  8:05 ` Muhammad Usama Anjum
@ 2024-05-30 19:24   ` John Hubbard
  0 siblings, 0 replies; 8+ messages in thread
From: John Hubbard @ 2024-05-30 19:24 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan, Andy Lutomirski
  Cc: Mark Brown, Vincenzo Frascino, Colin Ian King, Valentin Obst,
	linux-kselftest, LKML, llvm

On 5/29/24 1:05 AM, Muhammad Usama Anjum wrote:
> On 5/28/24 2:16 AM, John Hubbard wrote:
>> When building with clang, via:
>>
>>      make LLVM=1 -C tools/testing/selftests
>>
>> ...there are several warnings, and an error. This fixes all of those and
>> allows these tests to run and pass.
>>
>> 1. Fix linker error (undefined reference to memcpy) by providing a local
>>     version of memcpy.
>>
>> 2. clang complains about using this form:
>>
>>      if (g = h & 0xf0000000)
>>
>> ...so factor out the assignment into a separate step.
>>
>> 3. The code is passing a signed const char* to elf_hash(), which expects
>>     a const unsigned char *. There are several callers, so fix this at
>>     the source by allowing the function to accept a signed argument, and
>>     then converting to unsigned operations, once inside the function.
>>
>> 4. clang doesn't have __attribute__((externally_visible)) and generates
>>     a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
>>     to require that attribute in order to build, run and pass tests here,
>>     so remove it.
> Just checked with GCC 5.1, it builds fine without any errors.
> 
>>
>> [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/
>>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> LGTM
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> 

Thanks for the review and testing, Muhammad!

I'm also adding Andy Lutomirski to this thread, in case Shuah would like
the vDSO maintainer to ack or review. (scripts/get_maintainer.pl is letting
me down, I will need to run "git blame" more I guess.

thanks,
-- 
John Hubbard
NVIDIA

>> ---
>>
>> Changes since the first version:
>>
>> 1) Rebased onto Linux 6.10-rc1
>>
>> thanks,
>> John Hubbard
>>
>>   tools/testing/selftests/vDSO/parse_vdso.c      | 16 +++++++++++-----
>>   .../selftests/vDSO/vdso_standalone_test_x86.c  | 18 ++++++++++++++++--
>>   2 files changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c
>> index 413f75620a35..4ae417372e9e 100644
>> --- a/tools/testing/selftests/vDSO/parse_vdso.c
>> +++ b/tools/testing/selftests/vDSO/parse_vdso.c
>> @@ -55,14 +55,20 @@ static struct vdso_info
>>   	ELF(Verdef) *verdef;
>>   } vdso_info;
>>   
>> -/* Straight from the ELF specification. */
>> -static unsigned long elf_hash(const unsigned char *name)
>> +/*
>> + * Straight from the ELF specification...and then tweaked slightly, in order to
>> + * avoid a few clang warnings.
>> + */
>> +static unsigned long elf_hash(const char *name)
>>   {
>>   	unsigned long h = 0, g;
>> -	while (*name)
>> +	const unsigned char *uch_name = (const unsigned char *)name;
>> +
>> +	while (*uch_name)
>>   	{
>> -		h = (h << 4) + *name++;
>> -		if (g = h & 0xf0000000)
>> +		h = (h << 4) + *uch_name++;
>> +		g = h & 0xf0000000;
>> +		if (g)
>>   			h ^= g >> 24;
>>   		h &= ~g;
>>   	}
>> diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
>> index 8a44ff973ee1..27f6fdf11969 100644
>> --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
>> +++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
>> @@ -18,7 +18,7 @@
>>   
>>   #include "parse_vdso.h"
>>   
>> -/* We need a libc functions... */
>> +/* We need some libc functions... */
>>   int strcmp(const char *a, const char *b)
>>   {
>>   	/* This implementation is buggy: it never returns -1. */
>> @@ -34,6 +34,20 @@ int strcmp(const char *a, const char *b)
>>   	return 0;
>>   }
>>   
>> +/*
>> + * The clang build needs this, although gcc does not.
>> + * Stolen from lib/string.c.
>> + */
>> +void *memcpy(void *dest, const void *src, size_t count)
>> +{
>> +	char *tmp = dest;
>> +	const char *s = src;
>> +
>> +	while (count--)
>> +		*tmp++ = *s++;
>> +	return dest;
>> +}
>> +
>>   /* ...and two syscalls.  This is x86-specific. */
>>   static inline long x86_syscall3(long nr, long a0, long a1, long a2)
>>   {
>> @@ -70,7 +84,7 @@ void to_base10(char *lastdig, time_t n)
>>   	}
>>   }
>>   
>> -__attribute__((externally_visible)) void c_main(void **stack)
>> +void c_main(void **stack)
>>   {
>>   	/* Parse the stack */
>>   	long argc = (long)*stack;
>>
>> base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v2] selftests/vDSO: fix clang build errors and warnings
  2024-05-27 21:16 [PATCH v2] selftests/vDSO: fix clang build errors and warnings John Hubbard
  2024-05-29  8:05 ` Muhammad Usama Anjum
@ 2024-06-04 21:18 ` Edward Liaw
  2024-06-11 18:30 ` [PATCH v2] " Carlos Llamas
  2 siblings, 0 replies; 8+ messages in thread
From: Edward Liaw @ 2024-06-04 21:18 UTC (permalink / raw)
  To: jhubbard, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt
  Cc: Shuah Khan, Mark Brown, Vincenzo Frascino, Colin Ian King,
	Valentin Obst, linux-kselftest, LKML, llvm, Edward Liaw

> When building with clang, via:
>
>     make LLVM=1 -C tools/testing/selftests
>
> ...there are several warnings, and an error. This fixes all of those and
> allows these tests to run and pass.
>
> 1. Fix linker error (undefined reference to memcpy) by providing a local
>    version of memcpy.
>
> 2. clang complains about using this form:
>
>     if (g = h & 0xf0000000)
>
> ...so factor out the assignment into a separate step.
>
> 3. The code is passing a signed const char* to elf_hash(), which expects
>    a const unsigned char *. There are several callers, so fix this at
>    the source by allowing the function to accept a signed argument, and
>    then converting to unsigned operations, once inside the function.
>
> 4. clang doesn't have __attribute__((externally_visible)) and generates
>    a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
>    to require that attribute in order to build, run and pass tests here,
>    so remove it.
>
> [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>
> Changes since the first version:
>
> 1) Rebased onto Linux 6.10-rc1
>
> thanks,
> John Hubbard
>
>  tools/testing/selftests/vDSO/parse_vdso.c      | 16 +++++++++++-----
>  .../selftests/vDSO/vdso_standalone_test_x86.c  | 18 ++++++++++++++++--
>  2 files changed, 27 insertions(+), 7 deletions(-)
>
>
> base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b
> diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c
> index 413f75620a35..4ae417372e9e 100644
> --- a/tools/testing/selftests/vDSO/parse_vdso.c
> +++ b/tools/testing/selftests/vDSO/parse_vdso.c
> @@ -55,14 +55,20 @@ static struct vdso_info
>  	ELF(Verdef) *verdef;
>  } vdso_info;
>
> -/* Straight from the ELF specification. */
> -static unsigned long elf_hash(const unsigned char *name)
> +/*
> + * Straight from the ELF specification...and then tweaked slightly, in order to
> + * avoid a few clang warnings.
> + */
> +static unsigned long elf_hash(const char *name)
>  {
>  	unsigned long h = 0, g;
> -	while (*name)
> +	const unsigned char *uch_name = (const unsigned char *)name;
> +
> +	while (*uch_name)
>  	{
> -		h = (h << 4) + *name++;
> -		if (g = h & 0xf0000000)
> +		h = (h << 4) + *uch_name++;
> +		g = h & 0xf0000000;
> +		if (g)
>  			h ^= g >> 24;
>  		h &= ~g;
>  	}
> diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> index 8a44ff973ee1..27f6fdf11969 100644
> --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> +++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> @@ -18,7 +18,7 @@
>
>  #include "parse_vdso.h"
>
> -/* We need a libc functions... */
> +/* We need some libc functions... */
>  int strcmp(const char *a, const char *b)
>  {
>  	/* This implementation is buggy: it never returns -1. */
> @@ -34,6 +34,20 @@ int strcmp(const char *a, const char *b)
>  	return 0;
>  }
>
> +/*
> + * The clang build needs this, although gcc does not.
> + * Stolen from lib/string.c.
> + */
> +void *memcpy(void *dest, const void *src, size_t count)
> +{
> +	char *tmp = dest;
> +	const char *s = src;
> +
> +	while (count--)
> +		*tmp++ = *s++;
> +	return dest;
> +}
> +
>  /* ...and two syscalls.  This is x86-specific. */
>  static inline long x86_syscall3(long nr, long a0, long a1, long a2)
>  {
> @@ -70,7 +84,7 @@ void to_base10(char *lastdig, time_t n)
>  	}
>  }
>
> -__attribute__((externally_visible)) void c_main(void **stack)
> +void c_main(void **stack)
>  {
>  	/* Parse the stack */
>  	long argc = (long)*stack;
>

These work for me for compiling with clang.

Reviewed-by: Edward Liaw <edliaw@google.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings
  2024-05-27 21:16 [PATCH v2] selftests/vDSO: fix clang build errors and warnings John Hubbard
  2024-05-29  8:05 ` Muhammad Usama Anjum
  2024-06-04 21:18 ` [v2] " Edward Liaw
@ 2024-06-11 18:30 ` Carlos Llamas
  2024-06-14 22:51   ` John Hubbard
  2 siblings, 1 reply; 8+ messages in thread
From: Carlos Llamas @ 2024-06-11 18:30 UTC (permalink / raw)
  To: John Hubbard
  Cc: Shuah Khan, Mark Brown, Vincenzo Frascino, Colin Ian King,
	Valentin Obst, linux-kselftest, LKML, llvm

On Mon, May 27, 2024 at 02:16:22PM -0700, John Hubbard wrote:
> When building with clang, via:
> 
>     make LLVM=1 -C tools/testing/selftests
> 
> ...there are several warnings, and an error. This fixes all of those and
> allows these tests to run and pass.

It might be best to split the 4 _different_ fixes into separate patches.

> 
> 1. Fix linker error (undefined reference to memcpy) by providing a local
>    version of memcpy.
> 
> 2. clang complains about using this form:
> 
>     if (g = h & 0xf0000000)
> 
> ...so factor out the assignment into a separate step.

There has been multiple attempts to fix this. I can see these two:
https://lore.kernel.org/all/20211206102931.1433871-1-anders.roxell@linaro.org/
https://lore.kernel.org/all/20240501002150.1370861-1-edliaw@google.com/

... I guess we somehow missed those?

> 
> 3. The code is passing a signed const char* to elf_hash(), which expects
>    a const unsigned char *. There are several callers, so fix this at
>    the source by allowing the function to accept a signed argument, and
>    then converting to unsigned operations, once inside the function.
> 

There is also a v4 fix for this item that was sent out here:
https://lore.kernel.org/all/20240506181951.1804451-1-edliaw@google.com/

> 4. clang doesn't have __attribute__((externally_visible)) and generates
>    a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
>    to require that attribute in order to build, run and pass tests here,
>    so remove it.
> 
> [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/

What is this about? Left over from v1 maybe?

> 
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---

I would prefer to pick up the fixes from folks who sent out the patches
first but I'm fine either way.

Reviewed-by: Carlos Llamas <cmllamas@google.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings
  2024-06-11 18:30 ` [PATCH v2] " Carlos Llamas
@ 2024-06-14 22:51   ` John Hubbard
  2024-06-14 22:56     ` John Hubbard
  0 siblings, 1 reply; 8+ messages in thread
From: John Hubbard @ 2024-06-14 22:51 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Shuah Khan, Mark Brown, Vincenzo Frascino, Colin Ian King,
	Valentin Obst, linux-kselftest, LKML, llvm

On 6/11/24 11:30 AM, Carlos Llamas wrote:
> On Mon, May 27, 2024 at 02:16:22PM -0700, John Hubbard wrote:
>> When building with clang, via:
>>
>>      make LLVM=1 -C tools/testing/selftests
>>
>> ...there are several warnings, and an error. This fixes all of those and
>> allows these tests to run and pass.
> 
> It might be best to split the 4 _different_ fixes into separate patches.

If necessary, that can be done. It's sufficiently difficult to attract
attention for selftests (see below) that I'm reluctant to make it even
harder to get it all fixed, though.

> 
>>
>> 1. Fix linker error (undefined reference to memcpy) by providing a local
>>     version of memcpy.
>>
>> 2. clang complains about using this form:
>>
>>      if (g = h & 0xf0000000)
>>
>> ...so factor out the assignment into a separate step.
> 
> There has been multiple attempts to fix this. I can see these two:
> https://lore.kernel.org/all/20211206102931.1433871-1-anders.roxell@linaro.org/
> https://lore.kernel.org/all/20240501002150.1370861-1-edliaw@google.com/
> 
> ... I guess we somehow missed those?

I guess so. :)

> 
>>
>> 3. The code is passing a signed const char* to elf_hash(), which expects
>>     a const unsigned char *. There are several callers, so fix this at
>>     the source by allowing the function to accept a signed argument, and
>>     then converting to unsigned operations, once inside the function.
>>
> 
> There is also a v4 fix for this item that was sent out here:
> https://lore.kernel.org/all/20240506181951.1804451-1-edliaw@google.com/

No idea why these fixes are not getting picked up.

> 
>> 4. clang doesn't have __attribute__((externally_visible)) and generates
>>     a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
>>     to require that attribute in order to build, run and pass tests here,
>>     so remove it.
>>
>> [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/
> 
> What is this about? Left over from v1 maybe?

Maybe.

> 
>>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> ---
> 
> I would prefer to pick up the fixes from folks who sent out the patches
> first but I'm fine either way.

I'm perfectly fine with dropping this and letting the other patches go
in, but *something* needs to go in.

> 
> Reviewed-by: Carlos Llamas <cmllamas@google.com>

Thanks for the review! I have no earthly idea what will happen next.
I'd like to hear at least something from the maintainers about their
intentions here.

thanks,
-- 
John Hubbard
NVIDIA


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings
  2024-06-14 22:51   ` John Hubbard
@ 2024-06-14 22:56     ` John Hubbard
  2024-06-14 23:10       ` Carlos Llamas
  0 siblings, 1 reply; 8+ messages in thread
From: John Hubbard @ 2024-06-14 22:56 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Shuah Khan, Mark Brown, Vincenzo Frascino, Colin Ian King,
	Valentin Obst, linux-kselftest, LKML, llvm

On 6/14/24 3:51 PM, John Hubbard wrote:
> On 6/11/24 11:30 AM, Carlos Llamas wrote:
>> On Mon, May 27, 2024 at 02:16:22PM -0700, John Hubbard wrote:
...
>> Reviewed-by: Carlos Llamas <cmllamas@google.com>
> 
> Thanks for the review! I have no earthly idea what will happen next.
> I'd like to hear at least something from the maintainers about their
> intentions here.
> 

In fact, I have two more patches for vDSO (these are separate issues,
for the Makefile), and I just noticed that there are several accumulated
Reviewed-by and Tested-by tags on this patch here. So I think the way
forward is this:

I'll post a v3, with three patches for vDSO selftests, and the latest
tags. And let's see how that fares.

thanks,
-- 
John Hubbard
NVIDIA


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings
  2024-06-14 22:56     ` John Hubbard
@ 2024-06-14 23:10       ` Carlos Llamas
  0 siblings, 0 replies; 8+ messages in thread
From: Carlos Llamas @ 2024-06-14 23:10 UTC (permalink / raw)
  To: John Hubbard
  Cc: Shuah Khan, Mark Brown, Vincenzo Frascino, Colin Ian King,
	Valentin Obst, linux-kselftest, LKML, llvm, Andy Lutomirski,
	Thomas Gleixner

On Fri, Jun 14, 2024 at 03:56:01PM -0700, John Hubbard wrote:
> On 6/14/24 3:51 PM, John Hubbard wrote:
> > On 6/11/24 11:30 AM, Carlos Llamas wrote:
> > > On Mon, May 27, 2024 at 02:16:22PM -0700, John Hubbard wrote:
> ...
> > > Reviewed-by: Carlos Llamas <cmllamas@google.com>
> > 
> > Thanks for the review! I have no earthly idea what will happen next.
> > I'd like to hear at least something from the maintainers about their
> > intentions here.
> > 
> 
> In fact, I have two more patches for vDSO (these are separate issues,
> for the Makefile), and I just noticed that there are several accumulated
> Reviewed-by and Tested-by tags on this patch here. So I think the way
> forward is this:
> 
> I'll post a v3, with three patches for vDSO selftests, and the latest
> tags. And let's see how that fares.
> 

Yeap, a resend would have been good. This time it might be best to bring
Andy and Thomas in the loop too.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>

> thanks,
> -- 
> John Hubbard
> NVIDIA
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-06-14 23:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-27 21:16 [PATCH v2] selftests/vDSO: fix clang build errors and warnings John Hubbard
2024-05-29  8:05 ` Muhammad Usama Anjum
2024-05-30 19:24   ` John Hubbard
2024-06-04 21:18 ` [v2] " Edward Liaw
2024-06-11 18:30 ` [PATCH v2] " Carlos Llamas
2024-06-14 22:51   ` John Hubbard
2024-06-14 22:56     ` John Hubbard
2024-06-14 23:10       ` Carlos Llamas

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 NNTP newsgroup(s).