2011-06-21 19:10:21

by Hartley Sweeten

[permalink] [raw]
Subject: [PATCH] ARM: quiet sparse noise due to __ARCH_WANT_SYS_RT_SIG(ACTION|SUSPEND)

ARM defines __ARCH_WANT_SYS_RT_SIG(ACTION|SUSPEND) which
produces the following sparse warnings in kernel/signal.c:

warning: symbol 'sys_rt_sigaction' was not declared. Should it be static?
warning: symbol 'sys_rt_sigsuspend' was not declared. Should it be static?

Since ARM doesn't include <asm-generic/syscalls.h>, due to different
calling conventions for some system calls, prototype the functions
in <asm/unistd.h> to quiet the noise.

Signed-off-by: H Hartley Sweeten <[email protected]>
Cc: Russell King <[email protected]>
Cc: Mikael Pettersson <[email protected]>
Cc: "Eric W. Biederman" <[email protected]>
Cc: Mike Frysinger <[email protected]>
Cc: Tony Luck <[email protected]>

---

diff --git a/arch/arm/include/asm/unistd.h b/arch/arm/include/asm/unistd.h
index 2c04ed5..322c54e 100644
--- a/arch/arm/include/asm/unistd.h
+++ b/arch/arm/include/asm/unistd.h
@@ -467,6 +467,20 @@
#define __ARCH_WANT_SYS_SOCKETCALL
#endif

+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+#include <linux/linkage.h>
+#include <linux/compiler.h>
+#include <linux/signal.h>
+
+asmlinkage long sys_rt_sigaction(int sig, const struct sigaction __user *act,
+ struct sigaction __user *oact,
+ size_t sigsetsize);
+asmlinkage long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize);
+
+#endif /* !__ASSEMBLY__ */
+
/*
* "Conditional" syscalls
*


2011-06-21 19:16:38

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] ARM: quiet sparse noise due to __ARCH_WANT_SYS_RT_SIG(ACTION|SUSPEND)

On Tue, Jun 21, 2011 at 12:09:45PM -0700, H Hartley Sweeten wrote:
> ARM defines __ARCH_WANT_SYS_RT_SIG(ACTION|SUSPEND) which
> produces the following sparse warnings in kernel/signal.c:
>
> warning: symbol 'sys_rt_sigaction' was not declared. Should it be static?
> warning: symbol 'sys_rt_sigsuspend' was not declared. Should it be static?
>
> Since ARM doesn't include <asm-generic/syscalls.h>, due to different
> calling conventions for some system calls, prototype the functions
> in <asm/unistd.h> to quiet the noise.

NAK.

asm/unistd.h is an exported header. Please don't pollute it with kernel
internal stuff. Instead, follow the hint in asm-generic and create a new
header file to describe syscalls called... asm/syscalls.h.

And.. the problem is a lot larger than you mention above:

arch/arm/kernel/signal.c:68:16: warning: symbol 'sys_sigsuspend' was not declared. Should it be static?
arch/arm/kernel/signal.c:84:1: warning: symbol 'sys_sigaction' was not declared. Should it be static?
arch/arm/kernel/signal.c:328:16: warning: symbol 'sys_sigreturn' was not declared. Should it be static?
arch/arm/kernel/signal.c:360:16: warning: symbol 'sys_rt_sigreturn' was not declared. Should it be static?
arch/arm/kernel/sys_arm.c:34:16: warning: symbol 'sys_fork' was not declared. Should it be static?
arch/arm/kernel/sys_arm.c:47:16: warning: symbol 'sys_clone' was not declared. Should it be static?
arch/arm/kernel/sys_arm.c:57:16: warning: symbol 'sys_vfork' was not declared. Should it be static?
arch/arm/kernel/sys_arm.c:65:16: warning: symbol 'sys_execve' was not declared. Should it be static?
arch/arm/kernel/sys_arm.c:129:17: warning: symbol 'sys_arm_fadvise64_64' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:144:17: warning: symbol 'sys_oabi_stat64' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:154:17: warning: symbol 'sys_oabi_lstat64' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:164:17: warning: symbol 'sys_oabi_fstat64' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:174:17: warning: symbol 'sys_oabi_fstatat64' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:196:17: warning: symbol 'sys_oabi_fcntl64' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:249:17: warning: symbol 'sys_oabi_epoll_ctl' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:270:17: warning: symbol 'sys_oabi_epoll_wait' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:304:17: warning: symbol 'sys_oabi_semtimedop' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:343:17: warning: symbol 'sys_oabi_semop' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:349:16: warning: symbol 'sys_oabi_ipc' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:367:17: warning: symbol 'sys_oabi_bind' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:377:17: warning: symbol 'sys_oabi_connect' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:387:17: warning: symbol 'sys_oabi_sendto' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:400:17: warning: symbol 'sys_oabi_sendmsg' was not declared. Should it be static?
arch/arm/kernel/sys_oabi-compat.c:426:17: warning: symbol 'sys_oabi_socketcall' was not declared. Should it be static?
arch/arm/kernel/traps.c:464:16: warning: symbol 'arm_syscall' was not declared. Should it be static?

So you actually need something like this (pasted, so whitespace
damaged). I'll pull it out of my low priority queue and queue it up
for the next merge window.

diff --git a/arch/arm/include/asm/syscalls.h b/arch/arm/include/asm/syscalls.h
new file mode 100644
index 0000000..e775746
--- /dev/null
+++ b/arch/arm/include/asm/syscalls.h
@@ -0,0 +1,60 @@
+#ifndef _ASM_ARM_SYSCALLS_H
+#define _ASM_ARM_SYSCALLS_H
+
+#include <linux/linkage.h>
+#include <linux/types.h>
+
+struct msghdr;
+struct sockaddr;
+
+/* arch/arm/kernel/signal.c */
+asmlinkage int sys_sigsuspend(int, unsigned long, old_sigset_t);
+asmlinkage int sys_sigaction(int, const struct old_sigaction __user *,
+ struct old_sigaction __user *);
+asmlinkage int sys_sigreturn(struct pt_regs *);
+asmlinkage int sys_rt_sigreturn(struct pt_regs *);
+
+/* arch/arm/kernel/sys_arm.c */
+asmlinkage int sys_fork(struct pt_regs *);
+asmlinkage int sys_clone(unsigned long, unsigned long, int __user *, int,
+ int __user *, struct pt_regs *);
+asmlinkage int sys_vfork(struct pt_regs *);
+asmlinkage int sys_execve(const char __user *,
+ const char __user *const __user *,
+ const char __user *const __user *, struct pt_regs *);
+asmlinkage long sys_arm_fadvise64_64(int, int, loff_t, loff_t);
+
+/* arch/arm/kernel/sys_oabi-compat.c */
+struct oldabi_stat64;
+asmlinkage long sys_oabi_stat64(const char __user *,
+ struct oldabi_stat64 __user *);
+asmlinkage long sys_oabi_lstat64(const char __user *,
+ struct oldabi_stat64 __user *);
+asmlinkage long sys_oabi_fstat64(unsigned long, struct oldabi_stat64 __user *);+asmlinkage long sys_oabi_fstatat64(int, const char __user *,
+ struct oldabi_stat64 __user *, int);
+asmlinkage long sys_oabi_fcntl64(unsigned int, unsigned int, unsigned long);
+
+struct oabi_epoll_event;
+asmlinkage long sys_oabi_epoll_ctl(int, int, int,
+ struct oabi_epoll_event __user *);
+asmlinkage long sys_oabi_epoll_wait(int, struct oabi_epoll_event __user *,
+ int, int);
+
+struct oabi_sembuf;
+asmlinkage long sys_oabi_semtimedop(int, struct oabi_sembuf __user *, unsigned,+ const struct timespec __user *);
+asmlinkage long sys_oabi_semop(int, struct oabi_sembuf __user *,
+ unsigned);
+asmlinkage int sys_oabi_ipc(uint, int, int, int, void __user *, long);
+asmlinkage long sys_oabi_bind(int, struct sockaddr __user *, int);
+asmlinkage long sys_oabi_connect(int, struct sockaddr __user *, int);
+asmlinkage long sys_oabi_sendto(int, void __user *, size_t, unsigned,
+ struct sockaddr __user *, int);
+asmlinkage long sys_oabi_sendmsg(int, struct msghdr __user *, unsigned);
+asmlinkage long sys_oabi_socketcall(int, unsigned long __user *);
+
+/* arch/arm/kernel/traps.c */
+asmlinkage int arm_syscall(int, struct pt_regs *);
+
+#endif
diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index 0340224..9498fb5 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -16,6 +16,7 @@

#include <asm/elf.h>
#include <asm/cacheflush.h>
+#include <asm/syscalls.h>
#include <asm/ucontext.h>
#include <asm/unistd.h>
#include <asm/vfp.h>
diff --git a/arch/arm/kernel/sys_arm.c b/arch/arm/kernel/sys_arm.c
index 62e7c61..5bda298 100644
--- a/arch/arm/kernel/sys_arm.c
+++ b/arch/arm/kernel/sys_arm.c
@@ -28,6 +28,8 @@
#include <linux/uaccess.h>
#include <linux/slab.h>

+#include <asm/syscalls.h>
+
/* Fork a new task - this creates a new program thread.
* This is called indirectly via a small wrapper
*/
diff --git a/arch/arm/kernel/sys_oabi-compat.c b/arch/arm/kernel/sys_oabi-compat.c
index af0aaeb..e71a41c 100644
--- a/arch/arm/kernel/sys_oabi-compat.c
+++ b/arch/arm/kernel/sys_oabi-compat.c
@@ -85,6 +85,8 @@
#include <linux/uaccess.h>
#include <linux/slab.h>

+#include <asm/syscalls.h>
+
struct oldabi_stat64 {
unsigned long long st_dev;
unsigned int __pad1;
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index d52eec2..d040421 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -27,6 +27,7 @@

#include <asm/atomic.h>
#include <asm/cacheflush.h>
+#include <asm/syscalls.h>
#include <asm/system.h>
#include <asm/unistd.h>
#include <asm/traps.h>

2011-06-21 19:31:42

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] ARM: quiet sparse noise due to __ARCH_WANT_SYS_RT_SIG(ACTION|SUSPEND)

On Tue, Jun 21, 2011 at 15:09, H Hartley Sweeten wrote:
> --- a/arch/arm/include/asm/unistd.h
> +++ b/arch/arm/include/asm/unistd.h
> @@ -467,6 +467,20 @@
> +#ifndef __ASSEMBLY__
> +
> +#include <linux/types.h>
> +#include <linux/linkage.h>
> +#include <linux/compiler.h>
> +#include <linux/signal.h>
> +
> +asmlinkage long sys_rt_sigaction(int sig, const struct sigaction __user *act,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct sigaction __user *oact,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t sigsetsize);
> +asmlinkage long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize);
> +
> +#endif /* !__ASSEMBLY__ */

yikes ... this really the only way to fix the sparse warning ?
-mike

2011-06-21 21:08:18

by Hartley Sweeten

[permalink] [raw]
Subject: RE: [PATCH] ARM: quiet sparse noise due to __ARCH_WANT_SYS_RT_SIG(ACTION|SUSPEND)

On Tuesday, June 21, 2011 12:16 PM, Russell King wrote:
> On Tue, Jun 21, 2011 at 12:09:45PM -0700, H Hartley Sweeten wrote:
>> ARM defines __ARCH_WANT_SYS_RT_SIG(ACTION|SUSPEND) which
>> produces the following sparse warnings in kernel/signal.c:
>>
>> warning: symbol 'sys_rt_sigaction' was not declared. Should it be static?
>> warning: symbol 'sys_rt_sigsuspend' was not declared. Should it be static?
>>
>> Since ARM doesn't include <asm-generic/syscalls.h>, due to different
>> calling conventions for some system calls, prototype the functions
>> in <asm/unistd.h> to quiet the noise.
>
> NAK.
>
> asm/unistd.h is an exported header. Please don't pollute it with kernel
> internal stuff. Instead, follow the hint in asm-generic and create a new
> header file to describe syscalls called... asm/syscalls.h.
>
> And.. the problem is a lot larger than you mention above:
>
> arch/arm/kernel/signal.c:68:16: warning: symbol 'sys_sigsuspend' was not declared. Should it be static?
> arch/arm/kernel/signal.c:84:1: warning: symbol 'sys_sigaction' was not declared. Should it be static?
> arch/arm/kernel/signal.c:328:16: warning: symbol 'sys_sigreturn' was not declared. Should it be static?
> arch/arm/kernel/signal.c:360:16: warning: symbol 'sys_rt_sigreturn' was not declared. Should it be static?
> arch/arm/kernel/sys_arm.c:34:16: warning: symbol 'sys_fork' was not declared. Should it be static?
> arch/arm/kernel/sys_arm.c:47:16: warning: symbol 'sys_clone' was not declared. Should it be static?
> arch/arm/kernel/sys_arm.c:57:16: warning: symbol 'sys_vfork' was not declared. Should it be static?
> arch/arm/kernel/sys_arm.c:65:16: warning: symbol 'sys_execve' was not declared. Should it be static?
> arch/arm/kernel/sys_arm.c:129:17: warning: symbol 'sys_arm_fadvise64_64' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:144:17: warning: symbol 'sys_oabi_stat64' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:154:17: warning: symbol 'sys_oabi_lstat64' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:164:17: warning: symbol 'sys_oabi_fstat64' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:174:17: warning: symbol 'sys_oabi_fstatat64' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:196:17: warning: symbol 'sys_oabi_fcntl64' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:249:17: warning: symbol 'sys_oabi_epoll_ctl' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:270:17: warning: symbol 'sys_oabi_epoll_wait' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:304:17: warning: symbol 'sys_oabi_semtimedop' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:343:17: warning: symbol 'sys_oabi_semop' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:349:16: warning: symbol 'sys_oabi_ipc' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:367:17: warning: symbol 'sys_oabi_bind' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:377:17: warning: symbol 'sys_oabi_connect' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:387:17: warning: symbol 'sys_oabi_sendto' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:400:17: warning: symbol 'sys_oabi_sendmsg' was not declared. Should it be static?
> arch/arm/kernel/sys_oabi-compat.c:426:17: warning: symbol 'sys_oabi_socketcall' was not declared. Should it be static?
> arch/arm/kernel/traps.c:464:16: warning: symbol 'arm_syscall' was not declared. Should it be static?
>
> So you actually need something like this (pasted, so whitespace
> damaged). I'll pull it out of my low priority queue and queue it up
> for the next merge window.

Yes, the problem is bigger than just sys_rt_sig(action|suspend).

I was also working on fixing the issues above by adding an asm/syscalls.h.
I just wasn't sure how to handle the sys_rt_sig(action|suspend) functions.

Your patch was whitespace damaged so I applied it by hand to test.

It does fix the sparse issues you list above. So FWIW....

Tested-by: H Hartley Sweeten <[email protected]>


But, it still does not fix the sparse warnings in kernel/signal.c:

kernel/signal.c:2718:1: warning: symbol 'sys_rt_sigaction' was not declared. Should it be static?
kernel/signal.c:2810:1: warning: symbol 'sys_rt_sigsuspend' was not declared. Should it be static?


FYI, there are a couple other related sparse warnings in arch/arm/kernel, but
these are not exactly "syscalls":

arch/arm/kernel/ptrace.c:795:16: warning: symbol 'syscall_trace' was not declared. Should it be static?
arch/arm/kernel/signal.c:787:1: warning: symbol 'do_notify_resume' was not declared. Should it be static?
arch/arm/kernel/traps.c:343:29: warning: symbol 'do_undefinstr' was not declared. Should it be static?
arch/arm/kernel/traps.c:386:17: warning: symbol 'do_unexp_fiq' was not declared. Should it be static?
arch/arm/kernel/traps.c:398:17: warning: symbol 'bad_mode' was not declared. Should it be static?
arch/arm/kernel/traps.c:661:6: warning: symbol '__bad_xchg' was not declared. Should it be static?
arch/arm/kernel/traps.c:674:1: warning: symbol 'baddataabort' was not declared. Should it be static?
arch/arm/kernel/traps.c:706:6: warning: symbol '__readwrite_bug' was not declared. Should it be static?
arch/arm/kernel/traps.c:728:17: warning: symbol '__div0' was not declared. Should it be static?
arch/arm/kernel/traps.c:735:6: warning: symbol 'abort' was not declared. Should it be static?

Regards,
Hartley-

2011-06-21 22:32:20

by Hartley Sweeten

[permalink] [raw]
Subject: RE: [PATCH] ARM: quiet sparse noise due to __ARCH_WANT_SYS_RT_SIG(ACTION|SUSPEND)

On Tuesday, June 21, 2011 12:31 PM, Mike Frysinger wrote:
> On Tue, Jun 21, 2011 at 15:09, H Hartley Sweeten wrote:
>> --- a/arch/arm/include/asm/unistd.h
>> +++ b/arch/arm/include/asm/unistd.h
>> @@ -467,6 +467,20 @@
>> +#ifndef __ASSEMBLY__
>> +
>> +#include <linux/types.h>
>> +#include <linux/linkage.h>
>> +#include <linux/compiler.h>
>> +#include <linux/signal.h>
>> +
>> +asmlinkage long sys_rt_sigaction(int sig, const struct sigaction __user *act,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct sigaction __user *oact,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t sigsetsize);
>> +asmlinkage long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize);
>> +
>> +#endif /* !__ASSEMBLY__ */
>
> yikes ... this really the only way to fix the sparse warning ?

Not sure...

Russell's patch to add an asm/syscalls.h fixes the other system call
sparse warnings in arm/arm/kernel/* but it doesn't fix the ones in
kernel/signal.c. This patch does but, as Russell said, asm/unistd.h
is exported to it's a bit ugly.

BTW, arch/ia64 does something very similar:


#if !defined(__ASSEMBLY__) && !defined(ASSEMBLER)

#include <linux/types.h>
#include <linux/linkage.h>
#include <linux/compiler.h>

extern long __ia64_syscall (long a0, long a1, long a2, long a3, long a4, long nr);

asmlinkage unsigned long sys_mmap(
unsigned long addr, unsigned long len,
int prot, int flags,
int fd, long off);
asmlinkage unsigned long sys_mmap2(
unsigned long addr, unsigned long len,
int prot, int flags,
int fd, long pgoff);
struct pt_regs;
struct sigaction;
asmlinkage long sys_ia64_pipe(void);
asmlinkage long sys_rt_sigaction(int sig,
const struct sigaction __user *act,
struct sigaction __user *oact,
size_t sigsetsize);

Regards,
Hartley-