2024-05-27 21:01:20

by John Hubbard

[permalink] [raw]
Subject: [PATCH v2 0/6] selftests/x86: fix build errors and warnings found via clang

Hi,

Just a bunch of build and warnings fixes that show up when
building with clang. Some of these depend on each other, so
I'm sending them as a series.

Changes since the first version:

1) Rebased onto Linux 6.10-rc1

Enjoy!

thanks,
John Hubbard

John Hubbard (6):
selftests/x86: build test_FISTTP.c with clang
selftests/x86: build fsgsbase_restore.c with clang
selftests/x86: build sysret_rip.c with clang
selftests/x86: avoid -no-pie warnings from clang during compilation
selftests/x86: remove (or use) unused variables and functions
selftests/x86: fix printk warnings reported by clang

tools/testing/selftests/x86/Makefile | 10 +++++++
tools/testing/selftests/x86/amx.c | 16 -----------
.../testing/selftests/x86/clang_helpers_32.S | 11 ++++++++
.../testing/selftests/x86/clang_helpers_64.S | 28 +++++++++++++++++++
tools/testing/selftests/x86/fsgsbase.c | 6 ----
.../testing/selftests/x86/fsgsbase_restore.c | 11 ++++----
tools/testing/selftests/x86/sigreturn.c | 2 +-
.../testing/selftests/x86/syscall_arg_fault.c | 1 -
tools/testing/selftests/x86/sysret_rip.c | 20 ++++---------
tools/testing/selftests/x86/test_FISTTP.c | 8 +++---
tools/testing/selftests/x86/test_vsyscall.c | 15 ++++------
tools/testing/selftests/x86/vdso_restorer.c | 2 ++
12 files changed, 72 insertions(+), 58 deletions(-)
create mode 100644 tools/testing/selftests/x86/clang_helpers_32.S
create mode 100644 tools/testing/selftests/x86/clang_helpers_64.S


base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b
prerequisite-patch-id: 39d606b9b165077aa1a3a3b0a3b396dba0c20070
--
2.45.1



2024-05-27 21:02:44

by John Hubbard

[permalink] [raw]
Subject: [PATCH v2 6/6] selftests/x86: fix printk warnings reported by clang

These warnings are all of the form, "the format specified a short
(signed or unsigned) int, but the value is a full length int".

Signed-off-by: John Hubbard <[email protected]>
---
tools/testing/selftests/x86/sigreturn.c | 2 +-
tools/testing/selftests/x86/test_vsyscall.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c
index 5d7961a5f7f6..0b75b29f794b 100644
--- a/tools/testing/selftests/x86/sigreturn.c
+++ b/tools/testing/selftests/x86/sigreturn.c
@@ -487,7 +487,7 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
greg_t asm_ss = ctx->uc_mcontext.gregs[REG_CX];
if (asm_ss != sig_ss && sig == SIGTRAP) {
/* Sanity check failure. */
- printf("[FAIL]\tSIGTRAP: ss = %hx, frame ss = %hx, ax = %llx\n",
+ printf("[FAIL]\tSIGTRAP: ss = %hx, frame ss = %x, ax = %llx\n",
ss, *ssptr(ctx), (unsigned long long)asm_ss);
nerrs++;
}
diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c
index 1c9895cfc660..6de11b4df458 100644
--- a/tools/testing/selftests/x86/test_vsyscall.c
+++ b/tools/testing/selftests/x86/test_vsyscall.c
@@ -247,7 +247,7 @@ static void test_getcpu(int cpu)

if (ret_sys == 0) {
if (cpu_sys != cpu)
- ksft_print_msg("syscall reported CPU %hu but should be %d\n",
+ ksft_print_msg("syscall reported CPU %u but should be %d\n",
cpu_sys, cpu);

have_node = true;
@@ -265,10 +265,10 @@ static void test_getcpu(int cpu)

if (cpu_vdso != cpu || node_vdso != node) {
if (cpu_vdso != cpu)
- ksft_print_msg("vDSO reported CPU %hu but should be %d\n",
+ ksft_print_msg("vDSO reported CPU %u but should be %d\n",
cpu_vdso, cpu);
if (node_vdso != node)
- ksft_print_msg("vDSO reported node %hu but should be %hu\n",
+ ksft_print_msg("vDSO reported node %u but should be %u\n",
node_vdso, node);
ksft_test_result_fail("Wrong values\n");
} else {
@@ -290,10 +290,10 @@ static void test_getcpu(int cpu)

if (cpu_vsys != cpu || node_vsys != node) {
if (cpu_vsys != cpu)
- ksft_print_msg("vsyscall reported CPU %hu but should be %d\n",
+ ksft_print_msg("vsyscall reported CPU %u but should be %d\n",
cpu_vsys, cpu);
if (node_vsys != node)
- ksft_print_msg("vsyscall reported node %hu but should be %hu\n",
+ ksft_print_msg("vsyscall reported node %u but should be %u\n",
node_vsys, node);
ksft_test_result_fail("Wrong values\n");
} else {
--
2.45.1


2024-05-27 21:03:02

by John Hubbard

[permalink] [raw]
Subject: [PATCH v2 2/6] selftests/x86: build fsgsbase_restore.c with clang

First of all, in order to build with clang at all, one must first apply
Valentin Obst's build fix for LLVM [1]. Once that is done, then when
building with clang, via:

make LLVM=1 -C tools/testing/selftests

Fix this by moving the inline asm to "pure" assembly, in two new files:
clang_helpers_32.S, clang_helpers_64.S.

As a bonus, the pure asm avoids the need for ifdefs, and is now very
simple and easy on the eyes.

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

Signed-off-by: John Hubbard <[email protected]>
---
tools/testing/selftests/x86/Makefile | 2 ++
tools/testing/selftests/x86/clang_helpers_32.S | 11 +++++++++++
tools/testing/selftests/x86/clang_helpers_64.S | 12 ++++++++++++
tools/testing/selftests/x86/fsgsbase_restore.c | 11 +++++------
4 files changed, 30 insertions(+), 6 deletions(-)
create mode 100644 tools/testing/selftests/x86/clang_helpers_32.S
create mode 100644 tools/testing/selftests/x86/clang_helpers_64.S

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index c1269466e0f8..99bc2ef84f5a 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -113,6 +113,8 @@ endef
$(eval $(call extra-files,sysret_ss_attrs_64,thunks.S))
$(eval $(call extra-files,ptrace_syscall_32,raw_syscall_helper_32.S))
$(eval $(call extra-files,test_syscall_vdso_32,thunks_32.S))
+$(eval $(call extra-files,fsgsbase_restore_64,clang_helpers_64.S))
+$(eval $(call extra-files,fsgsbase_restore_32,clang_helpers_32.S))

# check_initial_reg_state is special: it needs a custom entry, and it
# needs to be static so that its interpreter doesn't destroy its initial
diff --git a/tools/testing/selftests/x86/clang_helpers_32.S b/tools/testing/selftests/x86/clang_helpers_32.S
new file mode 100644
index 000000000000..dc16271bac70
--- /dev/null
+++ b/tools/testing/selftests/x86/clang_helpers_32.S
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * 32-bit assembly helpers for asm operations that lack support in both gcc and
+ * clang. For example, clang asm does not support segment prefixes.
+ */
+.global dereference_seg_base
+dereference_seg_base:
+ mov %fs:(0), %eax
+ ret
+
+.section .note.GNU-stack,"",%progbits
diff --git a/tools/testing/selftests/x86/clang_helpers_64.S b/tools/testing/selftests/x86/clang_helpers_64.S
new file mode 100644
index 000000000000..0d81c71cad97
--- /dev/null
+++ b/tools/testing/selftests/x86/clang_helpers_64.S
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * 64-bit assembly helpers for asm operations that lack support in both gcc and
+ * clang. For example, clang asm does not support segment prefixes.
+ */
+.global dereference_seg_base
+
+dereference_seg_base:
+ mov %gs:(0), %rax
+ ret
+
+.section .note.GNU-stack,"",%progbits
diff --git a/tools/testing/selftests/x86/fsgsbase_restore.c b/tools/testing/selftests/x86/fsgsbase_restore.c
index 6fffadc51579..224058c1e4b2 100644
--- a/tools/testing/selftests/x86/fsgsbase_restore.c
+++ b/tools/testing/selftests/x86/fsgsbase_restore.c
@@ -39,12 +39,11 @@
# define SEG "%fs"
#endif

-static unsigned int dereference_seg_base(void)
-{
- int ret;
- asm volatile ("mov %" SEG ":(0), %0" : "=rm" (ret));
- return ret;
-}
+/*
+ * Defined in clang_helpers_[32|64].S, because unlike gcc, clang inline asm does
+ * not support segmentation prefixes.
+ */
+unsigned int dereference_seg_base(void);

static void init_seg(void)
{
--
2.45.1


2024-05-30 14:58:25

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] selftests/x86: fix build errors and warnings found via clang

On 5/27/24 15:00, John Hubbard wrote:
> Hi,
>
> Just a bunch of build and warnings fixes that show up when
> building with clang. Some of these depend on each other, so
> I'm sending them as a series.
>
> Changes since the first version:
>
> 1) Rebased onto Linux 6.10-rc1

x86 test patches usually go through x86 tree.

This series requires x86 maintainer review and ack for me
to take this through kselftest tree.


>
> Enjoy!
>
> thanks,
> John Hubbard
>
> John Hubbard (6):
> selftests/x86: build test_FISTTP.c with clang
> selftests/x86: build fsgsbase_restore.c with clang
> selftests/x86: build sysret_rip.c with clang
> selftests/x86: avoid -no-pie warnings from clang during compilation
> selftests/x86: remove (or use) unused variables and functions
> selftests/x86: fix printk warnings reported by clang
>
> tools/testing/selftests/x86/Makefile | 10 +++++++
> tools/testing/selftests/x86/amx.c | 16 -----------
> .../testing/selftests/x86/clang_helpers_32.S | 11 ++++++++
> .../testing/selftests/x86/clang_helpers_64.S | 28 +++++++++++++++++++
> tools/testing/selftests/x86/fsgsbase.c | 6 ----
> .../testing/selftests/x86/fsgsbase_restore.c | 11 ++++----
> tools/testing/selftests/x86/sigreturn.c | 2 +-
> .../testing/selftests/x86/syscall_arg_fault.c | 1 -
> tools/testing/selftests/x86/sysret_rip.c | 20 ++++---------
> tools/testing/selftests/x86/test_FISTTP.c | 8 +++---
> tools/testing/selftests/x86/test_vsyscall.c | 15 ++++------
> tools/testing/selftests/x86/vdso_restorer.c | 2 ++
> 12 files changed, 72 insertions(+), 58 deletions(-)
> create mode 100644 tools/testing/selftests/x86/clang_helpers_32.S
> create mode 100644 tools/testing/selftests/x86/clang_helpers_64.S
>
>
> base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b
> prerequisite-patch-id: 39d606b9b165077aa1a3a3b0a3b396dba0c20070

thanks,
-- Shuah


2024-05-30 19:46:30

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] selftests/x86: fix build errors and warnings found via clang

On 5/27/24 14:00, John Hubbard wrote:
> John Hubbard (6):
> selftests/x86: build test_FISTTP.c with clang
> selftests/x86: build fsgsbase_restore.c with clang
> selftests/x86: build sysret_rip.c with clang
> selftests/x86: avoid -no-pie warnings from clang during compilation
> selftests/x86: remove (or use) unused variables and functions
> selftests/x86: fix printk warnings reported by clang

John, could you and Muhammad have a chat and perhaps settle on a series
series that gets acks from both of you?

> https://lore.kernel.org/all/[email protected]/

I had Muhammad's in my queue and didn't realize we had two overlapping
series' bouncing around until now.

2024-05-30 20:02:01

by John Hubbard

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] selftests/x86: fix build errors and warnings found via clang

On 5/30/24 12:46 PM, Dave Hansen wrote:
> On 5/27/24 14:00, John Hubbard wrote:
>> John Hubbard (6):
>> selftests/x86: build test_FISTTP.c with clang
>> selftests/x86: build fsgsbase_restore.c with clang
>> selftests/x86: build sysret_rip.c with clang
>> selftests/x86: avoid -no-pie warnings from clang during compilation
>> selftests/x86: remove (or use) unused variables and functions
>> selftests/x86: fix printk warnings reported by clang
>
> John, could you and Muhammad have a chat and perhaps settle on a series
> series that gets acks from both of you?
>
>> https://lore.kernel.org/all/[email protected]/
>
> I had Muhammad's in my queue and didn't realize we had two overlapping
> series' bouncing around until now.

Aha OK. Muhummad, after looking through this, I see that our
test_FISTTP.c fix is identical, and that's about it. My series goes
a bit deeper IMHO and completely fixes all the errors; the tradeoff
is that it is more intrusive. Which I think is appropriate.

Would you be OK with my posting v3 that uses your patch for
test_FISTTP.c [1], and the rest of my patches for the rest?


[1]
https://lore.kernel.org/all/[email protected]/

thanks,
--
John Hubbard
NVIDIA


2024-05-31 05:13:19

by Muhammad Usama Anjum

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] selftests/x86: fix build errors and warnings found via clang

On 5/31/24 1:00 AM, John Hubbard wrote:
> On 5/30/24 12:46 PM, Dave Hansen wrote:
>> On 5/27/24 14:00, John Hubbard wrote:
>>> John Hubbard (6):
>>>    selftests/x86: build test_FISTTP.c with clang
>>>    selftests/x86: build fsgsbase_restore.c with clang
>>>    selftests/x86: build sysret_rip.c with clang
>>>    selftests/x86: avoid -no-pie warnings from clang during compilation
>>>    selftests/x86: remove (or use) unused variables and functions
>>>    selftests/x86: fix printk warnings reported by clang
>>
>> John, could you and Muhammad have a chat and perhaps settle on a series
>> series that gets acks from both of you?
>>
>>> https://lore.kernel.org/all/[email protected]/
>>
>> I had Muhammad's in my queue and didn't realize we had two overlapping
>> series' bouncing around until now.
>
> Aha OK. Muhummad, after looking through this, I see that our
> test_FISTTP.c fix is identical, and that's about it. My series goes
> a bit deeper IMHO and completely fixes all the errors; the tradeoff
> is that it is more intrusive. Which I think is appropriate.
>
> Would you be OK with my posting v3 that uses your patch for
> test_FISTTP.c [1], and the rest of my patches for the rest?
Yeah, sure go ahead. I'll test/review the v3 series.

>
>
> [1]
> https://lore.kernel.org/all/[email protected]/
>
> thanks,

--
BR,
Muhammad Usama Anjum