linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] powerpc: Make setjmp/longjump signature standard
@ 2020-03-27 10:07 Clement Courbet
  2020-03-27 17:10 ` Nick Desaulniers
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Clement Courbet @ 2020-03-27 10:07 UTC (permalink / raw)
  Cc: Greg Kroah-Hartman, Nick Desaulniers, linux-kernel,
	clang-built-linux, Paul Mackerras, Clement Courbet,
	Nathan Chancellor, linuxppc-dev, Thomas Gleixner

Declaring setjmp()/longjmp() as taking longs makes the signature
non-standard, and makes clang complain. In the past, this has been
worked around by adding -ffreestanding to the compile flags.

The implementation looks like it only ever propagates the value
(in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
with integer parameters.

This allows removing -ffreestanding from the compilation flags.

Context:
https://lore.kernel.org/patchwork/patch/1214060
https://lore.kernel.org/patchwork/patch/1216174

Signed-off-by: Clement Courbet <courbet@google.com>
---
 arch/powerpc/include/asm/setjmp.h | 6 ++++--
 arch/powerpc/kexec/Makefile       | 3 ---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
index e9f81bb3f83b..84bb0d140d59 100644
--- a/arch/powerpc/include/asm/setjmp.h
+++ b/arch/powerpc/include/asm/setjmp.h
@@ -7,7 +7,9 @@
 
 #define JMP_BUF_LEN    23
 
-extern long setjmp(long *) __attribute__((returns_twice));
-extern void longjmp(long *, long) __attribute__((noreturn));
+typedef long *jmp_buf;
+
+extern int setjmp(jmp_buf env) __attribute__((returns_twice));
+extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
 
 #endif /* _ASM_POWERPC_SETJMP_H */
diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
index 378f6108a414..86380c69f5ce 100644
--- a/arch/powerpc/kexec/Makefile
+++ b/arch/powerpc/kexec/Makefile
@@ -3,9 +3,6 @@
 # Makefile for the linux kernel.
 #
 
-# Avoid clang warnings around longjmp/setjmp declarations
-CFLAGS_crash.o += -ffreestanding
-
 obj-y				+= core.o crash.o core_$(BITS).o
 
 obj-$(CONFIG_PPC32)		+= relocate_32.o
-- 
2.25.1.696.g5e7596f4ac-goog


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

* Re: [PATCH v1] powerpc: Make setjmp/longjump signature standard
  2020-03-27 10:07 [PATCH v1] powerpc: Make setjmp/longjump signature standard Clement Courbet
@ 2020-03-27 17:10 ` Nick Desaulniers
  2020-03-27 17:27   ` Nathan Chancellor
  2020-03-27 17:45 ` Christophe Leroy
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2020-03-27 17:10 UTC (permalink / raw)
  To: Clement Courbet
  Cc: LKML, clang-built-linux, Paul Mackerras, Greg Kroah-Hartman,
	Nathan Chancellor, linuxppc-dev, Thomas Gleixner

On Fri, Mar 27, 2020 at 3:08 AM Clement Courbet <courbet@google.com> wrote:
>
> Declaring setjmp()/longjmp() as taking longs makes the signature
> non-standard, and makes clang complain. In the past, this has been
> worked around by adding -ffreestanding to the compile flags.
>
> The implementation looks like it only ever propagates the value
> (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> with integer parameters.
>
> This allows removing -ffreestanding from the compilation flags.
>
> Context:
> https://lore.kernel.org/patchwork/patch/1214060
> https://lore.kernel.org/patchwork/patch/1216174
>
> Signed-off-by: Clement Courbet <courbet@google.com>

Hi Clement, thanks for the patch! Would you mind sending a V2 that
included a similar fix to arch/powerpc/xmon/Makefile?

For context, this was the original patch:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aea447141c7e7824b81b49acd1bc78
which was then modified to:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9029ef9c95765e7b63c4d9aa780674447db1ec0

So on your V2, if you include in the commit message, the line:

Fixes c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")

then that will help our LTS branch maintainers back port it to the
appropriate branches.

>
> ---
>  arch/powerpc/include/asm/setjmp.h | 6 ++++--
>  arch/powerpc/kexec/Makefile       | 3 ---
>  2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
> index e9f81bb3f83b..84bb0d140d59 100644
> --- a/arch/powerpc/include/asm/setjmp.h
> +++ b/arch/powerpc/include/asm/setjmp.h
> @@ -7,7 +7,9 @@
>
>  #define JMP_BUF_LEN    23
>
> -extern long setjmp(long *) __attribute__((returns_twice));
> -extern void longjmp(long *, long) __attribute__((noreturn));
> +typedef long *jmp_buf;
> +
> +extern int setjmp(jmp_buf env) __attribute__((returns_twice));
> +extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
>
>  #endif /* _ASM_POWERPC_SETJMP_H */
> diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
> index 378f6108a414..86380c69f5ce 100644
> --- a/arch/powerpc/kexec/Makefile
> +++ b/arch/powerpc/kexec/Makefile
> @@ -3,9 +3,6 @@
>  # Makefile for the linux kernel.
>  #
>
> -# Avoid clang warnings around longjmp/setjmp declarations
> -CFLAGS_crash.o += -ffreestanding
> -
>  obj-y                          += core.o crash.o core_$(BITS).o
>
>  obj-$(CONFIG_PPC32)            += relocate_32.o
> --
> 2.25.1.696.g5e7596f4ac-goog
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH v1] powerpc: Make setjmp/longjump signature standard
  2020-03-27 17:10 ` Nick Desaulniers
@ 2020-03-27 17:27   ` Nathan Chancellor
  0 siblings, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2020-03-27 17:27 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: LKML, clang-built-linux, Paul Mackerras, Clement Courbet,
	Greg Kroah-Hartman, Thomas Gleixner, linuxppc-dev

On Fri, Mar 27, 2020 at 10:10:44AM -0700, Nick Desaulniers wrote:
> On Fri, Mar 27, 2020 at 3:08 AM Clement Courbet <courbet@google.com> wrote:
> >
> > Declaring setjmp()/longjmp() as taking longs makes the signature
> > non-standard, and makes clang complain. In the past, this has been
> > worked around by adding -ffreestanding to the compile flags.
> >
> > The implementation looks like it only ever propagates the value
> > (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> > with integer parameters.
> >
> > This allows removing -ffreestanding from the compilation flags.
> >
> > Context:
> > https://lore.kernel.org/patchwork/patch/1214060
> > https://lore.kernel.org/patchwork/patch/1216174
> >
> > Signed-off-by: Clement Courbet <courbet@google.com>

Thanks for fixing this properly, not really sure why I did not think of
this in the first place. I guess my thought was the warning makes it
seem like clang is going to ignore the kernel's implementation of
setjmp/longjmp but I can't truly remember.

> Hi Clement, thanks for the patch! Would you mind sending a V2 that
> included a similar fix to arch/powerpc/xmon/Makefile?

Agreed.

> For context, this was the original patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aea447141c7e7824b81b49acd1bc78
> which was then modified to:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9029ef9c95765e7b63c4d9aa780674447db1ec0
> 
> So on your V2, if you include in the commit message, the line:
> 
> Fixes c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")
> 
> then that will help our LTS branch maintainers back port it to the
> appropriate branches.

The tags should be:

Cc: stable@vger.kernel.org # v4.14+
Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")

that way it explicitly gets picked up for stable, rather than Sasha's
AUTOSEL process, which could miss it.

With the xmon/Makefile -ffreestanding removed and the tags updated,
consider this:

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

Cheers,
Nathan

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

* Re: [PATCH v1] powerpc: Make setjmp/longjump signature standard
  2020-03-27 10:07 [PATCH v1] powerpc: Make setjmp/longjump signature standard Clement Courbet
  2020-03-27 17:10 ` Nick Desaulniers
@ 2020-03-27 17:45 ` Christophe Leroy
  2020-03-27 18:27   ` Nathan Chancellor
  2020-03-30  6:42 ` Clement Courbet
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Christophe Leroy @ 2020-03-27 17:45 UTC (permalink / raw)
  To: Clement Courbet
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux,
	Paul Mackerras, Greg Kroah-Hartman, Nathan Chancellor,
	linuxppc-dev, Thomas Gleixner

Subject line, change longjump to longjmp

Le 27/03/2020 à 11:07, Clement Courbet a écrit :
> Declaring setjmp()/longjmp() as taking longs makes the signature
> non-standard, and makes clang complain. In the past, this has been
> worked around by adding -ffreestanding to the compile flags.
> 
> The implementation looks like it only ever propagates the value
> (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> with integer parameters.
> 
> This allows removing -ffreestanding from the compilation flags.
> 
> Context:
> https://lore.kernel.org/patchwork/patch/1214060
> https://lore.kernel.org/patchwork/patch/1216174
> 
> Signed-off-by: Clement Courbet <courbet@google.com>
> ---
>   arch/powerpc/include/asm/setjmp.h | 6 ++++--
>   arch/powerpc/kexec/Makefile       | 3 ---
>   2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
> index e9f81bb3f83b..84bb0d140d59 100644
> --- a/arch/powerpc/include/asm/setjmp.h
> +++ b/arch/powerpc/include/asm/setjmp.h
> @@ -7,7 +7,9 @@
>   
>   #define JMP_BUF_LEN    23
>   
> -extern long setjmp(long *) __attribute__((returns_twice));
> -extern void longjmp(long *, long) __attribute__((noreturn));
> +typedef long *jmp_buf;

Do we need that new opaque typedef ? Why not just keep long * ?

> +
> +extern int setjmp(jmp_buf env) __attribute__((returns_twice));
> +extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
>   
>   #endif /* _ASM_POWERPC_SETJMP_H */
> diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
> index 378f6108a414..86380c69f5ce 100644
> --- a/arch/powerpc/kexec/Makefile
> +++ b/arch/powerpc/kexec/Makefile
> @@ -3,9 +3,6 @@
>   # Makefile for the linux kernel.
>   #
>   
> -# Avoid clang warnings around longjmp/setjmp declarations
> -CFLAGS_crash.o += -ffreestanding
> -
>   obj-y				+= core.o crash.o core_$(BITS).o
>   
>   obj-$(CONFIG_PPC32)		+= relocate_32.o
> 

Christophe

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

* Re: [PATCH v1] powerpc: Make setjmp/longjump signature standard
  2020-03-27 17:45 ` Christophe Leroy
@ 2020-03-27 18:27   ` Nathan Chancellor
  0 siblings, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2020-03-27 18:27 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux,
	Paul Mackerras, Clement Courbet, Greg Kroah-Hartman,
	Thomas Gleixner, linuxppc-dev

On Fri, Mar 27, 2020 at 06:45:21PM +0100, Christophe Leroy wrote:
> Subject line, change longjump to longjmp
> 
> Le 27/03/2020 à 11:07, Clement Courbet a écrit :
> > Declaring setjmp()/longjmp() as taking longs makes the signature
> > non-standard, and makes clang complain. In the past, this has been
> > worked around by adding -ffreestanding to the compile flags.
> > 
> > The implementation looks like it only ever propagates the value
> > (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> > with integer parameters.
> > 
> > This allows removing -ffreestanding from the compilation flags.
> > 
> > Context:
> > https://lore.kernel.org/patchwork/patch/1214060
> > https://lore.kernel.org/patchwork/patch/1216174
> > 
> > Signed-off-by: Clement Courbet <courbet@google.com>
> > ---
> >   arch/powerpc/include/asm/setjmp.h | 6 ++++--
> >   arch/powerpc/kexec/Makefile       | 3 ---
> >   2 files changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
> > index e9f81bb3f83b..84bb0d140d59 100644
> > --- a/arch/powerpc/include/asm/setjmp.h
> > +++ b/arch/powerpc/include/asm/setjmp.h
> > @@ -7,7 +7,9 @@
> >   #define JMP_BUF_LEN    23
> > -extern long setjmp(long *) __attribute__((returns_twice));
> > -extern void longjmp(long *, long) __attribute__((noreturn));
> > +typedef long *jmp_buf;
> 
> Do we need that new opaque typedef ? Why not just keep long * ?

Yes, otherwise the warning comes back:

In file included from arch/powerpc/kexec/crash.c:25:
arch/powerpc/include/asm/setjmp.h:10:12: 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 int setjmp(long *env) __attribute__((returns_twice));
           ^
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 *env, int val) __attribute__((noreturn));
            ^
2 errors generated.

> > +
> > +extern int setjmp(jmp_buf env) __attribute__((returns_twice));
> > +extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
> >   #endif /* _ASM_POWERPC_SETJMP_H */
> > diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
> > index 378f6108a414..86380c69f5ce 100644
> > --- a/arch/powerpc/kexec/Makefile
> > +++ b/arch/powerpc/kexec/Makefile
> > @@ -3,9 +3,6 @@
> >   # Makefile for the linux kernel.
> >   #
> > -# Avoid clang warnings around longjmp/setjmp declarations
> > -CFLAGS_crash.o += -ffreestanding
> > -
> >   obj-y				+= core.o crash.o core_$(BITS).o
> >   obj-$(CONFIG_PPC32)		+= relocate_32.o
> > 
> 
> Christophe

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

* [PATCH v1] powerpc: Make setjmp/longjump signature standard
  2020-03-27 10:07 [PATCH v1] powerpc: Make setjmp/longjump signature standard Clement Courbet
  2020-03-27 17:10 ` Nick Desaulniers
  2020-03-27 17:45 ` Christophe Leroy
@ 2020-03-30  6:42 ` Clement Courbet
  2020-03-30  6:43 ` [PATCH v2] powerpc: Make setjmp/longjmp " Clement Courbet
  2020-03-30  8:03 ` [PATCH v3] " Clement Courbet
  4 siblings, 0 replies; 11+ messages in thread
From: Clement Courbet @ 2020-03-30  6:42 UTC (permalink / raw)
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux,
	Paul Mackerras, Clement Courbet, Nathan Chancellor, linuxppc-dev,
	Thomas Gleixner, Allison Randal

Thanks you all for the comments. Everything addressed, plus the array vs
pointer suggestion from Segher Boessenkool on the other thread, which
is only cosmetic and does not change anything wrt behaviour.


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

* [PATCH v2] powerpc: Make setjmp/longjmp signature standard
  2020-03-27 10:07 [PATCH v1] powerpc: Make setjmp/longjump signature standard Clement Courbet
                   ` (2 preceding siblings ...)
  2020-03-30  6:42 ` Clement Courbet
@ 2020-03-30  6:43 ` Clement Courbet
  2020-03-30  7:57   ` Nathan Chancellor
  2020-03-30  8:03 ` [PATCH v3] " Clement Courbet
  4 siblings, 1 reply; 11+ messages in thread
From: Clement Courbet @ 2020-03-30  6:43 UTC (permalink / raw)
  Cc: Greg Kroah-Hartman, Nick Desaulniers, linux-kernel, stable,
	clang-built-linux, Paul Mackerras, Clement Courbet,
	Nathan Chancellor, linuxppc-dev, Thomas Gleixner, Allison Randal

Declaring setjmp()/longjmp() as taking longs makes the signature
non-standard, and makes clang complain. In the past, this has been
worked around by adding -ffreestanding to the compile flags.

The implementation looks like it only ever propagates the value
(in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
with integer parameters.

This allows removing -ffreestanding from the compilation flags.

Context:
https://lore.kernel.org/patchwork/patch/1214060
https://lore.kernel.org/patchwork/patch/1216174

Signed-off-by: Clement Courbet <courbet@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

---

v2:
Use and array type as suggested by Segher Boessenkool
Cc: stable@vger.kernel.org # v4.14+
Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")
---
 arch/powerpc/include/asm/setjmp.h | 6 ++++--
 arch/powerpc/kexec/Makefile       | 3 ---
 arch/powerpc/xmon/Makefile        | 3 ---
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
index e9f81bb3f83b..f798e80e4106 100644
--- a/arch/powerpc/include/asm/setjmp.h
+++ b/arch/powerpc/include/asm/setjmp.h
@@ -7,7 +7,9 @@
 
 #define JMP_BUF_LEN    23
 
-extern long setjmp(long *) __attribute__((returns_twice));
-extern void longjmp(long *, long) __attribute__((noreturn));
+typedef long jmp_buf[JMP_BUF_LEN];
+
+extern int setjmp(jmp_buf env) __attribute__((returns_twice));
+extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
 
 #endif /* _ASM_POWERPC_SETJMP_H */
diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
index 378f6108a414..86380c69f5ce 100644
--- a/arch/powerpc/kexec/Makefile
+++ b/arch/powerpc/kexec/Makefile
@@ -3,9 +3,6 @@
 # Makefile for the linux kernel.
 #
 
-# Avoid clang warnings around longjmp/setjmp declarations
-CFLAGS_crash.o += -ffreestanding
-
 obj-y				+= core.o crash.o core_$(BITS).o
 
 obj-$(CONFIG_PPC32)		+= relocate_32.o
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index c3842dbeb1b7..6f9cccea54f3 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,9 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for xmon
 
-# Avoid clang warnings around longjmp/setjmp declarations
-subdir-ccflags-y := -ffreestanding
-
 GCOV_PROFILE := n
 KCOV_INSTRUMENT := n
 UBSAN_SANITIZE := n
-- 
2.26.0.rc2.310.g2932bb562d-goog


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

* Re: [PATCH v2] powerpc: Make setjmp/longjmp signature standard
  2020-03-30  6:43 ` [PATCH v2] powerpc: Make setjmp/longjmp " Clement Courbet
@ 2020-03-30  7:57   ` Nathan Chancellor
  0 siblings, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2020-03-30  7:57 UTC (permalink / raw)
  To: Clement Courbet
  Cc: Greg Kroah-Hartman, Nick Desaulniers, linux-kernel, stable,
	clang-built-linux, Paul Mackerras, Thomas Gleixner, linuxppc-dev,
	Allison Randal

On Mon, Mar 30, 2020 at 08:43:19AM +0200, Clement Courbet wrote:
> Declaring setjmp()/longjmp() as taking longs makes the signature
> non-standard, and makes clang complain. In the past, this has been
> worked around by adding -ffreestanding to the compile flags.
> 
> The implementation looks like it only ever propagates the value
> (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> with integer parameters.
> 
> This allows removing -ffreestanding from the compilation flags.
> 
> Context:
> https://lore.kernel.org/patchwork/patch/1214060
> https://lore.kernel.org/patchwork/patch/1216174
> 
> Signed-off-by: Clement Courbet <courbet@google.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> 
> ---
> 
> v2:
> Use and array type as suggested by Segher Boessenkool
> Cc: stable@vger.kernel.org # v4.14+
> Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")

Actual diff itself looks good but these two tags need to be above your
signoff to be included in the final changelog. Not sure if Michael will
want a v3 with that or if it can manually be done when applying.

Cheers,
Nathan

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

* [PATCH v3] powerpc: Make setjmp/longjmp signature standard
  2020-03-27 10:07 [PATCH v1] powerpc: Make setjmp/longjump signature standard Clement Courbet
                   ` (3 preceding siblings ...)
  2020-03-30  6:43 ` [PATCH v2] powerpc: Make setjmp/longjmp " Clement Courbet
@ 2020-03-30  8:03 ` Clement Courbet
  2020-03-30 16:20   ` Nick Desaulniers
  2020-04-01 12:53   ` Michael Ellerman
  4 siblings, 2 replies; 11+ messages in thread
From: Clement Courbet @ 2020-03-30  8:03 UTC (permalink / raw)
  Cc: Greg Kroah-Hartman, Nick Desaulniers, linux-kernel, stable,
	clang-built-linux, Paul Mackerras, Clement Courbet,
	Nathan Chancellor, linuxppc-dev, Thomas Gleixner

Declaring setjmp()/longjmp() as taking longs makes the signature
non-standard, and makes clang complain. In the past, this has been
worked around by adding -ffreestanding to the compile flags.

The implementation looks like it only ever propagates the value
(in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
with integer parameters.

This allows removing -ffreestanding from the compilation flags.

Context:
https://lore.kernel.org/patchwork/patch/1214060
https://lore.kernel.org/patchwork/patch/1216174

Signed-off-by: Clement Courbet <courbet@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Cc: stable@vger.kernel.org # v4.14+
Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")

---

v2:
Use and array type as suggested by Segher Boessenkool
Add fix tags.

v3:
Properly place tags.
---
 arch/powerpc/include/asm/setjmp.h | 6 ++++--
 arch/powerpc/kexec/Makefile       | 3 ---
 arch/powerpc/xmon/Makefile        | 3 ---
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
index e9f81bb3f83b..f798e80e4106 100644
--- a/arch/powerpc/include/asm/setjmp.h
+++ b/arch/powerpc/include/asm/setjmp.h
@@ -7,7 +7,9 @@
 
 #define JMP_BUF_LEN    23
 
-extern long setjmp(long *) __attribute__((returns_twice));
-extern void longjmp(long *, long) __attribute__((noreturn));
+typedef long jmp_buf[JMP_BUF_LEN];
+
+extern int setjmp(jmp_buf env) __attribute__((returns_twice));
+extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
 
 #endif /* _ASM_POWERPC_SETJMP_H */
diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
index 378f6108a414..86380c69f5ce 100644
--- a/arch/powerpc/kexec/Makefile
+++ b/arch/powerpc/kexec/Makefile
@@ -3,9 +3,6 @@
 # Makefile for the linux kernel.
 #
 
-# Avoid clang warnings around longjmp/setjmp declarations
-CFLAGS_crash.o += -ffreestanding
-
 obj-y				+= core.o crash.o core_$(BITS).o
 
 obj-$(CONFIG_PPC32)		+= relocate_32.o
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index c3842dbeb1b7..6f9cccea54f3 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,9 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for xmon
 
-# Avoid clang warnings around longjmp/setjmp declarations
-subdir-ccflags-y := -ffreestanding
-
 GCOV_PROFILE := n
 KCOV_INSTRUMENT := n
 UBSAN_SANITIZE := n
-- 
2.26.0.rc2.310.g2932bb562d-goog


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

* Re: [PATCH v3] powerpc: Make setjmp/longjmp signature standard
  2020-03-30  8:03 ` [PATCH v3] " Clement Courbet
@ 2020-03-30 16:20   ` Nick Desaulniers
  2020-04-01 12:53   ` Michael Ellerman
  1 sibling, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2020-03-30 16:20 UTC (permalink / raw)
  To: Clement Courbet
  Cc: Greg Kroah-Hartman, LKML, # 3.4.x, clang-built-linux,
	Paul Mackerras, Nathan Chancellor, linuxppc-dev, Thomas Gleixner

On Mon, Mar 30, 2020 at 1:04 AM Clement Courbet <courbet@google.com> wrote:
>
> Declaring setjmp()/longjmp() as taking longs makes the signature
> non-standard, and makes clang complain. In the past, this has been
> worked around by adding -ffreestanding to the compile flags.
>
> The implementation looks like it only ever propagates the value
> (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> with integer parameters.
>
> This allows removing -ffreestanding from the compilation flags.
>
> Context:
> https://lore.kernel.org/patchwork/patch/1214060
> https://lore.kernel.org/patchwork/patch/1216174
>
> Signed-off-by: Clement Courbet <courbet@google.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Cc: stable@vger.kernel.org # v4.14+
> Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")

Third time's a charm (for patches that tackle this warning). Thanks
for following up on this cleanup!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

The extent of my testing was compile testing with Clang.

>
> ---
>
> v2:
> Use and array type as suggested by Segher Boessenkool
> Add fix tags.
>
> v3:
> Properly place tags.
> ---
>  arch/powerpc/include/asm/setjmp.h | 6 ++++--
>  arch/powerpc/kexec/Makefile       | 3 ---
>  arch/powerpc/xmon/Makefile        | 3 ---
>  3 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
> index e9f81bb3f83b..f798e80e4106 100644
> --- a/arch/powerpc/include/asm/setjmp.h
> +++ b/arch/powerpc/include/asm/setjmp.h
> @@ -7,7 +7,9 @@
>
>  #define JMP_BUF_LEN    23
>
> -extern long setjmp(long *) __attribute__((returns_twice));
> -extern void longjmp(long *, long) __attribute__((noreturn));
> +typedef long jmp_buf[JMP_BUF_LEN];
> +
> +extern int setjmp(jmp_buf env) __attribute__((returns_twice));
> +extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
>
>  #endif /* _ASM_POWERPC_SETJMP_H */
> diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
> index 378f6108a414..86380c69f5ce 100644
> --- a/arch/powerpc/kexec/Makefile
> +++ b/arch/powerpc/kexec/Makefile
> @@ -3,9 +3,6 @@
>  # Makefile for the linux kernel.
>  #
>
> -# Avoid clang warnings around longjmp/setjmp declarations
> -CFLAGS_crash.o += -ffreestanding
> -
>  obj-y                          += core.o crash.o core_$(BITS).o
>
>  obj-$(CONFIG_PPC32)            += relocate_32.o
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index c3842dbeb1b7..6f9cccea54f3 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -1,9 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Makefile for xmon
>
> -# Avoid clang warnings around longjmp/setjmp declarations
> -subdir-ccflags-y := -ffreestanding
> -
>  GCOV_PROFILE := n
>  KCOV_INSTRUMENT := n
>  UBSAN_SANITIZE := n
> --
> 2.26.0.rc2.310.g2932bb562d-goog
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH v3] powerpc: Make setjmp/longjmp signature standard
  2020-03-30  8:03 ` [PATCH v3] " Clement Courbet
  2020-03-30 16:20   ` Nick Desaulniers
@ 2020-04-01 12:53   ` Michael Ellerman
  1 sibling, 0 replies; 11+ messages in thread
From: Michael Ellerman @ 2020-04-01 12:53 UTC (permalink / raw)
  To: Clement Courbet
  Cc: Greg Kroah-Hartman, Nick Desaulniers, linux-kernel, stable,
	clang-built-linux, Paul Mackerras, Clement Courbet,
	Nathan Chancellor, linuxppc-dev, Thomas Gleixner

On Mon, 2020-03-30 at 08:03:56 UTC, Clement Courbet wrote:
> Declaring setjmp()/longjmp() as taking longs makes the signature
> non-standard, and makes clang complain. In the past, this has been
> worked around by adding -ffreestanding to the compile flags.
> 
> The implementation looks like it only ever propagates the value
> (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> with integer parameters.
> 
> This allows removing -ffreestanding from the compilation flags.
> 
> Context:
> https://lore.kernel.org/patchwork/patch/1214060
> https://lore.kernel.org/patchwork/patch/1216174
> 
> Signed-off-by: Clement Courbet <courbet@google.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Cc: stable@vger.kernel.org # v4.14+
> Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/c17eb4dca5a353a9dbbb8ad6934fe57af7165e91

cheers

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

end of thread, other threads:[~2020-04-01 13:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-27 10:07 [PATCH v1] powerpc: Make setjmp/longjump signature standard Clement Courbet
2020-03-27 17:10 ` Nick Desaulniers
2020-03-27 17:27   ` Nathan Chancellor
2020-03-27 17:45 ` Christophe Leroy
2020-03-27 18:27   ` Nathan Chancellor
2020-03-30  6:42 ` Clement Courbet
2020-03-30  6:43 ` [PATCH v2] powerpc: Make setjmp/longjmp " Clement Courbet
2020-03-30  7:57   ` Nathan Chancellor
2020-03-30  8:03 ` [PATCH v3] " Clement Courbet
2020-03-30 16:20   ` Nick Desaulniers
2020-04-01 12:53   ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).