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/[email protected]
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
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 <[email protected]>
Reviewed-by: Edward Liaw <[email protected]>
Reviewed-by: Muhammad Usama Anjum <[email protected]>
Tested-by: Muhammad Usama Anjum <[email protected]>
Signed-off-by: John Hubbard <[email protected]>
---
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
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 <[email protected]>
> Reviewed-by: Edward Liaw <[email protected]>
> Reviewed-by: Muhammad Usama Anjum <[email protected]>
> Tested-by: Muhammad Usama Anjum <[email protected]>
> Signed-off-by: John Hubbard <[email protected]>
> ---
> 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
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/[email protected]
>
>
> 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