linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc: mark local variables around longjmp as volatile
@ 2021-04-29  8:06 Arnd Bergmann
  2021-06-25 10:35 ` Christophe Leroy
  2021-06-26 10:37 ` Michael Ellerman
  0 siblings, 2 replies; 3+ messages in thread
From: Arnd Bergmann @ 2021-04-29  8:06 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Arnd Bergmann, Benjamin Herrenschmidt, Paul Mackerras,
	Nicholas Piggin, Ravi Bangoria, Christophe Leroy, linuxppc-dev,
	linux-kernel

From: Arnd Bergmann <arnd@arndb.de>

gcc-11 points out that modifying local variables next to a
longjmp/setjmp may cause undefined behavior:

arch/powerpc/kexec/crash.c: In function 'crash_kexec_prepare_cpus.constprop':
arch/powerpc/kexec/crash.c:108:22: error: variable 'ncpus' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbere
d]
arch/powerpc/kexec/crash.c:109:13: error: variable 'tries' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbere
d]
arch/powerpc/xmon/xmon.c: In function 'xmon_print_symbol':
arch/powerpc/xmon/xmon.c:3625:21: error: variable 'name' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'stop_spus':
arch/powerpc/xmon/xmon.c:4057:13: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'restart_spus':
arch/powerpc/xmon/xmon.c:4098:13: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'dump_opal_msglog':
arch/powerpc/xmon/xmon.c:3008:16: error: variable 'pos' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'show_pte':
arch/powerpc/xmon/xmon.c:3207:29: error: variable 'tsk' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'show_tasks':
arch/powerpc/xmon/xmon.c:3302:29: error: variable 'tsk' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c: In function 'xmon_core':
arch/powerpc/xmon/xmon.c:494:13: error: variable 'cmd' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c:860:21: error: variable 'bp' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c:860:21: error: variable 'bp' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
arch/powerpc/xmon/xmon.c:492:48: error: argument 'fromipi' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]

According to the documentation, marking these as 'volatile' is
sufficient to avoid the problem, and it shuts up the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/kexec/crash.c |  4 ++--
 arch/powerpc/xmon/xmon.c   | 22 +++++++++++-----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index 0196d0c211ac..10f997e6bb95 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -105,8 +105,8 @@ void crash_ipi_callback(struct pt_regs *regs)
 static void crash_kexec_prepare_cpus(int cpu)
 {
 	unsigned int msecs;
-	unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
-	int tries = 0;
+	volatile unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
+	volatile int tries = 0;
 	int (*old_handler)(struct pt_regs *regs);
 
 	printk(KERN_EMERG "Sending IPI to other CPUs\n");
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index c8173e92f19d..ce0eacf77645 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -489,10 +489,10 @@ static void xmon_touch_watchdogs(void)
 	touch_nmi_watchdog();
 }
 
-static int xmon_core(struct pt_regs *regs, int fromipi)
+static int xmon_core(struct pt_regs *regs, volatile int fromipi)
 {
-	int cmd = 0;
-	struct bpt *bp;
+	volatile int cmd = 0;
+	struct bpt *volatile bp;
 	long recurse_jmp[JMP_BUF_LEN];
 	bool locked_down;
 	unsigned long offset;
@@ -857,7 +857,7 @@ static inline void force_enable_xmon(void)
 static struct bpt *at_breakpoint(unsigned long pc)
 {
 	int i;
-	struct bpt *bp;
+	struct bpt *volatile bp;
 
 	bp = bpts;
 	for (i = 0; i < NBPTS; ++i, ++bp)
@@ -3005,7 +3005,7 @@ static void dump_opal_msglog(void)
 {
 	unsigned char buf[128];
 	ssize_t res;
-	loff_t pos = 0;
+	volatile loff_t pos = 0;
 
 	if (!firmware_has_feature(FW_FEATURE_OPAL)) {
 		printf("Machine is not running OPAL firmware.\n");
@@ -3160,7 +3160,7 @@ memzcan(void)
 		printf("%.8lx\n", a - mskip);
 }
 
-static void show_task(struct task_struct *tsk)
+static void show_task(struct task_struct *volatile tsk)
 {
 	char state;
 
@@ -3204,7 +3204,7 @@ static void format_pte(void *ptep, unsigned long pte)
 static void show_pte(unsigned long addr)
 {
 	unsigned long tskv = 0;
-	struct task_struct *tsk = NULL;
+	struct task_struct *volatile tsk = NULL;
 	struct mm_struct *mm;
 	pgd_t *pgdp;
 	p4d_t *p4dp;
@@ -3299,7 +3299,7 @@ static void show_pte(unsigned long addr)
 static void show_tasks(void)
 {
 	unsigned long tskv;
-	struct task_struct *tsk = NULL;
+	struct task_struct *volatile tsk = NULL;
 
 	printf("     task_struct     ->thread.ksp    ->thread.regs    PID   PPID S  P CMD\n");
 
@@ -3622,7 +3622,7 @@ static void xmon_print_symbol(unsigned long address, const char *mid,
 			      const char *after)
 {
 	char *modname;
-	const char *name = NULL;
+	const char *volatile name = NULL;
 	unsigned long offset, size;
 
 	printf(REG, address);
@@ -4054,7 +4054,7 @@ void xmon_register_spus(struct list_head *list)
 static void stop_spus(void)
 {
 	struct spu *spu;
-	int i;
+	volatile int i;
 	u64 tmp;
 
 	for (i = 0; i < XMON_NUM_SPUS; i++) {
@@ -4095,7 +4095,7 @@ static void stop_spus(void)
 static void restart_spus(void)
 {
 	struct spu *spu;
-	int i;
+	volatile int i;
 
 	for (i = 0; i < XMON_NUM_SPUS; i++) {
 		if (!spu_info[i].spu)
-- 
2.29.2


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

* Re: [PATCH] powerpc: mark local variables around longjmp as volatile
  2021-04-29  8:06 [PATCH] powerpc: mark local variables around longjmp as volatile Arnd Bergmann
@ 2021-06-25 10:35 ` Christophe Leroy
  2021-06-26 10:37 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Christophe Leroy @ 2021-06-25 10:35 UTC (permalink / raw)
  To: Arnd Bergmann, Michael Ellerman
  Cc: Arnd Bergmann, Benjamin Herrenschmidt, Paul Mackerras,
	Nicholas Piggin, Ravi Bangoria, linuxppc-dev, linux-kernel,
	Linux Doc Mailing List



Le 29/04/2021 à 10:06, Arnd Bergmann a écrit :
> From: Arnd Bergmann <arnd@arndb.de>
> 
> gcc-11 points out that modifying local variables next to a
> longjmp/setjmp may cause undefined behavior:
> 
> arch/powerpc/kexec/crash.c: In function 'crash_kexec_prepare_cpus.constprop':
> arch/powerpc/kexec/crash.c:108:22: error: variable 'ncpus' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbere
> d]
> arch/powerpc/kexec/crash.c:109:13: error: variable 'tries' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbere
> d]
> arch/powerpc/xmon/xmon.c: In function 'xmon_print_symbol':
> arch/powerpc/xmon/xmon.c:3625:21: error: variable 'name' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'stop_spus':
> arch/powerpc/xmon/xmon.c:4057:13: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'restart_spus':
> arch/powerpc/xmon/xmon.c:4098:13: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'dump_opal_msglog':
> arch/powerpc/xmon/xmon.c:3008:16: error: variable 'pos' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'show_pte':
> arch/powerpc/xmon/xmon.c:3207:29: error: variable 'tsk' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'show_tasks':
> arch/powerpc/xmon/xmon.c:3302:29: error: variable 'tsk' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'xmon_core':
> arch/powerpc/xmon/xmon.c:494:13: error: variable 'cmd' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c:860:21: error: variable 'bp' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c:860:21: error: variable 'bp' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c:492:48: error: argument 'fromipi' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> 
> According to the documentation, marking these as 'volatile' is
> sufficient to avoid the problem, and it shuts up the warning.


I think this change deserves some comment in the code, and maybe also an update of 
https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html

Otherwise, there's a risk that one day or another, someone removes those 'volatile' markings.

Christophe


> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   arch/powerpc/kexec/crash.c |  4 ++--
>   arch/powerpc/xmon/xmon.c   | 22 +++++++++++-----------
>   2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
> index 0196d0c211ac..10f997e6bb95 100644
> --- a/arch/powerpc/kexec/crash.c
> +++ b/arch/powerpc/kexec/crash.c
> @@ -105,8 +105,8 @@ void crash_ipi_callback(struct pt_regs *regs)
>   static void crash_kexec_prepare_cpus(int cpu)
>   {
>   	unsigned int msecs;
> -	unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
> -	int tries = 0;
> +	volatile unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
> +	volatile int tries = 0;
>   	int (*old_handler)(struct pt_regs *regs);
>   
>   	printk(KERN_EMERG "Sending IPI to other CPUs\n");
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index c8173e92f19d..ce0eacf77645 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -489,10 +489,10 @@ static void xmon_touch_watchdogs(void)
>   	touch_nmi_watchdog();
>   }
>   
> -static int xmon_core(struct pt_regs *regs, int fromipi)
> +static int xmon_core(struct pt_regs *regs, volatile int fromipi)
>   {
> -	int cmd = 0;
> -	struct bpt *bp;
> +	volatile int cmd = 0;
> +	struct bpt *volatile bp;
>   	long recurse_jmp[JMP_BUF_LEN];
>   	bool locked_down;
>   	unsigned long offset;
> @@ -857,7 +857,7 @@ static inline void force_enable_xmon(void)
>   static struct bpt *at_breakpoint(unsigned long pc)
>   {
>   	int i;
> -	struct bpt *bp;
> +	struct bpt *volatile bp;
>   
>   	bp = bpts;
>   	for (i = 0; i < NBPTS; ++i, ++bp)
> @@ -3005,7 +3005,7 @@ static void dump_opal_msglog(void)
>   {
>   	unsigned char buf[128];
>   	ssize_t res;
> -	loff_t pos = 0;
> +	volatile loff_t pos = 0;
>   
>   	if (!firmware_has_feature(FW_FEATURE_OPAL)) {
>   		printf("Machine is not running OPAL firmware.\n");
> @@ -3160,7 +3160,7 @@ memzcan(void)
>   		printf("%.8lx\n", a - mskip);
>   }
>   
> -static void show_task(struct task_struct *tsk)
> +static void show_task(struct task_struct *volatile tsk)
>   {
>   	char state;
>   
> @@ -3204,7 +3204,7 @@ static void format_pte(void *ptep, unsigned long pte)
>   static void show_pte(unsigned long addr)
>   {
>   	unsigned long tskv = 0;
> -	struct task_struct *tsk = NULL;
> +	struct task_struct *volatile tsk = NULL;
>   	struct mm_struct *mm;
>   	pgd_t *pgdp;
>   	p4d_t *p4dp;
> @@ -3299,7 +3299,7 @@ static void show_pte(unsigned long addr)
>   static void show_tasks(void)
>   {
>   	unsigned long tskv;
> -	struct task_struct *tsk = NULL;
> +	struct task_struct *volatile tsk = NULL;
>   
>   	printf("     task_struct     ->thread.ksp    ->thread.regs    PID   PPID S  P CMD\n");
>   
> @@ -3622,7 +3622,7 @@ static void xmon_print_symbol(unsigned long address, const char *mid,
>   			      const char *after)
>   {
>   	char *modname;
> -	const char *name = NULL;
> +	const char *volatile name = NULL;
>   	unsigned long offset, size;
>   
>   	printf(REG, address);
> @@ -4054,7 +4054,7 @@ void xmon_register_spus(struct list_head *list)
>   static void stop_spus(void)
>   {
>   	struct spu *spu;
> -	int i;
> +	volatile int i;
>   	u64 tmp;
>   
>   	for (i = 0; i < XMON_NUM_SPUS; i++) {
> @@ -4095,7 +4095,7 @@ static void stop_spus(void)
>   static void restart_spus(void)
>   {
>   	struct spu *spu;
> -	int i;
> +	volatile int i;
>   
>   	for (i = 0; i < XMON_NUM_SPUS; i++) {
>   		if (!spu_info[i].spu)
> 

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

* Re: [PATCH] powerpc: mark local variables around longjmp as volatile
  2021-04-29  8:06 [PATCH] powerpc: mark local variables around longjmp as volatile Arnd Bergmann
  2021-06-25 10:35 ` Christophe Leroy
@ 2021-06-26 10:37 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2021-06-26 10:37 UTC (permalink / raw)
  To: Michael Ellerman, Arnd Bergmann
  Cc: linuxppc-dev, Paul Mackerras, Ravi Bangoria, Arnd Bergmann,
	Benjamin Herrenschmidt, Christophe Leroy, linux-kernel,
	Nicholas Piggin

On Thu, 29 Apr 2021 10:06:38 +0200, Arnd Bergmann wrote:
> gcc-11 points out that modifying local variables next to a
> longjmp/setjmp may cause undefined behavior:
> 
> arch/powerpc/kexec/crash.c: In function 'crash_kexec_prepare_cpus.constprop':
> arch/powerpc/kexec/crash.c:108:22: error: variable 'ncpus' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbere
> d]
> arch/powerpc/kexec/crash.c:109:13: error: variable 'tries' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbere
> d]
> arch/powerpc/xmon/xmon.c: In function 'xmon_print_symbol':
> arch/powerpc/xmon/xmon.c:3625:21: error: variable 'name' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'stop_spus':
> arch/powerpc/xmon/xmon.c:4057:13: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'restart_spus':
> arch/powerpc/xmon/xmon.c:4098:13: error: variable 'i' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'dump_opal_msglog':
> arch/powerpc/xmon/xmon.c:3008:16: error: variable 'pos' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'show_pte':
> arch/powerpc/xmon/xmon.c:3207:29: error: variable 'tsk' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'show_tasks':
> arch/powerpc/xmon/xmon.c:3302:29: error: variable 'tsk' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c: In function 'xmon_core':
> arch/powerpc/xmon/xmon.c:494:13: error: variable 'cmd' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c:860:21: error: variable 'bp' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c:860:21: error: variable 'bp' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> arch/powerpc/xmon/xmon.c:492:48: error: argument 'fromipi' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc: mark local variables around longjmp as volatile
      https://git.kernel.org/powerpc/c/a2305e3de819394a7adf68078964a92d06f9db33

cheers

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

end of thread, other threads:[~2021-06-26 10:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-29  8:06 [PATCH] powerpc: mark local variables around longjmp as volatile Arnd Bergmann
2021-06-25 10:35 ` Christophe Leroy
2021-06-26 10:37 ` 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).