On Fri, Mar 28, 2025 at 05:04:54PM -0600, Shuah Khan wrote:
On 1/15/25 16:37, Dmitry V. Levin wrote:
MIPS n32 is one of two ILP32 architectures supported by the kernel that have 64-bit syscall arguments (another one is x32).
When this test passed 32-bit arguments to syscall(), they were sign-extended in libc, PTRACE_GET_SYSCALL_INFO reported these sign-extended 64-bit values, and the test complained about the mismatch.
Fix this by passing arguments of the appropriate type to syscall(), which is "unsigned long long" on MIPS n32, and __kernel_ulong_t on other architectures.
As a side effect, this also extends the test on all 64-bit architectures by choosing constants that don't fit into 32-bit integers.
Signed-off-by: Dmitry V. Levin ldv@strace.io
v2: Fixed MIPS #ifdef.
.../selftests/ptrace/get_syscall_info.c | 53 +++++++++++-------- 1 file changed, 32 insertions(+), 21 deletions(-)
diff --git a/tools/testing/selftests/ptrace/get_syscall_info.c b/tools/testing/selftests/ptrace/get_syscall_info.c index 5bcd1c7b5be6..2970f72d66d3 100644 --- a/tools/testing/selftests/ptrace/get_syscall_info.c +++ b/tools/testing/selftests/ptrace/get_syscall_info.c @@ -11,8 +11,19 @@ #include <err.h> #include <signal.h> #include <asm/unistd.h> +#include <linux/types.h> #include "linux/ptrace.h" +#if defined(_MIPS_SIM) && _MIPS_SIM == _MIPS_SIM_NABI32 +/*
- MIPS N32 is the only architecture where __kernel_ulong_t
- does not match the bitness of syscall arguments.
- */
+typedef unsigned long long kernel_ulong_t; +#else +typedef __kernel_ulong_t kernel_ulong_t; +#endif
What's the reason for adding these typedefs? checkpatch should have warned you about adding new typedefs.
Also this introduces kernel_ulong_t in user-space test code. Something to avoid.
There has to be a new type for this test, and the natural way to do this is to use typedef. The alternative would be to #define kernel_ulong_t which is ugly. By the way, there are quite a few typedefs in selftests, and there seems to be given no rationale why adding new types in selftests is a bad idea.
That is, the new type in this test is being added on purpose, and I'd rather keep it this way.
static int kill_tracee(pid_t pid) { @@ -42,37 +53,37 @@ sys_ptrace(int request, pid_t pid, unsigned long addr, unsigned long data) TEST(get_syscall_info) {
- static const unsigned long args[][7] = {
- const kernel_ulong_t args[][7] = { /* a sequence of architecture-agnostic syscalls */ { __NR_chdir,
(unsigned long) "",0xbad1fed1,0xbad2fed2,0xbad3fed3,0xbad4fed4,0xbad5fed5
(uintptr_t) "",You could use ifdef here.
Not just here but in other cases as well. I think this would make the code less readable. I'd prefer a single ifdef with a single explanatory comment.