llvm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup
@ 2024-06-14 23:31 John Hubbard
  2024-06-14 23:31 ` [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings John Hubbard
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: John Hubbard @ 2024-06-14 23:31 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, John Hubbard

Hi,

Jason A. Donenfeld, I've added you because I ended up looking through
your latest "implement getrandom() in vDSO" series [1], which also
touches this Makefile, so just a heads up about upcoming (minor) merge
conflicts.

Changes since v2:

1. Added two patches, both of which apply solely to the Makefile.
These provide a smaller, cleaner, and more accurate Makefile.

2. Added Reviewed-by and Tested-by tags for the original patch, which
fixes all of the clang errors and warnings for this selftest.

3. Removed an obsolete blurb from the commit description of the original
patch, now that Valentin Obst LLVM build fix has been merged.


[1] https://lore.kernel.org/20240614190646.2081057-1-Jason@zx2c4.com


John Hubbard (3):
  selftests/vDSO: fix clang build errors and warnings
  selftests/mm: remove partially duplicated "all:" target in Makefile
  selftests/vDSO: remove duplicate compiler invocations from Makefile

 tools/testing/selftests/vDSO/Makefile         | 29 ++++++++-----------
 tools/testing/selftests/vDSO/parse_vdso.c     | 16 ++++++----
 .../selftests/vDSO/vdso_standalone_test_x86.c | 18 ++++++++++--
 3 files changed, 39 insertions(+), 24 deletions(-)


base-commit: 2ccbdf43d5e758f8493a95252073cf9078a5fea5
-- 
2.45.2


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

* [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings
  2024-06-14 23:31 [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
@ 2024-06-14 23:31 ` John Hubbard
  2024-06-14 23:38   ` John Hubbard
  2024-07-02 18:12   ` Edward Liaw
  2024-06-14 23:31 ` [PATCH v3 2/3] selftests/mm: remove partially duplicated "all:" target in Makefile John Hubbard
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: John Hubbard @ 2024-06-14 23:31 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, John Hubbard, Carlos Llamas, Edward Liaw,
	Muhammad Usama Anjum

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.

Reviewed-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Edward Liaw <edliaw@google.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 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;
-- 
2.45.2


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

* [PATCH v3 2/3] selftests/mm: remove partially duplicated "all:" target in Makefile
  2024-06-14 23:31 [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
  2024-06-14 23:31 ` [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings John Hubbard
@ 2024-06-14 23:31 ` John Hubbard
  2024-06-14 23:31 ` [PATCH v3 3/3] selftests/vDSO: remove duplicate compiler invocations from Makefile John Hubbard
  2024-06-14 23:34 ` [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
  3 siblings, 0 replies; 11+ messages in thread
From: John Hubbard @ 2024-06-14 23:31 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, John Hubbard

There were a couple of errors here:

1. TEST_GEN_PROGS was incorrectly prepending $(OUTPUT) to each program
to be built. However, lib.mk already does that because it assumes "bare"
program names are passed in, so this ended up creating
$(OUTPUT)/$(OUTPUT)/file.c, which of course won't work as intended.

2. lib.mk was included before TEST_GEN_PROGS was set, which led to
lib.mk's "all:" target not seeing anything to rebuild.

So nothing worked, which caused the author to force things by creating
an "all:" target locally--while still including ../lib.mk.

Fix all of this by including ../lib.mk at the right place, and removing
the $(OUTPUT) prefix to the programs to be built, and removing the
duplicate "all:" target.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/vDSO/Makefile | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index d53a4d8008f9..209ede5de208 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -1,16 +1,15 @@
 # SPDX-License-Identifier: GPL-2.0
-include ../lib.mk
-
 uname_M := $(shell uname -m 2>/dev/null || echo not)
 ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
 
-TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday $(OUTPUT)/vdso_test_getcpu
-TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi
-TEST_GEN_PROGS += $(OUTPUT)/vdso_test_clock_getres
+TEST_GEN_PROGS := vdso_test_gettimeofday
+TEST_GEN_PROGS += vdso_test_getcpu
+TEST_GEN_PROGS += vdso_test_abi
+TEST_GEN_PROGS += vdso_test_clock_getres
 ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
-TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
+TEST_GEN_PROGS += vdso_standalone_test_x86
 endif
-TEST_GEN_PROGS += $(OUTPUT)/vdso_test_correctness
+TEST_GEN_PROGS += vdso_test_correctness
 
 CFLAGS := -std=gnu99
 CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector
@@ -19,7 +18,7 @@ ifeq ($(CONFIG_X86_32),y)
 LDLIBS += -lgcc_s
 endif
 
-all: $(TEST_GEN_PROGS)
+include ../lib.mk
 $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c
 $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c
 $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c
-- 
2.45.2


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

* [PATCH v3 3/3] selftests/vDSO: remove duplicate compiler invocations from Makefile
  2024-06-14 23:31 [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
  2024-06-14 23:31 ` [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings John Hubbard
  2024-06-14 23:31 ` [PATCH v3 2/3] selftests/mm: remove partially duplicated "all:" target in Makefile John Hubbard
@ 2024-06-14 23:31 ` John Hubbard
  2024-06-14 23:34 ` [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
  3 siblings, 0 replies; 11+ messages in thread
From: John Hubbard @ 2024-06-14 23:31 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, John Hubbard

The Makefile open-codes compiler invocations that ../lib.mk already
provides.

Avoid this by using a Make feature that allows setting per-target
variables, which in this case are: CFLAGS and LDFLAGS. This approach
generates the exact same compiler invocations as before, but removes all
of the code duplication, along with the quirky mangled variable names.
So now the Makefile is smaller, less unusual, and easier to read.

The new dependencies are listed after including lib.mk, in order to
let lib.mk provide the first target ("all:"), and are grouped together
with their respective source file dependencies, for visual clarity.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/vDSO/Makefile | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 209ede5de208..98d8ba2afa00 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -12,8 +12,7 @@ endif
 TEST_GEN_PROGS += vdso_test_correctness
 
 CFLAGS := -std=gnu99
-CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector
-LDFLAGS_vdso_test_correctness := -ldl
+
 ifeq ($(CONFIG_X86_32),y)
 LDLIBS += -lgcc_s
 endif
@@ -23,12 +22,9 @@ $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c
 $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c
 $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c
 $(OUTPUT)/vdso_test_clock_getres: vdso_test_clock_getres.c
+
 $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c
-	$(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \
-		vdso_standalone_test_x86.c parse_vdso.c \
-		-o $@
+$(OUTPUT)/vdso_standalone_test_x86: CFLAGS +=-nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector
+
 $(OUTPUT)/vdso_test_correctness: vdso_test_correctness.c
-	$(CC) $(CFLAGS) \
-		vdso_test_correctness.c \
-		-o $@ \
-		$(LDFLAGS_vdso_test_correctness)
+$(OUTPUT)/vdso_test_correctness: LDFLAGS += -ldl
-- 
2.45.2


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

* Re: [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup
  2024-06-14 23:31 [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
                   ` (2 preceding siblings ...)
  2024-06-14 23:31 ` [PATCH v3 3/3] selftests/vDSO: remove duplicate compiler invocations from Makefile John Hubbard
@ 2024-06-14 23:34 ` John Hubbard
  2024-06-17 20:15   ` Shuah, Andy L: " John Hubbard
  3 siblings, 1 reply; 11+ messages in thread
From: John Hubbard @ 2024-06-14 23:34 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, Thomas Gleixner, Carlos Llamas

On 6/14/24 4:31 PM, John Hubbard wrote:
> Hi,
> 
> Jason A. Donenfeld, I've added you because I ended up looking through
> your latest "implement getrandom() in vDSO" series [1], which also
> touches this Makefile, so just a heads up about upcoming (minor) merge
> conflicts.

Belatedly adding Carlos Llamas and Thomas Gleixner to Cc.

> 
> Changes since v2:
> 
> 1. Added two patches, both of which apply solely to the Makefile.
> These provide a smaller, cleaner, and more accurate Makefile.
> 
> 2. Added Reviewed-by and Tested-by tags for the original patch, which
> fixes all of the clang errors and warnings for this selftest.
> 
> 3. Removed an obsolete blurb from the commit description of the original
> patch, now that Valentin Obst LLVM build fix has been merged.
> 
> 
> [1] https://lore.kernel.org/20240614190646.2081057-1-Jason@zx2c4.com
> 
> 
> John Hubbard (3):
>    selftests/vDSO: fix clang build errors and warnings
>    selftests/mm: remove partially duplicated "all:" target in Makefile
>    selftests/vDSO: remove duplicate compiler invocations from Makefile
> 
>   tools/testing/selftests/vDSO/Makefile         | 29 ++++++++-----------
>   tools/testing/selftests/vDSO/parse_vdso.c     | 16 ++++++----
>   .../selftests/vDSO/vdso_standalone_test_x86.c | 18 ++++++++++--
>   3 files changed, 39 insertions(+), 24 deletions(-)
> 
> 
> base-commit: 2ccbdf43d5e758f8493a95252073cf9078a5fea5

thanks,
-- 
John Hubbard
NVIDIA


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

* Re: [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings
  2024-06-14 23:31 ` [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings John Hubbard
@ 2024-06-14 23:38   ` John Hubbard
  2024-07-02 18:12   ` Edward Liaw
  1 sibling, 0 replies; 11+ messages in thread
From: John Hubbard @ 2024-06-14 23:38 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, Carlos Llamas, Edward Liaw, Muhammad Usama Anjum,
	Thomas Gleixner

Also Cc'ing Thomas Gleixner on this one, sorry for the omission.

On 6/14/24 4:31 PM, 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.
> 
> Reviewed-by: Carlos Llamas <cmllamas@google.com>
> Reviewed-by: Edward Liaw <edliaw@google.com>
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>   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;

thanks,
-- 
John Hubbard
NVIDIA


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

* Shuah, Andy L: [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup
  2024-06-14 23:34 ` [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
@ 2024-06-17 20:15   ` John Hubbard
  0 siblings, 0 replies; 11+ messages in thread
From: John Hubbard @ 2024-06-17 20:15 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, Thomas Gleixner, Carlos Llamas

On 6/14/24 4:34 PM, John Hubbard wrote:
> On 6/14/24 4:31 PM, John Hubbard wrote:
>> Hi,
>>
>> Jason A. Donenfeld, I've added you because I ended up looking through
>> your latest "implement getrandom() in vDSO" series [1], which also
>> touches this Makefile, so just a heads up about upcoming (minor) merge
>> conflicts.
> 
> Belatedly adding Carlos Llamas and Thomas Gleixner to Cc.
  
Hi Shuah, Andy, Thomas,

Patches 2 and 3 fix up the Makefile, and I'm particularly eager to get
those merged into 6.10, because the existing Makefile is leading people
astray. See the thread in [2] for an example of that.

As a result, Jason Donenfeld is now waiting for this, for his "implement
getrandom() in vDSO" series.

The Makefile is more of a kselftest infrastructure thing in a way, so
maybe Shuah can pull this in? I'm casting about for ways to move forward.

[2] https://lore.kernel.org/13483c92-cac5-4a3a-891f-22eb006c533b@nvidia.com


thanks,
-- 
John Hubbard
NVIDIA

>>
>> Changes since v2:
>>
>> 1. Added two patches, both of which apply solely to the Makefile.
>> These provide a smaller, cleaner, and more accurate Makefile.
>>
>> 2. Added Reviewed-by and Tested-by tags for the original patch, which
>> fixes all of the clang errors and warnings for this selftest.
>>
>> 3. Removed an obsolete blurb from the commit description of the original
>> patch, now that Valentin Obst LLVM build fix has been merged.
>>
>>
>> [1] https://lore.kernel.org/20240614190646.2081057-1-Jason@zx2c4.com
>>
>>
>> John Hubbard (3):
>>    selftests/vDSO: fix clang build errors and warnings
>>    selftests/mm: remove partially duplicated "all:" target in Makefile
>>    selftests/vDSO: remove duplicate compiler invocations from Makefile
>>
>>   tools/testing/selftests/vDSO/Makefile         | 29 ++++++++-----------
>>   tools/testing/selftests/vDSO/parse_vdso.c     | 16 ++++++----
>>   .../selftests/vDSO/vdso_standalone_test_x86.c | 18 ++++++++++--
>>   3 files changed, 39 insertions(+), 24 deletions(-)
>>
>>
>> base-commit: 2ccbdf43d5e758f8493a95252073cf9078a5fea5
> 
> thanks,



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

* Re: [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings
  2024-06-14 23:31 ` [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings John Hubbard
  2024-06-14 23:38   ` John Hubbard
@ 2024-07-02 18:12   ` Edward Liaw
  2024-07-02 18:25     ` John Hubbard
  1 sibling, 1 reply; 11+ messages in thread
From: Edward Liaw @ 2024-07-02 18:12 UTC (permalink / raw)
  To: John Hubbard
  Cc: Shuah Khan, Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, Carlos Llamas, Muhammad Usama Anjum

On Fri, Jun 14, 2024 at 4:31 PM John Hubbard <jhubbard@nvidia.com> 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.
>
> Reviewed-by: Carlos Llamas <cmllamas@google.com>
> Reviewed-by: Edward Liaw <edliaw@google.com>
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  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;
> --
> 2.45.2
>

Hi John,
Could you try re-submitting this series with the RESEND prefix?

Thanks,
Edward

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

* Re: [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings
  2024-07-02 18:12   ` Edward Liaw
@ 2024-07-02 18:25     ` John Hubbard
  2024-07-02 18:49       ` Edward Liaw
  0 siblings, 1 reply; 11+ messages in thread
From: John Hubbard @ 2024-07-02 18:25 UTC (permalink / raw)
  To: Edward Liaw
  Cc: Shuah Khan, Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, Carlos Llamas, Muhammad Usama Anjum

On 7/2/24 11:12 AM, Edward Liaw wrote:
> On Fri, Jun 14, 2024 at 4:31 PM John Hubbard <jhubbard@nvidia.com> wrote:
...
> Hi John,
> Could you try re-submitting this series with the RESEND prefix?
> 
> Thanks,
> Edward

Sure. Is that the key, for kselftests? Because I've got 5+ small
patchsets that are languishing there, and I'm at a loss for how to even
get a response, much less get something merged.


thanks,
-- 
John Hubbard
NVIDIA


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

* Re: [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings
  2024-07-02 18:25     ` John Hubbard
@ 2024-07-02 18:49       ` Edward Liaw
  2024-07-02 19:24         ` John Hubbard
  0 siblings, 1 reply; 11+ messages in thread
From: Edward Liaw @ 2024-07-02 18:49 UTC (permalink / raw)
  To: John Hubbard
  Cc: Shuah Khan, Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, Carlos Llamas, Muhammad Usama Anjum

On Tue, Jul 2, 2024 at 11:26 AM John Hubbard <jhubbard@nvidia.com> wrote:
>
> On 7/2/24 11:12 AM, Edward Liaw wrote:
> > On Fri, Jun 14, 2024 at 4:31 PM John Hubbard <jhubbard@nvidia.com> wrote:
> ...
> > Hi John,
> > Could you try re-submitting this series with the RESEND prefix?
> >
> > Thanks,
> > Edward
>
> Sure. Is that the key, for kselftests? Because I've got 5+ small
> patchsets that are languishing there, and I'm at a loss for how to even
> get a response, much less get something merged.

I don't really know, but it was suggested to me that it can help to
point out that the patch might have been missed before.

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

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

* Re: [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings
  2024-07-02 18:49       ` Edward Liaw
@ 2024-07-02 19:24         ` John Hubbard
  0 siblings, 0 replies; 11+ messages in thread
From: John Hubbard @ 2024-07-02 19:24 UTC (permalink / raw)
  To: Edward Liaw
  Cc: Shuah Khan, Jason A . Donenfeld, Andy Lutomirski, Mark Brown,
	Vincenzo Frascino, Colin Ian King, Valentin Obst, linux-kselftest,
	LKML, llvm, Carlos Llamas, Muhammad Usama Anjum

On 7/2/24 11:49 AM, Edward Liaw wrote:
> On Tue, Jul 2, 2024 at 11:26 AM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> On 7/2/24 11:12 AM, Edward Liaw wrote:
>>> On Fri, Jun 14, 2024 at 4:31 PM John Hubbard <jhubbard@nvidia.com> wrote:
>> ...
>>> Hi John,
>>> Could you try re-submitting this series with the RESEND prefix?
>>>
>>> Thanks,
>>> Edward
>>
>> Sure. Is that the key, for kselftests? Because I've got 5+ small
>> patchsets that are languishing there, and I'm at a loss for how to even
>> get a response, much less get something merged.
> 
> I don't really know, but it was suggested to me that it can help to
> point out that the patch might have been missed before.
> 

I wish. But we are three versions into it, plus a special email [1]
specifically to point out that this is still pending.

I think either people are on vacation, or these fixes are not considered
sufficiently important, or both. :)


[1] https://lore.kernel.org/e9de90dd-c541-45bf-a364-0e7aaf320314@nvidia.com

thanks,
-- 
John Hubbard
NVIDIA


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

end of thread, other threads:[~2024-07-02 19:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-14 23:31 [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
2024-06-14 23:31 ` [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings John Hubbard
2024-06-14 23:38   ` John Hubbard
2024-07-02 18:12   ` Edward Liaw
2024-07-02 18:25     ` John Hubbard
2024-07-02 18:49       ` Edward Liaw
2024-07-02 19:24         ` John Hubbard
2024-06-14 23:31 ` [PATCH v3 2/3] selftests/mm: remove partially duplicated "all:" target in Makefile John Hubbard
2024-06-14 23:31 ` [PATCH v3 3/3] selftests/vDSO: remove duplicate compiler invocations from Makefile John Hubbard
2024-06-14 23:34 ` [PATCH v3 0/3] selftests/vDSO: fix clang build errors, and Makefile cleanup John Hubbard
2024-06-17 20:15   ` Shuah, Andy L: " John Hubbard

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).