Message ID | 20210429080708.1520360-1-arnd@kernel.org (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | powerpc: mark local variables around longjmp as volatile | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (1b628dbc3ccb0b51963fcb3c60b57daac21dc016) |
snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 13 warnings, 0 checks, 87 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
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) >
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
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)