Linux-MIPS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/5] Clang build fixes for MIPS
@ 2019-08-12  3:31 Nathan Chancellor
  2019-08-12  3:31 ` [PATCH 1/5] MIPS: Don't use bc_false uninitialized in __mm_isBranchInstr Nathan Chancellor
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  3:31 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux

Hi all,

As of clang 9.0.0 at r366299 [1], we can build a QEMU bootable
malta_defconfig kernel with the following fixes (mostly due to -Werror)
and Nick's patch [2]. This has helped catch some potentially dubious
behavior with -Wuninitialized, which is stronger than GCC's
-Wmaybe-uninitialized.

If there are any comments or concerns, please let me know.

[1]: https://github.com/llvm/llvm-project/commit/7f308af5eeea2d1b24aee0361d39dc43bac4cfe5
[2]: https://lore.kernel.org/lkml/20190729211014.39333-1-ndesaulniers@google.com/

Cheers,
Nathan



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/5] MIPS: Don't use bc_false uninitialized in __mm_isBranchInstr
  2019-08-12  3:31 [PATCH 0/5] Clang build fixes for MIPS Nathan Chancellor
@ 2019-08-12  3:31 ` Nathan Chancellor
  2019-08-12  4:47   ` Paul Burton
  2019-08-12  3:31 ` [PATCH 2/5] MIPS/ptrace: Update mips_get_syscall_arg's return type Nathan Chancellor
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  3:31 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Nathan Chancellor

clang warns:

arch/mips/kernel/branch.c:148:8: error: variable 'bc_false' is used
uninitialized whenever switch case is taken
[-Werror,-Wsometimes-uninitialized]
                case mm_bc2t_op:
                     ^~~~~~~~~~
arch/mips/kernel/branch.c:157:8: note: uninitialized use occurs here
                        if (bc_false)
                            ^~~~~~~~
arch/mips/kernel/branch.c:149:8: error: variable 'bc_false' is used
uninitialized whenever switch case is taken
[-Werror,-Wsometimes-uninitialized]
                case mm_bc1t_op:
                     ^~~~~~~~~~
arch/mips/kernel/branch.c:157:8: note: uninitialized use occurs here
                        if (bc_false)
                            ^~~~~~~~
arch/mips/kernel/branch.c:142:4: note: variable 'bc_false' is declared
here
                        int bc_false = 0;
                        ^
2 errors generated.

When mm_bc1t_op and mm_bc2t_op are taken, the bc_false initialization
does not happen, which leads to a garbage value upon use, as illustrated
below with a small sample program.

$ mipsel-linux-gnu-gcc --version | head -n1
mipsel-linux-gnu-gcc (Debian 8.3.0-2) 8.3.0

$ clang --version | head -n1
ClangBuiltLinux clang version 9.0.0 (git://github.com/llvm/llvm-project
544315b4197034a3be8acd12cba56a75fb1f08dc) (based on LLVM 9.0.0svn)

$ cat test.c
 #include <stdio.h>

 static void switch_scoped(int opcode)
 {
	 switch (opcode) {
	 case 1:
	 case 2: {
		 int bc_false = 0;

		 bc_false = 4;
	 case 3:
	 case 4:
		 printf("\t* switch scoped bc_false = %d\n", bc_false);
	 }
	 }
 }

 static void function_scoped(int opcode)
 {
	 int bc_false = 0;

	 switch (opcode) {
	 case 1:
	 case 2: {
		 bc_false = 4;
	 case 3:
	 case 4:
		 printf("\t* function scoped bc_false = %d\n", bc_false);
	 }
	 }
 }

 int main(void)
 {
	 int opcode;

	 for (opcode = 1; opcode < 5; opcode++) {
		 printf("opcode = %d:\n", opcode);
		 switch_scoped(opcode);
		 function_scoped(opcode);
		 printf("\n");
	 }

	 return 0;
 }

$ mipsel-linux-gnu-gcc -std=gnu89 -static test.c && \
  qemu-mipsel a.out
opcode = 1:
        * switch scoped bc_false = 4
        * function scoped bc_false = 4

opcode = 2:
        * switch scoped bc_false = 4
        * function scoped bc_false = 4

opcode = 3:
        * switch scoped bc_false = 2147483004
        * function scoped bc_false = 0

opcode = 4:
        * switch scoped bc_false = 2147483004
        * function scoped bc_false = 0

$ clang -std=gnu89 --target=mipsel-linux-gnu -m32 -static test.c && \
  qemu-mipsel a.out
opcode = 1:
        * switch scoped bc_false = 4
        * function scoped bc_false = 4

opcode = 2:
        * switch scoped bc_false = 4
        * function scoped bc_false = 4

opcode = 3:
        * switch scoped bc_false = 2147483004
        * function scoped bc_false = 0

opcode = 4:
        * switch scoped bc_false = 2147483004
        * function scoped bc_false = 0

Move the definition up so that we get the right behavior and mark it
__maybe_unused as it will not be used when CONFIG_MIPS_FP_SUPPORT
isn't enabled.

Fixes: 6a1cc218b9cc ("MIPS: branch: Remove FP branch handling when CONFIG_MIPS_FP_SUPPORT=n")
Link: https://github.com/ClangBuiltLinux/linux/issues/603
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 arch/mips/kernel/branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/branch.c b/arch/mips/kernel/branch.c
index 1db29957a931..2c38f75d87ff 100644
--- a/arch/mips/kernel/branch.c
+++ b/arch/mips/kernel/branch.c
@@ -58,6 +58,7 @@ int __mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
 		       unsigned long *contpc)
 {
 	union mips_instruction insn = (union mips_instruction)dec_insn.insn;
+	int __maybe_unused bc_false = 0;
 
 	if (!cpu_has_mmips)
 		return 0;
@@ -139,7 +140,6 @@ int __mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
 #ifdef CONFIG_MIPS_FP_SUPPORT
 		case mm_bc2f_op:
 		case mm_bc1f_op: {
-			int bc_false = 0;
 			unsigned int fcr31;
 			unsigned int bit;
 
-- 
2.23.0.rc2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/5] MIPS/ptrace: Update mips_get_syscall_arg's return type
  2019-08-12  3:31 [PATCH 0/5] Clang build fixes for MIPS Nathan Chancellor
  2019-08-12  3:31 ` [PATCH 1/5] MIPS: Don't use bc_false uninitialized in __mm_isBranchInstr Nathan Chancellor
@ 2019-08-12  3:31 ` Nathan Chancellor
  2019-08-12  4:47   ` Paul Burton
  2019-08-12  3:31 ` [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang Nathan Chancellor
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  3:31 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Nathan Chancellor

clang warns:

arch/mips/include/asm/syscall.h:136:3: error: variable 'ret' is
uninitialized when used here [-Werror,-Wuninitialized]
                ret |= mips_get_syscall_arg(args++, task, regs, i++);
                ^~~
arch/mips/include/asm/syscall.h:129:9: note: initialize the variable
'ret' to silence this warning
        int ret;
               ^
                = 0
1 error generated.

It's not wrong; however, it's not an issue in practice because ret is
only assigned to, not read from. ret could just be initialized to zero
but looking into it further, ret has been unused since it was first
added in 2012 so just get rid of it and update mips_get_syscall_arg's
return type since none of the return values are ever checked. If it is
ever needed again, this commit can be reverted and ret can be properly
initialized.

Fixes: c0ff3c53d4f9 ("MIPS: Enable HAVE_ARCH_TRACEHOOK.")
Link: https://github.com/ClangBuiltLinux/linux/issues/604
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 arch/mips/include/asm/syscall.h | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
index 83bb439597d8..25fa651c937d 100644
--- a/arch/mips/include/asm/syscall.h
+++ b/arch/mips/include/asm/syscall.h
@@ -54,7 +54,7 @@ static inline void mips_syscall_update_nr(struct task_struct *task,
 		task_thread_info(task)->syscall = regs->regs[2];
 }
 
-static inline unsigned long mips_get_syscall_arg(unsigned long *arg,
+static inline void mips_get_syscall_arg(unsigned long *arg,
 	struct task_struct *task, struct pt_regs *regs, unsigned int n)
 {
 	unsigned long usp __maybe_unused = regs->regs[29];
@@ -63,23 +63,24 @@ static inline unsigned long mips_get_syscall_arg(unsigned long *arg,
 	case 0: case 1: case 2: case 3:
 		*arg = regs->regs[4 + n];
 
-		return 0;
+		return;
 
 #ifdef CONFIG_32BIT
 	case 4: case 5: case 6: case 7:
-		return get_user(*arg, (int *)usp + n);
+		get_user(*arg, (int *)usp + n);
+		return;
 #endif
 
 #ifdef CONFIG_64BIT
 	case 4: case 5: case 6: case 7:
 #ifdef CONFIG_MIPS32_O32
 		if (test_tsk_thread_flag(task, TIF_32BIT_REGS))
-			return get_user(*arg, (int *)usp + n);
+			get_user(*arg, (int *)usp + n);
 		else
 #endif
 			*arg = regs->regs[4 + n];
 
-		return 0;
+		return;
 #endif
 
 	default:
@@ -126,21 +127,13 @@ static inline void syscall_get_arguments(struct task_struct *task,
 {
 	unsigned int i = 0;
 	unsigned int n = 6;
-	int ret;
 
 	/* O32 ABI syscall() */
 	if (mips_syscall_is_indirect(task, regs))
 		i++;
 
 	while (n--)
-		ret |= mips_get_syscall_arg(args++, task, regs, i++);
-
-	/*
-	 * No way to communicate an error because this is a void function.
-	 */
-#if 0
-	return ret;
-#endif
+		mips_get_syscall_arg(args++, task, regs, i++);
 }
 
 extern const unsigned long sys_call_table[];
-- 
2.23.0.rc2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12  3:31 [PATCH 0/5] Clang build fixes for MIPS Nathan Chancellor
  2019-08-12  3:31 ` [PATCH 1/5] MIPS: Don't use bc_false uninitialized in __mm_isBranchInstr Nathan Chancellor
  2019-08-12  3:31 ` [PATCH 2/5] MIPS/ptrace: Update mips_get_syscall_arg's return type Nathan Chancellor
@ 2019-08-12  3:31 ` Nathan Chancellor
  2019-08-12  5:23   ` Nathan Chancellor
  2019-08-12  7:35   ` Jussi Kivilinna
  2019-08-12  3:31 ` [PATCH 4/5] lib/mpi: Fix for building for MIPS64 " Nathan Chancellor
  2019-08-12  3:31 ` [PATCH 5/5] MIPS: tlbex: Explicitly cast _PAGE_NO_EXEC to a boolean Nathan Chancellor
  4 siblings, 2 replies; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  3:31 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Vladimir Serbinenko, Jussi Kivilinna, Nathan Chancellor

From: Vladimir Serbinenko <phcoder@gmail.com>

clang doesn't recognise =l / =h assembly operand specifiers but apparently
handles C version well.

lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
inline asm context requiring an l-value: remove the cast or build with
-fheinous-gnu-extensions
                umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
                ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
        : "=l" ((USItype)(w0)), \
                ~~~~~~~~~~^~~
lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
in asm
                umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
                ^
lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
             "=h" ((USItype)(w1)) \
             ^
2 errors generated.

Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
Link: https://github.com/ClangBuiltLinux/linux/issues/605
Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89
Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
[jk: add changelog, rebase on libgcrypt repository, reformat changed
 line so it does not go over 80 characters]
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
[nc: Added build error and tags to commit message
     Added Vladimir's signoff with his permission
     Adjusted Jussi's comment to wrap at 73 characters
     Modified commit subject to mirror MIPS64 commit
     Removed space between defined and (__clang__)]
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 lib/mpi/longlong.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
index 3bb6260d8f42..8a1507fc94dd 100644
--- a/lib/mpi/longlong.h
+++ b/lib/mpi/longlong.h
@@ -639,7 +639,8 @@ do { \
 	**************  MIPS  *****************
 	***************************************/
 #if defined(__mips__) && W_TYPE_SIZE == 32
-#if (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4)
+#if defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \
+					      __GNUC_MINOR__ >= 4)
 #define umul_ppmm(w1, w0, u, v)			\
 do {						\
 	UDItype __ll = (UDItype)(u) * (v);	\
-- 
2.23.0.rc2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 4/5] lib/mpi: Fix for building for MIPS64 with Clang
  2019-08-12  3:31 [PATCH 0/5] Clang build fixes for MIPS Nathan Chancellor
                   ` (2 preceding siblings ...)
  2019-08-12  3:31 ` [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang Nathan Chancellor
@ 2019-08-12  3:31 ` " Nathan Chancellor
  2019-08-12  5:25   ` Nathan Chancellor
  2019-08-12 17:42   ` Nick Desaulniers
  2019-08-12  3:31 ` [PATCH 5/5] MIPS: tlbex: Explicitly cast _PAGE_NO_EXEC to a boolean Nathan Chancellor
  4 siblings, 2 replies; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  3:31 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Werner Koch, Nathan Chancellor

From: Werner Koch <wk@gnupg.org>

* mpi/longlong.h [MIPS64][__clang__]: Use the C version like we
already do for 32 bit MIPS

clang errors:

lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
inline asm context requiring an l-value: remove the cast or build with
-fheinous-gnu-extensions
                umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
                ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
        : "=l" ((USItype)(w0)), \
                ~~~~~~~~~~^~~
lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
in asm
                umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
                ^
lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
             "=h" ((USItype)(w1)) \
             ^
2 errors generated.

Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
Link: https://github.com/ClangBuiltLinux/linux/issues/605
Link: https://github.com/gpg/libgcrypt/commit/e7ae0ae243c8978a67c802169183187d88557be8
Signed-off-by: Werner Koch <wk@gnupg.org>
[nc: Added build error and tags to commit message
     Modified subject line
     Removed GnuPG-bug-id
     Removed space between defined and (__clang__)]
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 lib/mpi/longlong.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
index 8a1507fc94dd..5636e6a09f7a 100644
--- a/lib/mpi/longlong.h
+++ b/lib/mpi/longlong.h
@@ -688,7 +688,8 @@ do {									\
 		 : "d" ((UDItype)(u)),					\
 		   "d" ((UDItype)(v)));					\
 } while (0)
-#elif (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4)
+#elif defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \
+						__GNUC_MINOR__ >= 4)
 #define umul_ppmm(w1, w0, u, v) \
 do {									\
 	typedef unsigned int __ll_UTItype __attribute__((mode(TI)));	\
-- 
2.23.0.rc2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 5/5] MIPS: tlbex: Explicitly cast _PAGE_NO_EXEC to a boolean
  2019-08-12  3:31 [PATCH 0/5] Clang build fixes for MIPS Nathan Chancellor
                   ` (3 preceding siblings ...)
  2019-08-12  3:31 ` [PATCH 4/5] lib/mpi: Fix for building for MIPS64 " Nathan Chancellor
@ 2019-08-12  3:31 ` Nathan Chancellor
  2019-08-12  4:47   ` Paul Burton
  4 siblings, 1 reply; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  3:31 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Nathan Chancellor

clang warns:

arch/mips/mm/tlbex.c:634:19: error: use of logical '&&' with constant
operand [-Werror,-Wconstant-logical-operand]
        if (cpu_has_rixi && _PAGE_NO_EXEC) {
                         ^  ~~~~~~~~~~~~~
arch/mips/mm/tlbex.c:634:19: note: use '&' for a bitwise operation
        if (cpu_has_rixi && _PAGE_NO_EXEC) {
                         ^~
                         &
arch/mips/mm/tlbex.c:634:19: note: remove constant to silence this
warning
        if (cpu_has_rixi && _PAGE_NO_EXEC) {
                        ~^~~~~~~~~~~~~~~~
1 error generated.

Explicitly cast this value to a boolean so that clang understands we
intend for this to be a non-zero value.

Fixes: 00bf1c691d08 ("MIPS: tlbex: Avoid placing software PTE bits in Entry* PFN fields")
Link: https://github.com/ClangBuiltLinux/linux/issues/609
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 arch/mips/mm/tlbex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index eb21277f4141..071d48593464 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -629,7 +629,7 @@ static __maybe_unused void build_convert_pte_to_entrylo(u32 **p,
 		return;
 	}
 
-	if (cpu_has_rixi && _PAGE_NO_EXEC) {
+	if (cpu_has_rixi && !!_PAGE_NO_EXEC) {
 		if (fill_includes_sw_bits) {
 			UASM_i_ROTR(p, reg, reg, ilog2(_PAGE_GLOBAL));
 		} else {
-- 
2.23.0.rc2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] MIPS: Don't use bc_false uninitialized in __mm_isBranchInstr
  2019-08-12  3:31 ` [PATCH 1/5] MIPS: Don't use bc_false uninitialized in __mm_isBranchInstr Nathan Chancellor
@ 2019-08-12  4:47   ` Paul Burton
  0 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2019-08-12  4:47 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ralf Baechle, Paul Burton, James Hogan, Nick Desaulniers,
	linux-mips, linux-kernel, clang-built-linux, Nathan Chancellor,
	linux-mips

Hello,

Nathan Chancellor wrote:
> clang warns:
> 
> arch/mips/kernel/branch.c:148:8: error: variable 'bc_false' is used
> uninitialized whenever switch case is taken
> [-Werror,-Wsometimes-uninitialized]
>                 case mm_bc2t_op:
>                      ^~~~~~~~~~
> arch/mips/kernel/branch.c:157:8: note: uninitialized use occurs here
>                         if (bc_false)
>                             ^~~~~~~~
> arch/mips/kernel/branch.c:149:8: error: variable 'bc_false' is used
> uninitialized whenever switch case is taken
> [-Werror,-Wsometimes-uninitialized]
>                 case mm_bc1t_op:
>                      ^~~~~~~~~~
> arch/mips/kernel/branch.c:157:8: note: uninitialized use occurs here
>                         if (bc_false)
>                             ^~~~~~~~
> arch/mips/kernel/branch.c:142:4: note: variable 'bc_false' is declared
> here
>                         int bc_false = 0;
>                         ^
> 2 errors generated.
> 
> When mm_bc1t_op and mm_bc2t_op are taken, the bc_false initialization
> does not happen, which leads to a garbage value upon use, as illustrated
> below with a small sample program.
> 
> $ mipsel-linux-gnu-gcc --version | head -n1
> mipsel-linux-gnu-gcc (Debian 8.3.0-2) 8.3.0
> 
> $ clang --version | head -n1
> ClangBuiltLinux clang version 9.0.0 (git://github.com/llvm/llvm-project
> 544315b4197034a3be8acd12cba56a75fb1f08dc) (based on LLVM 9.0.0svn)
> 
> $ cat test.c
>  #include <stdio.h>
> 
>  static void switch_scoped(int opcode)
>  {
> 	 switch (opcode) {
> 	 case 1:
> 	 case 2: {
> 		 int bc_false = 0;
> 
> 		 bc_false = 4;
> 	 case 3:
> 	 case 4:
> 		 printf("\t* switch scoped bc_false = %d\n", bc_false);
> 	 }
> 	 }
>  }
> 
>  static void function_scoped(int opcode)
>  {
> 	 int bc_false = 0;
> 
> 	 switch (opcode) {
> 	 case 1:
> 	 case 2: {
> 		 bc_false = 4;
> 	 case 3:
> 	 case 4:
> 		 printf("\t* function scoped bc_false = %d\n", bc_false);
> 	 }
> 	 }
>  }
> 
>  int main(void)
>  {
> 	 int opcode;
> 
> 	 for (opcode = 1; opcode < 5; opcode++) {
> 		 printf("opcode = %d:\n", opcode);
> 		 switch_scoped(opcode);
> 		 function_scoped(opcode);
> 		 printf("\n");
> 	 }
> 
> 	 return 0;
>  }
> 
> $ mipsel-linux-gnu-gcc -std=gnu89 -static test.c && \
>   qemu-mipsel a.out
> opcode = 1:
>         * switch scoped bc_false = 4
>         * function scoped bc_false = 4
> 
> opcode = 2:
>         * switch scoped bc_false = 4
>         * function scoped bc_false = 4
> 
> opcode = 3:
>         * switch scoped bc_false = 2147483004
>         * function scoped bc_false = 0
> 
> opcode = 4:
>         * switch scoped bc_false = 2147483004
>         * function scoped bc_false = 0
> 
> $ clang -std=gnu89 --target=mipsel-linux-gnu -m32 -static test.c && \
>   qemu-mipsel a.out
> opcode = 1:
>         * switch scoped bc_false = 4
>         * function scoped bc_false = 4
> 
> opcode = 2:
>         * switch scoped bc_false = 4
>         * function scoped bc_false = 4
> 
> opcode = 3:
>         * switch scoped bc_false = 2147483004
>         * function scoped bc_false = 0
> 
> opcode = 4:
>         * switch scoped bc_false = 2147483004
>         * function scoped bc_false = 0
> 
> Move the definition up so that we get the right behavior and mark it
> __maybe_unused as it will not be used when CONFIG_MIPS_FP_SUPPORT
> isn't enabled.

Applied to mips-next.

> commit c2869aafe719
> https://git.kernel.org/mips/c/c2869aafe719
> 
> Fixes: 6a1cc218b9cc ("MIPS: branch: Remove FP branch handling when CONFIG_MIPS_FP_SUPPORT=n")
> Link: https://github.com/ClangBuiltLinux/linux/issues/603
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Paul Burton <paul.burton@mips.com>

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] MIPS/ptrace: Update mips_get_syscall_arg's return type
  2019-08-12  3:31 ` [PATCH 2/5] MIPS/ptrace: Update mips_get_syscall_arg's return type Nathan Chancellor
@ 2019-08-12  4:47   ` Paul Burton
  0 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2019-08-12  4:47 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ralf Baechle, Paul Burton, James Hogan, Nick Desaulniers,
	linux-mips, linux-kernel, clang-built-linux, Nathan Chancellor,
	linux-mips

Hello,

Nathan Chancellor wrote:
> clang warns:
> 
> arch/mips/include/asm/syscall.h:136:3: error: variable 'ret' is
> uninitialized when used here [-Werror,-Wuninitialized]
>                 ret |= mips_get_syscall_arg(args++, task, regs, i++);
>                 ^~~
> arch/mips/include/asm/syscall.h:129:9: note: initialize the variable
> 'ret' to silence this warning
>         int ret;
>                ^
>                 = 0
> 1 error generated.
> 
> It's not wrong; however, it's not an issue in practice because ret is
> only assigned to, not read from. ret could just be initialized to zero
> but looking into it further, ret has been unused since it was first
> added in 2012 so just get rid of it and update mips_get_syscall_arg's
> return type since none of the return values are ever checked. If it is
> ever needed again, this commit can be reverted and ret can be properly
> initialized.

Applied to mips-next.

> commit 077ff3be06e8
> https://git.kernel.org/mips/c/077ff3be06e8
> 
> Fixes: c0ff3c53d4f9 ("MIPS: Enable HAVE_ARCH_TRACEHOOK.")
> Link: https://github.com/ClangBuiltLinux/linux/issues/604
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Paul Burton <paul.burton@mips.com>

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] MIPS: tlbex: Explicitly cast _PAGE_NO_EXEC to a boolean
  2019-08-12  3:31 ` [PATCH 5/5] MIPS: tlbex: Explicitly cast _PAGE_NO_EXEC to a boolean Nathan Chancellor
@ 2019-08-12  4:47   ` Paul Burton
  0 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2019-08-12  4:47 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ralf Baechle, Paul Burton, James Hogan, Nick Desaulniers,
	linux-mips, linux-kernel, clang-built-linux, Nathan Chancellor,
	linux-mips

Hello,

Nathan Chancellor wrote:
> clang warns:
> 
> arch/mips/mm/tlbex.c:634:19: error: use of logical '&&' with constant
> operand [-Werror,-Wconstant-logical-operand]
>         if (cpu_has_rixi && _PAGE_NO_EXEC) {
>                          ^  ~~~~~~~~~~~~~
> arch/mips/mm/tlbex.c:634:19: note: use '&' for a bitwise operation
>         if (cpu_has_rixi && _PAGE_NO_EXEC) {
>                          ^~
>                          &
> arch/mips/mm/tlbex.c:634:19: note: remove constant to silence this
> warning
>         if (cpu_has_rixi && _PAGE_NO_EXEC) {
>                         ~^~~~~~~~~~~~~~~~
> 1 error generated.
> 
> Explicitly cast this value to a boolean so that clang understands we
> intend for this to be a non-zero value.
> 
> Fixes: 00bf1c691d08 ("MIPS: tlbex: Avoid placing software PTE bits in Entry* PFN fields")

Applied to mips-next.

> commit c59ae0a10551
> https://git.kernel.org/mips/c/c59ae0a10551
> 
> Fixes: 00bf1c691d08 ("MIPS: tlbex: Avoid placing software PTE bits in Entry* PFN fields")
> Link: https://github.com/ClangBuiltLinux/linux/issues/609
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Paul Burton <paul.burton@mips.com>

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12  3:31 ` [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang Nathan Chancellor
@ 2019-08-12  5:23   ` Nathan Chancellor
  2019-08-12  5:26     ` Nathan Chancellor
  2019-08-12 17:58     ` Paul Burton
  2019-08-12  7:35   ` Jussi Kivilinna
  1 sibling, 2 replies; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  5:23 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Vladimir Serbinenko, Jussi Kivilinna

On Sun, Aug 11, 2019 at 08:31:18PM -0700, Nathan Chancellor wrote:
> From: Vladimir Serbinenko <phcoder@gmail.com>
> 
> clang doesn't recognise =l / =h assembly operand specifiers but apparently
> handles C version well.
> 
> lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
> inline asm context requiring an l-value: remove the cast or build with
> -fheinous-gnu-extensions
>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
>         : "=l" ((USItype)(w0)), \
>                 ~~~~~~~~~~^~~
> lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
> in asm
>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>                 ^
> lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
>              "=h" ((USItype)(w1)) \
>              ^
> 2 errors generated.
> 
> Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
> Link: https://github.com/ClangBuiltLinux/linux/issues/605
> Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89
> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> [jk: add changelog, rebase on libgcrypt repository, reformat changed
>  line so it does not go over 80 characters]
> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> [nc: Added build error and tags to commit message
>      Added Vladimir's signoff with his permission
>      Adjusted Jussi's comment to wrap at 73 characters
>      Modified commit subject to mirror MIPS64 commit
>      Removed space between defined and (__clang__)]
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  lib/mpi/longlong.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
> index 3bb6260d8f42..8a1507fc94dd 100644
> --- a/lib/mpi/longlong.h
> +++ b/lib/mpi/longlong.h
> @@ -639,7 +639,8 @@ do { \
>  	**************  MIPS  *****************
>  	***************************************/
>  #if defined(__mips__) && W_TYPE_SIZE == 32
> -#if (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4)
> +#if defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \
> +					      __GNUC_MINOR__ >= 4)
>  #define umul_ppmm(w1, w0, u, v)			\
>  do {						\
>  	UDItype __ll = (UDItype)(u) * (v);	\
> -- 
> 2.23.0.rc2
> 

 Hi Paul,

 I noticed you didn't pick up this patch with the other ones you
 applied. I just wanted to make sure it wasn't because it was sent to
 the wrong person. This set of files doesn't appear to have an owner in
 MAINTAINERS, I guess based on git history either Andrew or Hubert (on
 CC) pick up patches for this set of files? If I need to, I can resend
 it to the proper people.

 Cheers,
 Nathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] lib/mpi: Fix for building for MIPS64 with Clang
  2019-08-12  3:31 ` [PATCH 4/5] lib/mpi: Fix for building for MIPS64 " Nathan Chancellor
@ 2019-08-12  5:25   ` Nathan Chancellor
  2019-08-12 17:42   ` Nick Desaulniers
  1 sibling, 0 replies; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  5:25 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Werner Koch, Andrew Morton, Herbert Xu

On Sun, Aug 11, 2019 at 08:31:19PM -0700, Nathan Chancellor wrote:
> From: Werner Koch <wk@gnupg.org>
> 
> * mpi/longlong.h [MIPS64][__clang__]: Use the C version like we
> already do for 32 bit MIPS
> 
> clang errors:
> 
> lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
> inline asm context requiring an l-value: remove the cast or build with
> -fheinous-gnu-extensions
>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
>         : "=l" ((USItype)(w0)), \
>                 ~~~~~~~~~~^~~
> lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
> in asm
>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>                 ^
> lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
>              "=h" ((USItype)(w1)) \
>              ^
> 2 errors generated.
> 
> Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
> Link: https://github.com/ClangBuiltLinux/linux/issues/605
> Link: https://github.com/gpg/libgcrypt/commit/e7ae0ae243c8978a67c802169183187d88557be8
> Signed-off-by: Werner Koch <wk@gnupg.org>
> [nc: Added build error and tags to commit message
>      Modified subject line
>      Removed GnuPG-bug-id
>      Removed space between defined and (__clang__)]
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  lib/mpi/longlong.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
> index 8a1507fc94dd..5636e6a09f7a 100644
> --- a/lib/mpi/longlong.h
> +++ b/lib/mpi/longlong.h
> @@ -688,7 +688,8 @@ do {									\
>  		 : "d" ((UDItype)(u)),					\
>  		   "d" ((UDItype)(v)));					\
>  } while (0)
> -#elif (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4)
> +#elif defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \
> +						__GNUC_MINOR__ >= 4)
>  #define umul_ppmm(w1, w0, u, v) \
>  do {									\
>  	typedef unsigned int __ll_UTItype __attribute__((mode(TI)));	\
> -- 
> 2.23.0.rc2
> 

Hi Paul,

I noticed you didn't pick up this patch with the other ones you applied.
I just wanted to make sure it wasn't because it was sent to the wrong
person. This set of files doesn't appear to have an owner in
MAINTAINERS, I guess based on git history either Andrew or Hubert (on
CC) pick up patches for this set of files? If I need to, I can resend it
to the proper people.

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12  5:23   ` Nathan Chancellor
@ 2019-08-12  5:26     ` Nathan Chancellor
  2019-08-12 12:28       ` Herbert Xu
  2019-08-12 17:58     ` Paul Burton
  1 sibling, 1 reply; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12  5:26 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Vladimir Serbinenko, Jussi Kivilinna, Andrew Morton, Herbert Xu

On Sun, Aug 11, 2019 at 10:23:55PM -0700, Nathan Chancellor wrote:
> On Sun, Aug 11, 2019 at 08:31:18PM -0700, Nathan Chancellor wrote:
> > From: Vladimir Serbinenko <phcoder@gmail.com>
> > 
> > clang doesn't recognise =l / =h assembly operand specifiers but apparently
> > handles C version well.
> > 
> > lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
> > inline asm context requiring an l-value: remove the cast or build with
> > -fheinous-gnu-extensions
> >                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
> >                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
> >         : "=l" ((USItype)(w0)), \
> >                 ~~~~~~~~~~^~~
> > lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
> > in asm
> >                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
> >                 ^
> > lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
> >              "=h" ((USItype)(w1)) \
> >              ^
> > 2 errors generated.
> > 
> > Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/605
> > Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89
> > Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> > [jk: add changelog, rebase on libgcrypt repository, reformat changed
> >  line so it does not go over 80 characters]
> > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> > [nc: Added build error and tags to commit message
> >      Added Vladimir's signoff with his permission
> >      Adjusted Jussi's comment to wrap at 73 characters
> >      Modified commit subject to mirror MIPS64 commit
> >      Removed space between defined and (__clang__)]
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >  lib/mpi/longlong.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
> > index 3bb6260d8f42..8a1507fc94dd 100644
> > --- a/lib/mpi/longlong.h
> > +++ b/lib/mpi/longlong.h
> > @@ -639,7 +639,8 @@ do { \
> >  	**************  MIPS  *****************
> >  	***************************************/
> >  #if defined(__mips__) && W_TYPE_SIZE == 32
> > -#if (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4)
> > +#if defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \
> > +					      __GNUC_MINOR__ >= 4)
> >  #define umul_ppmm(w1, w0, u, v)			\
> >  do {						\
> >  	UDItype __ll = (UDItype)(u) * (v);	\
> > -- 
> > 2.23.0.rc2
> > 
> 
>  Hi Paul,
> 
>  I noticed you didn't pick up this patch with the other ones you
>  applied. I just wanted to make sure it wasn't because it was sent to
>  the wrong person. This set of files doesn't appear to have an owner in
>  MAINTAINERS, I guess based on git history either Andrew or Hubert (on
>  CC) pick up patches for this set of files? If I need to, I can resend
>  it to the proper people.
> 
>  Cheers,
>  Nathan

Sigh, actually add Andrew and Herbert and get his name right, sorry :(

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12  3:31 ` [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang Nathan Chancellor
  2019-08-12  5:23   ` Nathan Chancellor
@ 2019-08-12  7:35   ` Jussi Kivilinna
  2019-08-12 17:14     ` Nathan Chancellor
  1 sibling, 1 reply; 20+ messages in thread
From: Jussi Kivilinna @ 2019-08-12  7:35 UTC (permalink / raw)
  To: Nathan Chancellor, Ralf Baechle, Paul Burton, James Hogan
  Cc: Nick Desaulniers, linux-mips, linux-kernel, clang-built-linux,
	Vladimir Serbinenko

Hello,

On 12.8.2019 6.31, Nathan Chancellor wrote:
> From: Vladimir Serbinenko <phcoder@gmail.com>
> 
> clang doesn't recognise =l / =h assembly operand specifiers but apparently
> handles C version well.
> 
> lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
> inline asm context requiring an l-value: remove the cast or build with
> -fheinous-gnu-extensions
>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
>         : "=l" ((USItype)(w0)), \
>                 ~~~~~~~~~~^~~
> lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
> in asm
>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>                 ^
> lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
>              "=h" ((USItype)(w1)) \
>              ^
> 2 errors generated.
> 
> Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
> Link: https://github.com/ClangBuiltLinux/linux/issues/605
> Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89
> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> [jk: add changelog, rebase on libgcrypt repository, reformat changed
>  line so it does not go over 80 characters]
> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

This is my signed-off-by for libgcrypt project, not kernel. I do not think
signed-offs can be passed from other projects in this way.

-Jussi

> [nc: Added build error and tags to commit message
>      Added Vladimir's signoff with his permission
>      Adjusted Jussi's comment to wrap at 73 characters
>      Modified commit subject to mirror MIPS64 commit
>      Removed space between defined and (__clang__)]
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  lib/mpi/longlong.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
> index 3bb6260d8f42..8a1507fc94dd 100644
> --- a/lib/mpi/longlong.h
> +++ b/lib/mpi/longlong.h
> @@ -639,7 +639,8 @@ do { \
>  	**************  MIPS  *****************
>  	***************************************/
>  #if defined(__mips__) && W_TYPE_SIZE == 32
> -#if (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4)
> +#if defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \
> +					      __GNUC_MINOR__ >= 4)
>  #define umul_ppmm(w1, w0, u, v)			\
>  do {						\
>  	UDItype __ll = (UDItype)(u) * (v);	\
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12  5:26     ` Nathan Chancellor
@ 2019-08-12 12:28       ` Herbert Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Herbert Xu @ 2019-08-12 12:28 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ralf Baechle, Paul Burton, James Hogan, Nick Desaulniers,
	linux-mips, linux-kernel, clang-built-linux, Vladimir Serbinenko,
	Jussi Kivilinna, Andrew Morton

On Sun, Aug 11, 2019 at 10:26:53PM -0700, Nathan Chancellor wrote:
>
> >  I noticed you didn't pick up this patch with the other ones you
> >  applied. I just wanted to make sure it wasn't because it was sent to
> >  the wrong person. This set of files doesn't appear to have an owner in
> >  MAINTAINERS, I guess based on git history either Andrew or Hubert (on
> >  CC) pick up patches for this set of files? If I need to, I can resend
> >  it to the proper people.
> > 
> >  Cheers,
> >  Nathan
> 
> Sigh, actually add Andrew and Herbert and get his name right, sorry :(

You can route it through the crypto tree by posting this to the
linux-crypto@vger.kernel.org mailing list.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12  7:35   ` Jussi Kivilinna
@ 2019-08-12 17:14     ` Nathan Chancellor
  2019-08-12 19:40       ` Jussi Kivilinna
  0 siblings, 1 reply; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12 17:14 UTC (permalink / raw)
  To: Jussi Kivilinna
  Cc: Ralf Baechle, Paul Burton, James Hogan, Nick Desaulniers,
	linux-mips, linux-kernel, clang-built-linux, Vladimir Serbinenko

On Mon, Aug 12, 2019 at 10:35:53AM +0300, Jussi Kivilinna wrote:
> Hello,
> 
> On 12.8.2019 6.31, Nathan Chancellor wrote:
> > From: Vladimir Serbinenko <phcoder@gmail.com>
> > 
> > clang doesn't recognise =l / =h assembly operand specifiers but apparently
> > handles C version well.
> > 
> > lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
> > inline asm context requiring an l-value: remove the cast or build with
> > -fheinous-gnu-extensions
> >                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
> >                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
> >         : "=l" ((USItype)(w0)), \
> >                 ~~~~~~~~~~^~~
> > lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
> > in asm
> >                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
> >                 ^
> > lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
> >              "=h" ((USItype)(w1)) \
> >              ^
> > 2 errors generated.
> > 
> > Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/605
> > Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89
> > Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> > [jk: add changelog, rebase on libgcrypt repository, reformat changed
> >  line so it does not go over 80 characters]
> > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> 
> This is my signed-off-by for libgcrypt project, not kernel. I do not think
> signed-offs can be passed from other projects in this way.
> 
> -Jussi

Hi Jussi,

I am no signoff expert but if I am reading the developer certificate of
origin in the libgcrypt repo correctly [1], your signoff on this commit
falls under:

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

This file is maintained under the LGPL because it was taken straight
from the libgcrypr repo and per (b), I can submit this commit here
with everything intact.

However, I don't want to upset you in any way though so if you are not
comfortable with that, I suppose I can remove it as if Vladimir
submitted this fix to me directly (as I got permission for his signoff).
I need to resubmit this fix to an appropriate maintainer so let me know
what you think.

[1]: https://github.com/gpg/libgcrypt/blob/3bb858551cd5d84e43b800edfa2b07d1529718a9/doc/DCO

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] lib/mpi: Fix for building for MIPS64 with Clang
  2019-08-12  3:31 ` [PATCH 4/5] lib/mpi: Fix for building for MIPS64 " Nathan Chancellor
  2019-08-12  5:25   ` Nathan Chancellor
@ 2019-08-12 17:42   ` Nick Desaulniers
  2019-08-12 17:45     ` Nathan Chancellor
  1 sibling, 1 reply; 20+ messages in thread
From: Nick Desaulniers @ 2019-08-12 17:42 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ralf Baechle, Paul Burton, James Hogan, linux-mips, LKML,
	clang-built-linux, Werner Koch

On Sun, Aug 11, 2019 at 8:31 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> From: Werner Koch <wk@gnupg.org>
>
> * mpi/longlong.h [MIPS64][__clang__]: Use the C version like we
> already do for 32 bit MIPS
>
> clang errors:
>
> lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
> inline asm context requiring an l-value: remove the cast or build with
> -fheinous-gnu-extensions

Reminds me of:
https://github.com/ClangBuiltLinux/linux/commit/7b7c1df2883dd4393592859758c3e76207da8b1d

>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
>         : "=l" ((USItype)(w0)), \
>                 ~~~~~~~~~~^~~
> lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
> in asm
>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>                 ^
> lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
>              "=h" ((USItype)(w1)) \
>              ^
> 2 errors generated.
>
> Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
> Link: https://github.com/ClangBuiltLinux/linux/issues/605
> Link: https://github.com/gpg/libgcrypt/commit/e7ae0ae243c8978a67c802169183187d88557be8
> Signed-off-by: Werner Koch <wk@gnupg.org>
> [nc: Added build error and tags to commit message
>      Modified subject line
>      Removed GnuPG-bug-id
>      Removed space between defined and (__clang__)]
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  lib/mpi/longlong.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
> index 8a1507fc94dd..5636e6a09f7a 100644
> --- a/lib/mpi/longlong.h
> +++ b/lib/mpi/longlong.h
> @@ -688,7 +688,8 @@ do {                                                                        \
>                  : "d" ((UDItype)(u)),                                  \
>                    "d" ((UDItype)(v)));                                 \
>  } while (0)
> -#elif (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4)
> +#elif defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \
> +                                               __GNUC_MINOR__ >= 4)

So the minimum supported version of GCC to build the kernel is
currently 4.6, so we can clean up a lot more here.  I'd remove the
check, and delete the #elif and #else implementations.

>  #define umul_ppmm(w1, w0, u, v) \
>  do {                                                                   \
>         typedef unsigned int __ll_UTItype __attribute__((mode(TI)));    \
> --
> 2.23.0.rc2
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] lib/mpi: Fix for building for MIPS64 with Clang
  2019-08-12 17:42   ` Nick Desaulniers
@ 2019-08-12 17:45     ` Nathan Chancellor
  0 siblings, 0 replies; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12 17:45 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Ralf Baechle, Paul Burton, James Hogan, linux-mips, LKML,
	clang-built-linux, Werner Koch

On Mon, Aug 12, 2019 at 10:42:31AM -0700, Nick Desaulniers wrote:
> On Sun, Aug 11, 2019 at 8:31 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > From: Werner Koch <wk@gnupg.org>
> >
> > * mpi/longlong.h [MIPS64][__clang__]: Use the C version like we
> > already do for 32 bit MIPS
> >
> > clang errors:
> >
> > lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
> > inline asm context requiring an l-value: remove the cast or build with
> > -fheinous-gnu-extensions
> 
> Reminds me of:
> https://github.com/ClangBuiltLinux/linux/commit/7b7c1df2883dd4393592859758c3e76207da8b1d
> 
> >                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
> >                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
> >         : "=l" ((USItype)(w0)), \
> >                 ~~~~~~~~~~^~~
> > lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
> > in asm
> >                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
> >                 ^
> > lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
> >              "=h" ((USItype)(w1)) \
> >              ^
> > 2 errors generated.
> >
> > Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/605
> > Link: https://github.com/gpg/libgcrypt/commit/e7ae0ae243c8978a67c802169183187d88557be8
> > Signed-off-by: Werner Koch <wk@gnupg.org>
> > [nc: Added build error and tags to commit message
> >      Modified subject line
> >      Removed GnuPG-bug-id
> >      Removed space between defined and (__clang__)]
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >  lib/mpi/longlong.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
> > index 8a1507fc94dd..5636e6a09f7a 100644
> > --- a/lib/mpi/longlong.h
> > +++ b/lib/mpi/longlong.h
> > @@ -688,7 +688,8 @@ do {                                                                        \
> >                  : "d" ((UDItype)(u)),                                  \
> >                    "d" ((UDItype)(v)));                                 \
> >  } while (0)
> > -#elif (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4)
> > +#elif defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \
> > +                                               __GNUC_MINOR__ >= 4)
> 
> So the minimum supported version of GCC to build the kernel is
> currently 4.6, so we can clean up a lot more here.  I'd remove the
> check, and delete the #elif and #else implementations.
> 

Sure, I will dig into this further and send a v2.

Thanks for the review,
Nathan

> >  #define umul_ppmm(w1, w0, u, v) \
> >  do {                                                                   \
> >         typedef unsigned int __ll_UTItype __attribute__((mode(TI)));    \
> > --
> > 2.23.0.rc2
> >
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12  5:23   ` Nathan Chancellor
  2019-08-12  5:26     ` Nathan Chancellor
@ 2019-08-12 17:58     ` Paul Burton
  1 sibling, 0 replies; 20+ messages in thread
From: Paul Burton @ 2019-08-12 17:58 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ralf Baechle, James Hogan, Nick Desaulniers, linux-mips,
	linux-kernel, clang-built-linux, Vladimir Serbinenko,
	Jussi Kivilinna

Hi Nathan,

On Sun, Aug 11, 2019 at 10:23:55PM -0700, Nathan Chancellor wrote:
>  I noticed you didn't pick up this patch with the other ones you
>  applied. I just wanted to make sure it wasn't because it was sent to
>  the wrong person. This set of files doesn't appear to have an owner in
>  MAINTAINERS, I guess based on git history either Andrew or Hubert (on
>  CC) pick up patches for this set of files? If I need to, I can resend
>  it to the proper people.

The 3 arch/mips patches were trivial for me to apply because I'm very
familiar with the code & know they should go via the MIPS tree.

I'm far less familiar with lib/mpi & needed to look up maintainers,
which is why I didn't apply them in the few minutes I had spare.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12 17:14     ` Nathan Chancellor
@ 2019-08-12 19:40       ` Jussi Kivilinna
  2019-08-12 19:45         ` Nathan Chancellor
  0 siblings, 1 reply; 20+ messages in thread
From: Jussi Kivilinna @ 2019-08-12 19:40 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ralf Baechle, Paul Burton, James Hogan, Nick Desaulniers,
	linux-mips, linux-kernel, clang-built-linux, Vladimir Serbinenko,
	gcrypt-devel, Herbert Xu

Hello,

On 12.8.2019 20.14, Nathan Chancellor wrote:
> On Mon, Aug 12, 2019 at 10:35:53AM +0300, Jussi Kivilinna wrote:
>> Hello,
>>
>> On 12.8.2019 6.31, Nathan Chancellor wrote:
>>> From: Vladimir Serbinenko <phcoder@gmail.com>
>>>
>>> clang doesn't recognise =l / =h assembly operand specifiers but apparently
>>> handles C version well.
>>>
>>> lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a
>>> inline asm context requiring an l-value: remove the cast or build with
>>> -fheinous-gnu-extensions
>>>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>>>                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm'
>>>         : "=l" ((USItype)(w0)), \
>>>                 ~~~~~~~~~~^~~
>>> lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h'
>>> in asm
>>>                 umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb);
>>>                 ^
>>> lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm'
>>>              "=h" ((USItype)(w1)) \
>>>              ^
>>> 2 errors generated.
>>>
>>> Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)")
>>> Link: https://github.com/ClangBuiltLinux/linux/issues/605
>>> Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89
>>> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
>>> [jk: add changelog, rebase on libgcrypt repository, reformat changed
>>>  line so it does not go over 80 characters]
>>> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
>>
>> This is my signed-off-by for libgcrypt project, not kernel. I do not think
>> signed-offs can be passed from other projects in this way.
>>
>> -Jussi
> 
> Hi Jussi,
> 
> I am no signoff expert but if I am reading the developer certificate of
> origin in the libgcrypt repo correctly [1], your signoff on this commit
> falls under:
> 
> (d) I understand and agree that this project and the contribution
>     are public and that a record of the contribution (including all
>     personal information I submit with it, including my sign-off) is
>     maintained indefinitely and may be redistributed consistent with
>     this project or the open source license(s) involved.

There is nothing wrong with the commit in libgcrypt repo and/or my 
libgcrypt-DCO-sign-off.

> 
> This file is maintained under the LGPL because it was taken straight
> from the libgcrypr repo and per (b), I can submit this commit here
> with everything intact.

But you do not have my kernel-DCO-sign-off for this patch. I have not
been involved with this kernel patch in anyway, have not integrated 
it to kernel, not testing it on kernel.. I do not own it. However, 
with this signed-off-by line you have involved me to kernel patch 
process in which for this patch I'm not interested. So to be clear, 
I retract my kernel-DCO-signed-off for this kernel patch:

  NOT-Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

Of course you can copy the original libgcrypt commit message to this
patch, but I think it needs to be clearly quoted so that my 
libgcrypt-DCO-signed-off line wont be mixed with kernel-DOC-signed-off
lines. 

> 
> However, I don't want to upset you in any way though so if you are not
> comfortable with that, I suppose I can remove it as if Vladimir
> submitted this fix to me directly (as I got permission for his signoff).
> I need to resubmit this fix to an appropriate maintainer so let me know
> what you think.

That's quite complicated approach. Fast and easier process would be if you
just own the patch yourself. Libgcrypt (and target file in libgcrypt) 
is LGPL v2.1+, so the license is compatible with kernel and you are good 
to go with just your own (kernel DCO) signed-off-by.

-Jussi

> 
> [1]: https://github.com/gpg/libgcrypt/blob/3bb858551cd5d84e43b800edfa2b07d1529718a9/doc/DCO
> 
> Cheers,
> Nathan
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang
  2019-08-12 19:40       ` Jussi Kivilinna
@ 2019-08-12 19:45         ` Nathan Chancellor
  0 siblings, 0 replies; 20+ messages in thread
From: Nathan Chancellor @ 2019-08-12 19:45 UTC (permalink / raw)
  To: Jussi Kivilinna
  Cc: Ralf Baechle, Paul Burton, James Hogan, Nick Desaulniers,
	linux-mips, linux-kernel, clang-built-linux, Vladimir Serbinenko,
	gcrypt-devel, Herbert Xu

On Mon, Aug 12, 2019 at 10:40:49PM +0300, Jussi Kivilinna wrote:
> That's quite complicated approach. Fast and easier process would be if you
> just own the patch yourself. Libgcrypt (and target file in libgcrypt) 
> is LGPL v2.1+, so the license is compatible with kernel and you are good 
> to go with just your own (kernel DCO) signed-off-by.
> 
> -Jussi

I have gone this route as another developer pointed out that we can
eliminate all of the inline asm umul_ppmm definitions because the kernel
requires GCC 4.6 and newer and that is completely different from the
libgcrypt patches.

https://lore.kernel.org/lkml/20190812193256.55103-1-natechancellor@gmail.com/

Thanks for weighing in and cheers!
Nathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, back to index

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-12  3:31 [PATCH 0/5] Clang build fixes for MIPS Nathan Chancellor
2019-08-12  3:31 ` [PATCH 1/5] MIPS: Don't use bc_false uninitialized in __mm_isBranchInstr Nathan Chancellor
2019-08-12  4:47   ` Paul Burton
2019-08-12  3:31 ` [PATCH 2/5] MIPS/ptrace: Update mips_get_syscall_arg's return type Nathan Chancellor
2019-08-12  4:47   ` Paul Burton
2019-08-12  3:31 ` [PATCH 3/5] lib/mpi: Fix for building for MIPS32 with Clang Nathan Chancellor
2019-08-12  5:23   ` Nathan Chancellor
2019-08-12  5:26     ` Nathan Chancellor
2019-08-12 12:28       ` Herbert Xu
2019-08-12 17:58     ` Paul Burton
2019-08-12  7:35   ` Jussi Kivilinna
2019-08-12 17:14     ` Nathan Chancellor
2019-08-12 19:40       ` Jussi Kivilinna
2019-08-12 19:45         ` Nathan Chancellor
2019-08-12  3:31 ` [PATCH 4/5] lib/mpi: Fix for building for MIPS64 " Nathan Chancellor
2019-08-12  5:25   ` Nathan Chancellor
2019-08-12 17:42   ` Nick Desaulniers
2019-08-12 17:45     ` Nathan Chancellor
2019-08-12  3:31 ` [PATCH 5/5] MIPS: tlbex: Explicitly cast _PAGE_NO_EXEC to a boolean Nathan Chancellor
2019-08-12  4:47   ` Paul Burton

Linux-MIPS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mips/0 linux-mips/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mips linux-mips/ https://lore.kernel.org/linux-mips \
		linux-mips@vger.kernel.org linux-mips@archiver.kernel.org
	public-inbox-index linux-mips

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mips


AGPL code for this site: git clone https://public-inbox.org/ public-inbox