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-2f9ed7d1c...
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
On 5/28/24 2:16 AM, John Hubbard wrote:
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-2f9ed7d1c...
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
On 5/29/24 1:05 AM, Muhammad Usama Anjum wrote:
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,
On Mon, May 27, 2024 at 02:16:22PM -0700, John Hubbard wrote:
It might be best to split the 4 _different_ fixes into separate patches.
There has been multiple attempts to fix this. I can see these two: https://lore.kernel.org/all/20211206102931.1433871-1-anders.roxell@linaro.or... https://lore.kernel.org/all/20240501002150.1370861-1-edliaw@google.com/
... I guess we somehow missed those?
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/
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
On 6/11/24 11:30 AM, Carlos Llamas wrote:
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.
I guess so. :)
No idea why these fixes are not getting picked up.
Maybe.
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,
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:
...
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,
On Fri, Jun 14, 2024 at 03:56:01PM -0700, John Hubbard wrote:
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
linux-kselftest-mirror@lists.linaro.org