[PATCH] signal: fix building with clang

[PATCH] signal: fix building with clang

[Arnd Bergmann]

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

Re: [PATCH] signal: fix building with clang

[Christian Brauner]

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>

Re: [PATCH] signal: fix building with clang

[Oleg Nesterov]

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.

Re: [PATCH] signal: fix building with clang

[Arnd Bergmann]

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

Re: [PATCH] signal: fix building with clang

[Oleg Nesterov]

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))

Re: [PATCH] signal: fix building with clang

[Nick Desaulniers]

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

Re: [PATCH] signal: fix building with clang

[Arnd Bergmann]

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

Re: [PATCH] signal: fix building with clang

[Nick Desaulniers]

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

Re: [PATCH] signal: fix building with clang

[Joe Perches]

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/

Re: [PATCH] signal: fix building with clang

[Nick Desaulniers]

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