* [PATCH] signal: fix building with clang
@ 2019-03-07 9:11 Arnd Bergmann
2019-03-07 10:03 ` Christian Brauner
2019-03-07 15:28 ` Oleg Nesterov
0 siblings, 2 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-03-07 9:11 UTC (permalink / raw)
To: linux-kernel
Cc: Nick Desaulniers, Oleg Nesterov, Eric W . Biederman,
Alexander Viro, linux-arch, Arnd Bergmann, Ralf Baechle,
Paul Burton, James Hogan, Andrew Morton, Deepa Dinamani,
Christian Brauner, Gustavo A. R. Silva, linux-mips
clang warns about the sigset_t manipulating functions (sigaddset, sigdelset,
sigisemptyset, ...) because it performs semantic analysis before discarding
dead code, unlike gcc that does this in the reverse order.
The result is a long list of warnings like:
In file included from arch/arm64/include/asm/ftrace.h:21:
include/linux/compat.h:489:10: error: array index 3 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds]
case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
^ ~
include/linux/compat.h:138:2: note: array 'sig' declared here
compat_sigset_word sig[_COMPAT_NSIG_WORDS];
^
include/linux/compat.h:489:42: error: array index 2 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds]
case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
^ ~
include/linux/compat.h:138:2: note: array 'sig' declared here
compat_sigset_word sig[_COMPAT_NSIG_WORDS];
^
As a (rather ugly) workaround, I turn the nice switch()/case statements
into preprocessor conditionals, and where that is not possible, use the
'%' operator to modify the warning case into an operation that clang
will not warn about. Since that only matters for dead code, the actual
behavior does not change.
Link: https://bugs.llvm.org/show_bug.cgi?id=38789
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/mips/include/uapi/asm/signal.h | 3 +-
include/linux/signal.h | 72 ++++++++++++++---------------
2 files changed, 37 insertions(+), 38 deletions(-)
diff --git a/arch/mips/include/uapi/asm/signal.h b/arch/mips/include/uapi/asm/signal.h
index 53104b10aae2..8e71a2f778f7 100644
--- a/arch/mips/include/uapi/asm/signal.h
+++ b/arch/mips/include/uapi/asm/signal.h
@@ -11,9 +11,10 @@
#define _UAPI_ASM_SIGNAL_H
#include <linux/types.h>
+#include <asm/bitsperlong.h>
#define _NSIG 128
-#define _NSIG_BPW (sizeof(unsigned long) * 8)
+#define _NSIG_BPW __BITS_PER_LONG
#define _NSIG_WORDS (_NSIG / _NSIG_BPW)
typedef struct {
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 9702016734b1..b967d502ab61 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -82,35 +82,33 @@ static inline int sigismember(sigset_t *set, int _sig)
static inline int sigisemptyset(sigset_t *set)
{
- switch (_NSIG_WORDS) {
- case 4:
- return (set->sig[3] | set->sig[2] |
- set->sig[1] | set->sig[0]) == 0;
- case 2:
- return (set->sig[1] | set->sig[0]) == 0;
- case 1:
- return set->sig[0] == 0;
- default:
- BUILD_BUG();
- return 0;
- }
+#if _NSIG_WORDS == 4
+ return (set->sig[3] | set->sig[2] |
+ set->sig[1] | set->sig[0]) == 0;
+#elif _NSIG_WORDS == 2
+ return (set->sig[1] | set->sig[0]) == 0;
+#elif _NSIG_WORDS == 1
+ return set->sig[0] == 0;
+#else
+ BUILD_BUG();
+#endif
}
static inline int sigequalsets(const sigset_t *set1, const sigset_t *set2)
{
- switch (_NSIG_WORDS) {
- case 4:
- return (set1->sig[3] == set2->sig[3]) &&
- (set1->sig[2] == set2->sig[2]) &&
- (set1->sig[1] == set2->sig[1]) &&
- (set1->sig[0] == set2->sig[0]);
- case 2:
- return (set1->sig[1] == set2->sig[1]) &&
- (set1->sig[0] == set2->sig[0]);
- case 1:
- return set1->sig[0] == set2->sig[0];
- }
+#if _NSIG_WORDS == 4
+ return (set1->sig[3] == set2->sig[3]) &&
+ (set1->sig[2] == set2->sig[2]) &&
+ (set1->sig[1] == set2->sig[1]) &&
+ (set1->sig[0] == set2->sig[0]);
+#elif _NSIG_WORDS == 2
+ return (set1->sig[1] == set2->sig[1]) &&
+ (set1->sig[0] == set2->sig[0]);
+#elif _NSIG_WORDS == 1
+ return set1->sig[0] == set2->sig[0];
+#else
return 0;
+#endif
}
#define sigmask(sig) (1UL << ((sig) - 1))
@@ -125,14 +123,14 @@ static inline void name(sigset_t *r, const sigset_t *a, const sigset_t *b) \
\
switch (_NSIG_WORDS) { \
case 4: \
- a3 = a->sig[3]; a2 = a->sig[2]; \
- b3 = b->sig[3]; b2 = b->sig[2]; \
- r->sig[3] = op(a3, b3); \
- r->sig[2] = op(a2, b2); \
+ a3 = a->sig[3%_NSIG_WORDS]; a2 = a->sig[2%_NSIG_WORDS]; \
+ b3 = b->sig[3%_NSIG_WORDS]; b2 = b->sig[2%_NSIG_WORDS]; \
+ r->sig[3%_NSIG_WORDS] = op(a3, b3); \
+ r->sig[2%_NSIG_WORDS] = op(a2, b2); \
/* fall through */ \
case 2: \
- a1 = a->sig[1]; b1 = b->sig[1]; \
- r->sig[1] = op(a1, b1); \
+ a1 = a->sig[1%_NSIG_WORDS]; b1 = b->sig[1%_NSIG_WORDS]; \
+ r->sig[1%_NSIG_WORDS] = op(a1, b1); \
/* fall through */ \
case 1: \
a0 = a->sig[0]; b0 = b->sig[0]; \
@@ -161,10 +159,10 @@ _SIG_SET_BINOP(sigandnsets, _sig_andn)
static inline void name(sigset_t *set) \
{ \
switch (_NSIG_WORDS) { \
- case 4: set->sig[3] = op(set->sig[3]); \
- set->sig[2] = op(set->sig[2]); \
+ case 4: set->sig[3%_NSIG_WORDS] = op(set->sig[3%_NSIG_WORDS]); \
+ set->sig[2%_NSIG_WORDS] = op(set->sig[2%_NSIG_WORDS]); \
/* fall through */ \
- case 2: set->sig[1] = op(set->sig[1]); \
+ case 2: set->sig[1%_NSIG_WORDS] = op(set->sig[1%_NSIG_WORDS]); \
/* fall through */ \
case 1: set->sig[0] = op(set->sig[0]); \
break; \
@@ -185,7 +183,7 @@ static inline void sigemptyset(sigset_t *set)
default:
memset(set, 0, sizeof(sigset_t));
break;
- case 2: set->sig[1] = 0;
+ case 2: set->sig[1%_NSIG_WORDS] = 0;
/* fall through */
case 1: set->sig[0] = 0;
break;
@@ -198,7 +196,7 @@ static inline void sigfillset(sigset_t *set)
default:
memset(set, -1, sizeof(sigset_t));
break;
- case 2: set->sig[1] = -1;
+ case 2: set->sig[1%_NSIG_WORDS] = -1;
/* fall through */
case 1: set->sig[0] = -1;
break;
@@ -229,7 +227,7 @@ static inline void siginitset(sigset_t *set, unsigned long mask)
default:
memset(&set->sig[1], 0, sizeof(long)*(_NSIG_WORDS-1));
break;
- case 2: set->sig[1] = 0;
+ case 2: set->sig[1%_NSIG_WORDS] = 0;
case 1: ;
}
}
@@ -241,7 +239,7 @@ static inline void siginitsetinv(sigset_t *set, unsigned long mask)
default:
memset(&set->sig[1], -1, sizeof(long)*(_NSIG_WORDS-1));
break;
- case 2: set->sig[1] = -1;
+ case 2: set->sig[1%_NSIG_WORDS] = -1;
case 1: ;
}
}
--
2.20.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] signal: fix building with clang
2019-03-07 9:11 [PATCH] signal: fix building with clang Arnd Bergmann
@ 2019-03-07 10:03 ` Christian Brauner
2019-03-07 15:28 ` Oleg Nesterov
1 sibling, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2019-03-07 10:03 UTC (permalink / raw)
To: Arnd Bergmann, linux-kernel
Cc: Nick Desaulniers, Oleg Nesterov, Eric W . Biederman,
Alexander Viro, linux-arch, Ralf Baechle, Paul Burton,
James Hogan, Andrew Morton, Deepa Dinamani, Gustavo A. R. Silva,
linux-mips
On March 7, 2019 10:11:52 AM GMT+01:00, Arnd Bergmann <arnd@arndb.de> wrote:
>clang warns about the sigset_t manipulating functions (sigaddset,
>sigdelset,
>sigisemptyset, ...) because it performs semantic analysis before
>discarding
>dead code, unlike gcc that does this in the reverse order.
>
>The result is a long list of warnings like:
>
>In file included from arch/arm64/include/asm/ftrace.h:21:
>include/linux/compat.h:489:10: error: array index 3 is past the end of
>the array (which contains 2 elements) [-Werror,-Warray-bounds]
> case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
> ^ ~
>include/linux/compat.h:138:2: note: array 'sig' declared here
> compat_sigset_word sig[_COMPAT_NSIG_WORDS];
> ^
>include/linux/compat.h:489:42: error: array index 2 is past the end of
>the array (which contains 2 elements) [-Werror,-Warray-bounds]
> case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
> ^ ~
>include/linux/compat.h:138:2: note: array 'sig' declared here
> compat_sigset_word sig[_COMPAT_NSIG_WORDS];
> ^
>
>As a (rather ugly) workaround, I turn the nice switch()/case statements
>into preprocessor conditionals, and where that is not possible, use the
>'%' operator to modify the warning case into an operation that clang
>will not warn about. Since that only matters for dead code, the actual
>behavior does not change.
>
>Link: https://bugs.llvm.org/show_bug.cgi?id=38789
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>---
> arch/mips/include/uapi/asm/signal.h | 3 +-
> include/linux/signal.h | 72 ++++++++++++++---------------
> 2 files changed, 37 insertions(+), 38 deletions(-)
>
>diff --git a/arch/mips/include/uapi/asm/signal.h
>b/arch/mips/include/uapi/asm/signal.h
>index 53104b10aae2..8e71a2f778f7 100644
>--- a/arch/mips/include/uapi/asm/signal.h
>+++ b/arch/mips/include/uapi/asm/signal.h
>@@ -11,9 +11,10 @@
> #define _UAPI_ASM_SIGNAL_H
>
> #include <linux/types.h>
>+#include <asm/bitsperlong.h>
>
> #define _NSIG 128
>-#define _NSIG_BPW (sizeof(unsigned long) * 8)
>+#define _NSIG_BPW __BITS_PER_LONG
> #define _NSIG_WORDS (_NSIG / _NSIG_BPW)
>
> typedef struct {
>diff --git a/include/linux/signal.h b/include/linux/signal.h
>index 9702016734b1..b967d502ab61 100644
>--- a/include/linux/signal.h
>+++ b/include/linux/signal.h
>@@ -82,35 +82,33 @@ static inline int sigismember(sigset_t *set, int
>_sig)
>
> static inline int sigisemptyset(sigset_t *set)
> {
>- switch (_NSIG_WORDS) {
>- case 4:
>- return (set->sig[3] | set->sig[2] |
>- set->sig[1] | set->sig[0]) == 0;
>- case 2:
>- return (set->sig[1] | set->sig[0]) == 0;
>- case 1:
>- return set->sig[0] == 0;
>- default:
>- BUILD_BUG();
>- return 0;
>- }
>+#if _NSIG_WORDS == 4
>+ return (set->sig[3] | set->sig[2] |
>+ set->sig[1] | set->sig[0]) == 0;
>+#elif _NSIG_WORDS == 2
>+ return (set->sig[1] | set->sig[0]) == 0;
>+#elif _NSIG_WORDS == 1
>+ return set->sig[0] == 0;
>+#else
>+ BUILD_BUG();
>+#endif
> }
>
>static inline int sigequalsets(const sigset_t *set1, const sigset_t
>*set2)
> {
>- switch (_NSIG_WORDS) {
>- case 4:
>- return (set1->sig[3] == set2->sig[3]) &&
>- (set1->sig[2] == set2->sig[2]) &&
>- (set1->sig[1] == set2->sig[1]) &&
>- (set1->sig[0] == set2->sig[0]);
>- case 2:
>- return (set1->sig[1] == set2->sig[1]) &&
>- (set1->sig[0] == set2->sig[0]);
>- case 1:
>- return set1->sig[0] == set2->sig[0];
>- }
>+#if _NSIG_WORDS == 4
>+ return (set1->sig[3] == set2->sig[3]) &&
>+ (set1->sig[2] == set2->sig[2]) &&
>+ (set1->sig[1] == set2->sig[1]) &&
>+ (set1->sig[0] == set2->sig[0]);
>+#elif _NSIG_WORDS == 2
>+ return (set1->sig[1] == set2->sig[1]) &&
>+ (set1->sig[0] == set2->sig[0]);
>+#elif _NSIG_WORDS == 1
>+ return set1->sig[0] == set2->sig[0];
>+#else
> return 0;
>+#endif
> }
>
> #define sigmask(sig) (1UL << ((sig) - 1))
>@@ -125,14 +123,14 @@ static inline void name(sigset_t *r, const
>sigset_t *a, const sigset_t *b) \
> \
> switch (_NSIG_WORDS) { \
> case 4: \
>- a3 = a->sig[3]; a2 = a->sig[2]; \
>- b3 = b->sig[3]; b2 = b->sig[2]; \
>- r->sig[3] = op(a3, b3); \
>- r->sig[2] = op(a2, b2); \
>+ a3 = a->sig[3%_NSIG_WORDS]; a2 = a->sig[2%_NSIG_WORDS]; \
>+ b3 = b->sig[3%_NSIG_WORDS]; b2 = b->sig[2%_NSIG_WORDS]; \
>+ r->sig[3%_NSIG_WORDS] = op(a3, b3); \
>+ r->sig[2%_NSIG_WORDS] = op(a2, b2); \
> /* fall through */ \
> case 2: \
>- a1 = a->sig[1]; b1 = b->sig[1]; \
>- r->sig[1] = op(a1, b1); \
>+ a1 = a->sig[1%_NSIG_WORDS]; b1 = b->sig[1%_NSIG_WORDS]; \
>+ r->sig[1%_NSIG_WORDS] = op(a1, b1); \
> /* fall through */ \
> case 1: \
> a0 = a->sig[0]; b0 = b->sig[0]; \
>@@ -161,10 +159,10 @@ _SIG_SET_BINOP(sigandnsets, _sig_andn)
> static inline void name(sigset_t *set) \
> { \
> switch (_NSIG_WORDS) { \
>- case 4: set->sig[3] = op(set->sig[3]); \
>- set->sig[2] = op(set->sig[2]); \
>+ case 4: set->sig[3%_NSIG_WORDS] = op(set->sig[3%_NSIG_WORDS]); \
>+ set->sig[2%_NSIG_WORDS] = op(set->sig[2%_NSIG_WORDS]); \
> /* fall through */ \
>- case 2: set->sig[1] = op(set->sig[1]); \
>+ case 2: set->sig[1%_NSIG_WORDS] = op(set->sig[1%_NSIG_WORDS]); \
> /* fall through */ \
> case 1: set->sig[0] = op(set->sig[0]); \
> break; \
>@@ -185,7 +183,7 @@ static inline void sigemptyset(sigset_t *set)
> default:
> memset(set, 0, sizeof(sigset_t));
> break;
>- case 2: set->sig[1] = 0;
>+ case 2: set->sig[1%_NSIG_WORDS] = 0;
> /* fall through */
> case 1: set->sig[0] = 0;
> break;
>@@ -198,7 +196,7 @@ static inline void sigfillset(sigset_t *set)
> default:
> memset(set, -1, sizeof(sigset_t));
> break;
>- case 2: set->sig[1] = -1;
>+ case 2: set->sig[1%_NSIG_WORDS] = -1;
> /* fall through */
> case 1: set->sig[0] = -1;
> break;
>@@ -229,7 +227,7 @@ static inline void siginitset(sigset_t *set,
>unsigned long mask)
> default:
> memset(&set->sig[1], 0, sizeof(long)*(_NSIG_WORDS-1));
> break;
>- case 2: set->sig[1] = 0;
>+ case 2: set->sig[1%_NSIG_WORDS] = 0;
> case 1: ;
> }
> }
>@@ -241,7 +239,7 @@ static inline void siginitsetinv(sigset_t *set,
>unsigned long mask)
> default:
> memset(&set->sig[1], -1, sizeof(long)*(_NSIG_WORDS-1));
> break;
>- case 2: set->sig[1] = -1;
>+ case 2: set->sig[1%_NSIG_WORDS] = -1;
> case 1: ;
> }
> }
Ugh, it's not pretty but I don't have any objections. :)
Acked-by: Christian Brauner <christian@brauner.io>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] signal: fix building with clang
2019-03-07 9:11 [PATCH] signal: fix building with clang Arnd Bergmann
2019-03-07 10:03 ` Christian Brauner
@ 2019-03-07 15:28 ` Oleg Nesterov
2019-03-07 15:37 ` Arnd Bergmann
1 sibling, 1 reply; 10+ messages in thread
From: Oleg Nesterov @ 2019-03-07 15:28 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Nick Desaulniers, Eric W . Biederman,
Alexander Viro, linux-arch, Ralf Baechle, Paul Burton,
James Hogan, Andrew Morton, Deepa Dinamani, Christian Brauner,
Gustavo A. R. Silva, linux-mips
On 03/07, Arnd Bergmann wrote:
>
> clang warns about the sigset_t manipulating functions (sigaddset, sigdelset,
> sigisemptyset, ...) because it performs semantic analysis before discarding
> dead code, unlike gcc that does this in the reverse order.
>
> The result is a long list of warnings like:
>
> In file included from arch/arm64/include/asm/ftrace.h:21:
> include/linux/compat.h:489:10: error: array index 3 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds]
> case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
stupid question... I have no idea if this can work or not, but may be we can just do
--- x/Makefile
+++ x/Makefile
@@ -701,6 +701,7 @@ KBUILD_CPPFLAGS += $(call cc-option,-Qun
KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier)
KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
+KBUILD_CFLAGS += $(call cc-disable-warning, array-bounds)
# Quiet clang warning: comparison of unsigned expression < 0 is always false
KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the
?
> As a (rather ugly) workaround,
Yes :/
But I am not going to argue, just a couple of questions.
> I turn the nice switch()/case statements
> into preprocessor conditionals, and where that is not possible, use the
> '%' operator
I can't say what looks worse... to me it would be either use ifdef's or %'s
everywhere in signal.h, with this patch the code doesn't look consistent.
But I won't insist.
> static inline int sigisemptyset(sigset_t *set)
> {
> - switch (_NSIG_WORDS) {
> - case 4:
> - return (set->sig[3] | set->sig[2] |
> - set->sig[1] | set->sig[0]) == 0;
> - case 2:
> - return (set->sig[1] | set->sig[0]) == 0;
> - case 1:
> - return set->sig[0] == 0;
> - default:
> - BUILD_BUG();
> - return 0;
> - }
> +#if _NSIG_WORDS == 4
> + return (set->sig[3] | set->sig[2] |
> + set->sig[1] | set->sig[0]) == 0;
> +#elif _NSIG_WORDS == 2
> + return (set->sig[1] | set->sig[0]) == 0;
> +#elif _NSIG_WORDS == 1
> + return set->sig[0] == 0;
> +#else
> + BUILD_BUG();
> +#endif
> }
Or perhaps we can simply rewrite this and other helpers?
I don't think that, say,
static inline int sigisemptyset(sigset_t *set)
{
for (i = 0; i < ARRAY_SIZE(set->sig); ++i)
set->sig[i] = 0;
}
will make asm worse...
Oleg.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] signal: fix building with clang
2019-03-07 15:28 ` Oleg Nesterov
@ 2019-03-07 15:37 ` Arnd Bergmann
2019-03-07 16:46 ` Oleg Nesterov
0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2019-03-07 15:37 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Linux Kernel Mailing List, Nick Desaulniers, Eric W . Biederman,
Alexander Viro, linux-arch, Ralf Baechle, Paul Burton,
James Hogan, Andrew Morton, Deepa Dinamani, Christian Brauner,
Gustavo A. R. Silva, linux-mips
On Thu, Mar 7, 2019 at 4:28 PM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 03/07, Arnd Bergmann wrote:
> >
> > clang warns about the sigset_t manipulating functions (sigaddset, sigdelset,
> > sigisemptyset, ...) because it performs semantic analysis before discarding
> > dead code, unlike gcc that does this in the reverse order.
> >
> > The result is a long list of warnings like:
> >
> > In file included from arch/arm64/include/asm/ftrace.h:21:
> > include/linux/compat.h:489:10: error: array index 3 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds]
> > case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
>
> stupid question... I have no idea if this can work or not, but may be we can just do
>
> --- x/Makefile
> +++ x/Makefile
> @@ -701,6 +701,7 @@ KBUILD_CPPFLAGS += $(call cc-option,-Qun
> KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier)
> KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
> KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> +KBUILD_CFLAGS += $(call cc-disable-warning, array-bounds)
> # Quiet clang warning: comparison of unsigned expression < 0 is always false
> KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
> # CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the
>
> ?
I would definitely not go that far, since the warning is generally
rather useful,
but we could try something more localized
__diag_push();
__diag_ignore(clang, 7, "-Warray-bounds");
...
__diag_pop();
> > I turn the nice switch()/case statements
> > into preprocessor conditionals, and where that is not possible, use the
> > '%' operator
>
> I can't say what looks worse... to me it would be either use ifdef's or %'s
> everywhere in signal.h, with this patch the code doesn't look consistent.
> But I won't insist.
I tried using just #ifdefs, but that did not work inside of macros.
We could use % everywhere, or possible wrap it inside of another
macro.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] signal: fix building with clang
2019-03-07 15:37 ` Arnd Bergmann
@ 2019-03-07 16:46 ` Oleg Nesterov
2019-03-07 18:41 ` Nick Desaulniers
2019-03-07 21:45 ` Arnd Bergmann
0 siblings, 2 replies; 10+ messages in thread
From: Oleg Nesterov @ 2019-03-07 16:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Kernel Mailing List, Nick Desaulniers, Eric W . Biederman,
Alexander Viro, linux-arch, Ralf Baechle, Paul Burton,
James Hogan, Andrew Morton, Deepa Dinamani, Christian Brauner,
Gustavo A. R. Silva, linux-mips
On 03/07, Arnd Bergmann wrote:
>
> We could use % everywhere,
Yes.
But again, why not simply use the "for (;;)" loops? Why we can't kill the
supid switch(_NSIG_WORDS) tricks altogether?
Oleg.
--- x/include/linux/signal.h
+++ x/include/linux/signal.h
@@ -121,26 +121,9 @@
#define _SIG_SET_BINOP(name, op) \
static inline void name(sigset_t *r, const sigset_t *a, const sigset_t *b) \
{ \
- unsigned long a0, a1, a2, a3, b0, b1, b2, b3; \
- \
- switch (_NSIG_WORDS) { \
- case 4: \
- a3 = a->sig[3]; a2 = a->sig[2]; \
- b3 = b->sig[3]; b2 = b->sig[2]; \
- r->sig[3] = op(a3, b3); \
- r->sig[2] = op(a2, b2); \
- /* fall through */ \
- case 2: \
- a1 = a->sig[1]; b1 = b->sig[1]; \
- r->sig[1] = op(a1, b1); \
- /* fall through */ \
- case 1: \
- a0 = a->sig[0]; b0 = b->sig[0]; \
- r->sig[0] = op(a0, b0); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
+ int i; \
+ for (i = 0; i < ARRAY_SIZE(r->sig); ++i) \
+ r->sig[i] = op(a->sig[i], b->sig[i]); \
}
#define _sig_or(x,y) ((x) | (y))
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] signal: fix building with clang
2019-03-07 16:46 ` Oleg Nesterov
@ 2019-03-07 18:41 ` Nick Desaulniers
2019-03-07 21:45 ` Arnd Bergmann
1 sibling, 0 replies; 10+ messages in thread
From: Nick Desaulniers @ 2019-03-07 18:41 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Arnd Bergmann, Linux Kernel Mailing List, Eric W . Biederman,
Alexander Viro, linux-arch, Ralf Baechle, Paul Burton,
James Hogan, Andrew Morton, Deepa Dinamani, Christian Brauner,
Gustavo A. R. Silva, linux-mips
On Thu, Mar 7, 2019 at 8:46 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 03/07, Arnd Bergmann wrote:
> >
> > We could use % everywhere,
>
> Yes.
>
> But again, why not simply use the "for (;;)" loops? Why we can't kill the
> supid switch(_NSIG_WORDS) tricks altogether?
>
> Oleg.
>
> --- x/include/linux/signal.h
> +++ x/include/linux/signal.h
> @@ -121,26 +121,9 @@
> #define _SIG_SET_BINOP(name, op) \
> static inline void name(sigset_t *r, const sigset_t *a, const sigset_t *b) \
> { \
> - unsigned long a0, a1, a2, a3, b0, b1, b2, b3; \
> - \
> - switch (_NSIG_WORDS) { \
> - case 4: \
> - a3 = a->sig[3]; a2 = a->sig[2]; \
> - b3 = b->sig[3]; b2 = b->sig[2]; \
> - r->sig[3] = op(a3, b3); \
> - r->sig[2] = op(a2, b2); \
> - /* fall through */ \
> - case 2: \
> - a1 = a->sig[1]; b1 = b->sig[1]; \
> - r->sig[1] = op(a1, b1); \
> - /* fall through */ \
> - case 1: \
> - a0 = a->sig[0]; b0 = b->sig[0]; \
> - r->sig[0] = op(a0, b0); \
> - break; \
> - default: \
> - BUILD_BUG(); \
> - } \
> + int i; \
> + for (i = 0; i < ARRAY_SIZE(r->sig); ++i) \
> + r->sig[i] = op(a->sig[i], b->sig[i]); \
> }
>
> #define _sig_or(x,y) ((x) | (y))
>
That looks much cleaner IMO.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] signal: fix building with clang
2019-03-07 16:46 ` Oleg Nesterov
2019-03-07 18:41 ` Nick Desaulniers
@ 2019-03-07 21:45 ` Arnd Bergmann
2019-03-08 0:22 ` Nick Desaulniers
1 sibling, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2019-03-07 21:45 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Linux Kernel Mailing List, Nick Desaulniers, Eric W . Biederman,
Alexander Viro, linux-arch, Ralf Baechle, Paul Burton,
James Hogan, Andrew Morton, Deepa Dinamani, Christian Brauner,
Gustavo A. R. Silva, linux-mips
On Thu, Mar 7, 2019 at 5:46 PM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 03/07, Arnd Bergmann wrote:
> >
> > We could use % everywhere,
>
> Yes.
>
> But again, why not simply use the "for (;;)" loops? Why we can't kill the
> supid switch(_NSIG_WORDS) tricks altogether?
I'd have to try, but I think you are right. It was probably an
overoptimization back in 1997 when the code got added to
linux-2.1.68pre1, and compilers have become smarter in the
meantime ;-)
Also, the common case these days is _NSIG_WORDS==1, which
is true on all 64-bit architectures other than MIPS64.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] signal: fix building with clang
2019-03-07 21:45 ` Arnd Bergmann
@ 2019-03-08 0:22 ` Nick Desaulniers
2019-03-08 0:27 ` Joe Perches
0 siblings, 1 reply; 10+ messages in thread
From: Nick Desaulniers @ 2019-03-08 0:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Oleg Nesterov, Linux Kernel Mailing List, Eric W . Biederman,
Alexander Viro, linux-arch, Ralf Baechle, Paul Burton,
James Hogan, Andrew Morton, Deepa Dinamani, Christian Brauner,
Gustavo A. R. Silva, linux-mips
On Thu, Mar 7, 2019 at 1:45 PM Arnd Bergmann <arnd@arndb.de> wrote:
> I'd have to try, but I think you are right. It was probably an
> overoptimization back in 1997 when the code got added to
> linux-2.1.68pre1, and compilers have become smarter in the
> meantime ;-)
How do you track history pre-git (2.6.XX)?
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] signal: fix building with clang
2019-03-08 0:22 ` Nick Desaulniers
@ 2019-03-08 0:27 ` Joe Perches
2019-03-08 21:09 ` Nick Desaulniers
0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2019-03-08 0:27 UTC (permalink / raw)
To: Nick Desaulniers, Arnd Bergmann
Cc: Oleg Nesterov, Linux Kernel Mailing List, Eric W . Biederman,
Alexander Viro, linux-arch, Ralf Baechle, Paul Burton,
James Hogan, Andrew Morton, Deepa Dinamani, Christian Brauner,
Gustavo A. R. Silva, linux-mips
On Thu, 2019-03-07 at 16:22 -0800, Nick Desaulniers wrote:
> On Thu, Mar 7, 2019 at 1:45 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > I'd have to try, but I think you are right. It was probably an
> > overoptimization back in 1997 when the code got added to
> > linux-2.1.68pre1, and compilers have become smarter in the
> > meantime ;-)
>
> How do you track history pre-git (2.6.XX)?
https://landley.net/kdocs/fullhist/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] signal: fix building with clang
2019-03-08 0:27 ` Joe Perches
@ 2019-03-08 21:09 ` Nick Desaulniers
0 siblings, 0 replies; 10+ messages in thread
From: Nick Desaulniers @ 2019-03-08 21:09 UTC (permalink / raw)
To: Joe Perches
Cc: Arnd Bergmann, Oleg Nesterov, Linux Kernel Mailing List,
Eric W . Biederman, Alexander Viro, linux-arch, Ralf Baechle,
Paul Burton, James Hogan, Andrew Morton, Deepa Dinamani,
Christian Brauner, Gustavo A. R. Silva, linux-mips
On Thu, Mar 7, 2019 at 4:27 PM Joe Perches <joe@perches.com> wrote:
>
> On Thu, 2019-03-07 at 16:22 -0800, Nick Desaulniers wrote:
> > On Thu, Mar 7, 2019 at 1:45 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > I'd have to try, but I think you are right. It was probably an
> > > overoptimization back in 1997 when the code got added to
> > > linux-2.1.68pre1, and compilers have become smarter in the
> > > meantime ;-)
> >
> > How do you track history pre-git (2.6.XX)?
>
> https://landley.net/kdocs/fullhist/
Ah neat! Thanks for the link. Now to wire that up to fugitive:
http://vimcasts.org/episodes/fugitive-vim-exploring-the-history-of-a-git-repository/
The LLVM project recently switched to git from svn. For quite some
time the move was delayed in order to preserve history (including for
parked release branches and all kinds of edge cases and things that
don't quite translate from svn to git). Now I better appreciate the
history preservation.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-03-08 21:10 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 9:11 [PATCH] signal: fix building with clang Arnd Bergmann
2019-03-07 10:03 ` Christian Brauner
2019-03-07 15:28 ` Oleg Nesterov
2019-03-07 15:37 ` Arnd Bergmann
2019-03-07 16:46 ` Oleg Nesterov
2019-03-07 18:41 ` Nick Desaulniers
2019-03-07 21:45 ` Arnd Bergmann
2019-03-08 0:22 ` Nick Desaulniers
2019-03-08 0:27 ` Joe Perches
2019-03-08 21:09 ` Nick Desaulniers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.