* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-12 2:32 [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
@ 2019-08-12 5:37 ` Christophe Leroy
2019-08-12 16:55 ` Nathan Chancellor
2019-08-12 17:35 ` Nick Desaulniers
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ messages in thread
From: Christophe Leroy @ 2019-08-12 5:37 UTC (permalink / raw)
To: Nathan Chancellor, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Nick Desaulniers
Cc: clang-built-linux, linuxppc-dev, linux-kernel, stable
Le 12/08/2019 à 04:32, Nathan Chancellor a écrit :
> Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> about the setjmp and longjmp declarations.
>
> r367387 in clang added another diagnostic around this, complaining that
> there is no jmp_buf declaration.
>
[...]
>
> Cc: stable@vger.kernel.org # 4.19+
> Link: https://github.com/ClangBuiltLinux/linux/issues/625
> Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
[...]
>
> arch/powerpc/kernel/Makefile | 5 +++--
> arch/powerpc/xmon/Makefile | 5 +++--
What about scripts/recordmcount.c and scripts/sortextable.c which
contains calls to setjmp() and longjmp() ?
And arch/um/ ?
Christophe
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index ea0c69236789..44e340ed4722 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -5,8 +5,9 @@
>
> CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of setjmp.h and declaration of jmp_buf type)
> +CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header) \
> + $(call cc-disable-warning, incomplete-setjmp-declaration)
>
> ifdef CONFIG_PPC64
> CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index f142570ad860..53f341391210 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -1,8 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> # Makefile for xmon
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of setjmp.h and declaration of jmp_buf type)
> +subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header) \
> + $(call cc-disable-warning, incomplete-setjmp-declaration)
>
> GCOV_PROFILE := n
> KCOV_INSTRUMENT := n
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-12 5:37 ` Christophe Leroy
@ 2019-08-12 16:55 ` Nathan Chancellor
0 siblings, 0 replies; 18+ messages in thread
From: Nathan Chancellor @ 2019-08-12 16:55 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nick Desaulniers, clang-built-linux, linuxppc-dev, linux-kernel,
stable
On Mon, Aug 12, 2019 at 07:37:51AM +0200, Christophe Leroy wrote:
>
>
> Le 12/08/2019 à 04:32, Nathan Chancellor a écrit :
> > Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> > setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> > about the setjmp and longjmp declarations.
> >
> > r367387 in clang added another diagnostic around this, complaining that
> > there is no jmp_buf declaration.
> >
> [...]
>
> >
> > Cc: stable@vger.kernel.org # 4.19+
> > Link: https://github.com/ClangBuiltLinux/linux/issues/625
> > Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> [...]
>
> >
> > arch/powerpc/kernel/Makefile | 5 +++--
> > arch/powerpc/xmon/Makefile | 5 +++--
>
> What about scripts/recordmcount.c and scripts/sortextable.c which contains
> calls to setjmp() and longjmp() ?
>
> And arch/um/ ?
>
> Christophe
Hi Christophe,
It looks like all of those will be using the system's setjmp header,
which won't cause these warnings.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-12 2:32 [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
2019-08-12 5:37 ` Christophe Leroy
@ 2019-08-12 17:35 ` Nick Desaulniers
2019-08-27 0:12 ` Nathan Chancellor
2019-08-28 13:43 ` Michael Ellerman
3 siblings, 0 replies; 18+ messages in thread
From: Nick Desaulniers @ 2019-08-12 17:35 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev, LKML, clang-built-linux, # 3.4.x
On Sun, Aug 11, 2019 at 7:42 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> about the setjmp and longjmp declarations.
>
> r367387 in clang added another diagnostic around this, complaining that
> there is no jmp_buf declaration.
>
> In file included from ../arch/powerpc/xmon/xmon.c:47:
> ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header <setjmp.h>.
> [-Werror,-Wincomplete-setjmp-declaration]
> extern long setjmp(long *);
> ^
> ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header <setjmp.h>.
> [-Werror,-Wincomplete-setjmp-declaration]
> extern void longjmp(long *, long);
> ^
> 2 errors generated.
>
> Take the same approach as the above commit by disabling the warning for
> the same reason, we provide our own longjmp/setjmp function.
>
> Cc: stable@vger.kernel.org # 4.19+
> Link: https://github.com/ClangBuiltLinux/linux/issues/625
> Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Thanks for the patch, Nathan.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> instead as it makes it clear to clang that we are not using the builtin
> longjmp and setjmp functions, which I think is why these warnings are
> appearing (at least according to the commit that introduced this waring).
>
> Sample patch:
> https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
>
> However, this is the most conservative approach, as I have already had
> someone notice this error when building LLVM with PGO on tip of tree
> LLVM.
>
> arch/powerpc/kernel/Makefile | 5 +++--
> arch/powerpc/xmon/Makefile | 5 +++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index ea0c69236789..44e340ed4722 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -5,8 +5,9 @@
>
> CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of setjmp.h and declaration of jmp_buf type)
> +CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header) \
> + $(call cc-disable-warning, incomplete-setjmp-declaration)
>
> ifdef CONFIG_PPC64
> CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index f142570ad860..53f341391210 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -1,8 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> # Makefile for xmon
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of setjmp.h and declaration of jmp_buf type)
> +subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header) \
> + $(call cc-disable-warning, incomplete-setjmp-declaration)
>
> GCOV_PROFILE := n
> KCOV_INSTRUMENT := n
> --
> 2.23.0.rc2
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-12 2:32 [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
2019-08-12 5:37 ` Christophe Leroy
2019-08-12 17:35 ` Nick Desaulniers
@ 2019-08-27 0:12 ` Nathan Chancellor
2019-08-28 13:43 ` Michael Ellerman
3 siblings, 0 replies; 18+ messages in thread
From: Nathan Chancellor @ 2019-08-27 0:12 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nick Desaulniers
Cc: linuxppc-dev, linux-kernel, clang-built-linux, stable
On Sun, Aug 11, 2019 at 07:32:15PM -0700, Nathan Chancellor wrote:
> Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> about the setjmp and longjmp declarations.
>
> r367387 in clang added another diagnostic around this, complaining that
> there is no jmp_buf declaration.
>
> In file included from ../arch/powerpc/xmon/xmon.c:47:
> ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header <setjmp.h>.
> [-Werror,-Wincomplete-setjmp-declaration]
> extern long setjmp(long *);
> ^
> ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header <setjmp.h>.
> [-Werror,-Wincomplete-setjmp-declaration]
> extern void longjmp(long *, long);
> ^
> 2 errors generated.
>
> Take the same approach as the above commit by disabling the warning for
> the same reason, we provide our own longjmp/setjmp function.
>
> Cc: stable@vger.kernel.org # 4.19+
> Link: https://github.com/ClangBuiltLinux/linux/issues/625
> Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> instead as it makes it clear to clang that we are not using the builtin
> longjmp and setjmp functions, which I think is why these warnings are
> appearing (at least according to the commit that introduced this waring).
>
> Sample patch:
> https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
>
> However, this is the most conservative approach, as I have already had
> someone notice this error when building LLVM with PGO on tip of tree
> LLVM.
>
> arch/powerpc/kernel/Makefile | 5 +++--
> arch/powerpc/xmon/Makefile | 5 +++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index ea0c69236789..44e340ed4722 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -5,8 +5,9 @@
>
> CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of setjmp.h and declaration of jmp_buf type)
> +CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header) \
> + $(call cc-disable-warning, incomplete-setjmp-declaration)
>
> ifdef CONFIG_PPC64
> CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index f142570ad860..53f341391210 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -1,8 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> # Makefile for xmon
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of setjmp.h and declaration of jmp_buf type)
> +subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header) \
> + $(call cc-disable-warning, incomplete-setjmp-declaration)
>
> GCOV_PROFILE := n
> KCOV_INSTRUMENT := n
> --
> 2.23.0.rc2
>
Did any of the maintainers have any comment on this patch?
Cheers,
Nathan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-12 2:32 [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
` (2 preceding siblings ...)
2019-08-27 0:12 ` Nathan Chancellor
@ 2019-08-28 13:43 ` Michael Ellerman
2019-08-28 17:53 ` Nathan Chancellor
3 siblings, 1 reply; 18+ messages in thread
From: Michael Ellerman @ 2019-08-28 13:43 UTC (permalink / raw)
To: Nathan Chancellor, Benjamin Herrenschmidt, Paul Mackerras,
Nick Desaulniers
Cc: linuxppc-dev, linux-kernel, clang-built-linux, Nathan Chancellor, stable
Nathan Chancellor <natechancellor@gmail.com> writes:
> Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> about the setjmp and longjmp declarations.
>
> r367387 in clang added another diagnostic around this, complaining that
> there is no jmp_buf declaration.
>
> In file included from ../arch/powerpc/xmon/xmon.c:47:
> ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header <setjmp.h>.
> [-Werror,-Wincomplete-setjmp-declaration]
> extern long setjmp(long *);
> ^
> ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header <setjmp.h>.
> [-Werror,-Wincomplete-setjmp-declaration]
> extern void longjmp(long *, long);
> ^
> 2 errors generated.
>
> Take the same approach as the above commit by disabling the warning for
> the same reason, we provide our own longjmp/setjmp function.
>
> Cc: stable@vger.kernel.org # 4.19+
> Link: https://github.com/ClangBuiltLinux/linux/issues/625
> Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> instead as it makes it clear to clang that we are not using the builtin
> longjmp and setjmp functions, which I think is why these warnings are
> appearing (at least according to the commit that introduced this waring).
>
> Sample patch:
> https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
Couldn't we just add those flags to CFLAGS for the whole kernel? Rather
than making them per-file.
I mean there's no kernel code that wants to use clang's builtin
setjmp/longjmp implementation at all right?
cheers
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-28 13:43 ` Michael Ellerman
@ 2019-08-28 17:53 ` Nathan Chancellor
2019-08-28 18:01 ` Nick Desaulniers
0 siblings, 1 reply; 18+ messages in thread
From: Nathan Chancellor @ 2019-08-28 17:53 UTC (permalink / raw)
To: Michael Ellerman
Cc: Benjamin Herrenschmidt, Paul Mackerras, Nick Desaulniers,
linuxppc-dev, linux-kernel, clang-built-linux, stable
On Wed, Aug 28, 2019 at 11:43:53PM +1000, Michael Ellerman wrote:
> Nathan Chancellor <natechancellor@gmail.com> writes:
>
> > Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> > setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> > about the setjmp and longjmp declarations.
> >
> > r367387 in clang added another diagnostic around this, complaining that
> > there is no jmp_buf declaration.
> >
> > In file included from ../arch/powerpc/xmon/xmon.c:47:
> > ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> > built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> > type, commonly provided in the header <setjmp.h>.
> > [-Werror,-Wincomplete-setjmp-declaration]
> > extern long setjmp(long *);
> > ^
> > ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> > built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> > type, commonly provided in the header <setjmp.h>.
> > [-Werror,-Wincomplete-setjmp-declaration]
> > extern void longjmp(long *, long);
> > ^
> > 2 errors generated.
> >
> > Take the same approach as the above commit by disabling the warning for
> > the same reason, we provide our own longjmp/setjmp function.
> >
> > Cc: stable@vger.kernel.org # 4.19+
> > Link: https://github.com/ClangBuiltLinux/linux/issues/625
> > Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> > instead as it makes it clear to clang that we are not using the builtin
> > longjmp and setjmp functions, which I think is why these warnings are
> > appearing (at least according to the commit that introduced this waring).
> >
> > Sample patch:
> > https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
>
> Couldn't we just add those flags to CFLAGS for the whole kernel? Rather
> than making them per-file.
Yes, I don't think this would be unreasonable. Are you referring to the
cc-disable-warning flags or the -fno-builtin flags? I personally think
the -fno-builtin flags convey to clang what the kernel is intending to
do better than disabling the warnings outright.
> I mean there's no kernel code that wants to use clang's builtin
> setjmp/longjmp implementation at all right?
>
> cheers
I did a quick search of the tree and it looks like powerpc and x86/um
are the only architectures that do anything with setjmp/longjmp. x86/um
avoids this by using a define flag to change setjmp to kernel_setjmp:
arch/um/Makefile: -Dlongjmp=kernel_longjmp -Dsetjmp=kernel_setjmp \
Seems like adding those flags should be safe.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-28 17:53 ` Nathan Chancellor
@ 2019-08-28 18:01 ` Nick Desaulniers
2019-08-28 18:45 ` Nathan Chancellor
0 siblings, 1 reply; 18+ messages in thread
From: Nick Desaulniers @ 2019-08-28 18:01 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev, LKML, clang-built-linux, # 3.4.x
On Wed, Aug 28, 2019 at 10:53 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Wed, Aug 28, 2019 at 11:43:53PM +1000, Michael Ellerman wrote:
> > Nathan Chancellor <natechancellor@gmail.com> writes:
> >
> > > Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> > > setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> > > about the setjmp and longjmp declarations.
> > >
> > > r367387 in clang added another diagnostic around this, complaining that
> > > there is no jmp_buf declaration.
> > >
> > > In file included from ../arch/powerpc/xmon/xmon.c:47:
> > > ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> > > built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> > > type, commonly provided in the header <setjmp.h>.
> > > [-Werror,-Wincomplete-setjmp-declaration]
> > > extern long setjmp(long *);
> > > ^
> > > ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> > > built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> > > type, commonly provided in the header <setjmp.h>.
> > > [-Werror,-Wincomplete-setjmp-declaration]
> > > extern void longjmp(long *, long);
> > > ^
> > > 2 errors generated.
> > >
> > > Take the same approach as the above commit by disabling the warning for
> > > the same reason, we provide our own longjmp/setjmp function.
> > >
> > > Cc: stable@vger.kernel.org # 4.19+
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/625
> > > Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > > ---
> > >
> > > It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> > > instead as it makes it clear to clang that we are not using the builtin
> > > longjmp and setjmp functions, which I think is why these warnings are
> > > appearing (at least according to the commit that introduced this waring).
> > >
> > > Sample patch:
> > > https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
> >
> > Couldn't we just add those flags to CFLAGS for the whole kernel? Rather
> > than making them per-file.
>
> Yes, I don't think this would be unreasonable. Are you referring to the
> cc-disable-warning flags or the -fno-builtin flags? I personally think
> the -fno-builtin flags convey to clang what the kernel is intending to
> do better than disabling the warnings outright.
The `-f` family of flags have dire implications for codegen, I'd
really prefer we think long and hard before adding/removing them to
suppress warnings. I don't think it's a solution for this particular
problem.
>
> > I mean there's no kernel code that wants to use clang's builtin
> > setjmp/longjmp implementation at all right?
> >
> > cheers
>
> I did a quick search of the tree and it looks like powerpc and x86/um
> are the only architectures that do anything with setjmp/longjmp. x86/um
> avoids this by using a define flag to change setjmp to kernel_setjmp:
>
> arch/um/Makefile: -Dlongjmp=kernel_longjmp -Dsetjmp=kernel_setjmp \
>
> Seems like adding those flags should be safe.
>
> Cheers,
> Nathan
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-28 18:01 ` Nick Desaulniers
@ 2019-08-28 18:45 ` Nathan Chancellor
2019-08-28 22:16 ` Nick Desaulniers
[not found] ` <6801a83ed6d54d95b87a41c57ef6e6b0@AcuMS.aculab.com>
0 siblings, 2 replies; 18+ messages in thread
From: Nathan Chancellor @ 2019-08-28 18:45 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev, LKML, clang-built-linux, # 3.4.x
On Wed, Aug 28, 2019 at 11:01:14AM -0700, Nick Desaulniers wrote:
> On Wed, Aug 28, 2019 at 10:53 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Wed, Aug 28, 2019 at 11:43:53PM +1000, Michael Ellerman wrote:
> > > Nathan Chancellor <natechancellor@gmail.com> writes:
> > >
> > > > Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> > > > setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> > > > about the setjmp and longjmp declarations.
> > > >
> > > > r367387 in clang added another diagnostic around this, complaining that
> > > > there is no jmp_buf declaration.
> > > >
> > > > In file included from ../arch/powerpc/xmon/xmon.c:47:
> > > > ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> > > > built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> > > > type, commonly provided in the header <setjmp.h>.
> > > > [-Werror,-Wincomplete-setjmp-declaration]
> > > > extern long setjmp(long *);
> > > > ^
> > > > ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> > > > built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> > > > type, commonly provided in the header <setjmp.h>.
> > > > [-Werror,-Wincomplete-setjmp-declaration]
> > > > extern void longjmp(long *, long);
> > > > ^
> > > > 2 errors generated.
> > > >
> > > > Take the same approach as the above commit by disabling the warning for
> > > > the same reason, we provide our own longjmp/setjmp function.
> > > >
> > > > Cc: stable@vger.kernel.org # 4.19+
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/625
> > > > Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> > > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > > > ---
> > > >
> > > > It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> > > > instead as it makes it clear to clang that we are not using the builtin
> > > > longjmp and setjmp functions, which I think is why these warnings are
> > > > appearing (at least according to the commit that introduced this waring).
> > > >
> > > > Sample patch:
> > > > https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
> > >
> > > Couldn't we just add those flags to CFLAGS for the whole kernel? Rather
> > > than making them per-file.
> >
> > Yes, I don't think this would be unreasonable. Are you referring to the
> > cc-disable-warning flags or the -fno-builtin flags? I personally think
> > the -fno-builtin flags convey to clang what the kernel is intending to
> > do better than disabling the warnings outright.
>
> The `-f` family of flags have dire implications for codegen, I'd
> really prefer we think long and hard before adding/removing them to
> suppress warnings. I don't think it's a solution for this particular
> problem.
I am fine with whatever approach gets this warning fixed to the
maintainer's satisfaction...
However, I think that -fno-builtin-* would be appropriate here because
we are providing our own setjmp implementation, meaning clang should not
be trying to do anything with the builtin implementation like building a
declaration for it.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-28 18:45 ` Nathan Chancellor
@ 2019-08-28 22:16 ` Nick Desaulniers
2019-08-29 8:32 ` Segher Boessenkool
[not found] ` <6801a83ed6d54d95b87a41c57ef6e6b0@AcuMS.aculab.com>
1 sibling, 1 reply; 18+ messages in thread
From: Nick Desaulniers @ 2019-08-28 22:16 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev, LKML, clang-built-linux, # 3.4.x
On Wed, Aug 28, 2019 at 11:45 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Wed, Aug 28, 2019 at 11:01:14AM -0700, Nick Desaulniers wrote:
> > On Wed, Aug 28, 2019 at 10:53 AM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > >
> > > Yes, I don't think this would be unreasonable. Are you referring to the
> > > cc-disable-warning flags or the -fno-builtin flags? I personally think
> > > the -fno-builtin flags convey to clang what the kernel is intending to
> > > do better than disabling the warnings outright.
> >
> > The `-f` family of flags have dire implications for codegen, I'd
> > really prefer we think long and hard before adding/removing them to
> > suppress warnings. I don't think it's a solution for this particular
> > problem.
>
> I am fine with whatever approach gets this warning fixed to the
> maintainer's satisfaction...
>
> However, I think that -fno-builtin-* would be appropriate here because
> we are providing our own setjmp implementation, meaning clang should not
> be trying to do anything with the builtin implementation like building a
> declaration for it.
That's a good reason IMO. IIRC, the -fno-builtin-* flags don't warn
if * is some unrecognized value, so -fno-builtin-setjmp may not
actually do anything, and you may need to scan the source (of clang or
llvm).
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-08-28 22:16 ` Nick Desaulniers
@ 2019-08-29 8:32 ` Segher Boessenkool
0 siblings, 0 replies; 18+ messages in thread
From: Segher Boessenkool @ 2019-08-29 8:32 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, LKML, # 3.4.x, clang-built-linux,
Paul Mackerras, linuxppc-dev
On Wed, Aug 28, 2019 at 03:16:19PM -0700, Nick Desaulniers wrote:
> That's a good reason IMO. IIRC, the -fno-builtin-* flags don't warn
> if * is some unrecognized value, so -fno-builtin-setjmp may not
> actually do anything, and you may need to scan the source (of clang or
> llvm).
-fno-builtin-foo makes the compiler not handle "foo" the same as it
handles "__builtin_foo". If the compiler has no idea about "foo", well,
that is exactly what it does then anyhow, so why would it warn :-)
Segher
^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <6801a83ed6d54d95b87a41c57ef6e6b0@AcuMS.aculab.com>]
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
[not found] ` <6801a83ed6d54d95b87a41c57ef6e6b0@AcuMS.aculab.com>
@ 2019-09-03 5:55 ` Nathan Chancellor
2019-09-03 19:31 ` Segher Boessenkool
0 siblings, 1 reply; 18+ messages in thread
From: Nathan Chancellor @ 2019-09-03 5:55 UTC (permalink / raw)
To: David Laight
Cc: Nick Desaulniers, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, LKML, clang-built-linux, # 3.4.x
On Thu, Aug 29, 2019 at 09:59:48AM +0000, David Laight wrote:
> From: Nathan Chancellor
> > Sent: 28 August 2019 19:45
> ...
> > However, I think that -fno-builtin-* would be appropriate here because
> > we are providing our own setjmp implementation, meaning clang should not
> > be trying to do anything with the builtin implementation like building a
> > declaration for it.
>
> Isn't implementing setjmp impossible unless you tell the compiler that
> you function is 'setjmp-like' ?
No idea, PowerPC is the only architecture that does such a thing.
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/tree/arch/powerpc/kernel/misc.S#n43
Goes back all the way to before git history (all the way to ppc64's
addition actually):
https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=61542216fa90397a2e70c46583edf26bc81994df
https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/arch/ppc64/xmon/setjmp.c?id=5f12b0bff93831620218e8ed3970903ecb7861ce
I would just like this warning fixed given that PowerPC builds with
-Werror by default so it is causing a build failure in our CI.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-09-03 5:55 ` Nathan Chancellor
@ 2019-09-03 19:31 ` Segher Boessenkool
2019-09-04 0:24 ` Nathan Chancellor
0 siblings, 1 reply; 18+ messages in thread
From: Segher Boessenkool @ 2019-09-03 19:31 UTC (permalink / raw)
To: Nathan Chancellor
Cc: David Laight, Nick Desaulniers, LKML, # 3.4.x, clang-built-linux,
Paul Mackerras, linuxppc-dev
On Mon, Sep 02, 2019 at 10:55:53PM -0700, Nathan Chancellor wrote:
> On Thu, Aug 29, 2019 at 09:59:48AM +0000, David Laight wrote:
> > From: Nathan Chancellor
> > > Sent: 28 August 2019 19:45
> > ...
> > > However, I think that -fno-builtin-* would be appropriate here because
> > > we are providing our own setjmp implementation, meaning clang should not
> > > be trying to do anything with the builtin implementation like building a
> > > declaration for it.
> >
> > Isn't implementing setjmp impossible unless you tell the compiler that
> > you function is 'setjmp-like' ?
>
> No idea, PowerPC is the only architecture that does such a thing.
Since setjmp can return more than once, yes, exciting things can happen
if you do not tell the compiler about this.
Segher
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
2019-09-03 19:31 ` Segher Boessenkool
@ 2019-09-04 0:24 ` Nathan Chancellor
[not found] ` <1bcd7086f3d24dfa82eec03980f30fbc@AcuMS.aculab.com>
0 siblings, 1 reply; 18+ messages in thread
From: Nathan Chancellor @ 2019-09-04 0:24 UTC (permalink / raw)
To: Segher Boessenkool
Cc: David Laight, Nick Desaulniers, LKML, # 3.4.x, clang-built-linux,
Paul Mackerras, linuxppc-dev
On Tue, Sep 03, 2019 at 02:31:28PM -0500, Segher Boessenkool wrote:
> On Mon, Sep 02, 2019 at 10:55:53PM -0700, Nathan Chancellor wrote:
> > On Thu, Aug 29, 2019 at 09:59:48AM +0000, David Laight wrote:
> > > From: Nathan Chancellor
> > > > Sent: 28 August 2019 19:45
> > > ...
> > > > However, I think that -fno-builtin-* would be appropriate here because
> > > > we are providing our own setjmp implementation, meaning clang should not
> > > > be trying to do anything with the builtin implementation like building a
> > > > declaration for it.
> > >
> > > Isn't implementing setjmp impossible unless you tell the compiler that
> > > you function is 'setjmp-like' ?
> >
> > No idea, PowerPC is the only architecture that does such a thing.
>
> Since setjmp can return more than once, yes, exciting things can happen
> if you do not tell the compiler about this.
>
>
> Segher
>
Fair enough so I guess we are back to just outright disabling the
warning.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 18+ messages in thread