Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx/

Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
LGTM
Reviewed-by: Muhammad Usama Anjum <usama.anjum@xxxxxxxxxxxxx>
Tested-by: Muhammad Usama Anjum <usama.anjum@xxxxxxxxxxxxx>


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







[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux