* [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags
@ 2022-03-03 11:07 Vincent Mailhol
2022-03-04 19:18 ` Josh Poimboeuf
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Vincent Mailhol @ 2022-03-03 11:07 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86
Cc: linux-kernel, H . Peter Anvin, Nick Desaulniers, Alexey Dobriyan,
Josh Poimboeuf, Vincent Mailhol
The macro __WARN_FLAGS() uses a local variable named "f". This being a
common name, there is a risk of shadowing other variables.
For example:
| In file included from ./include/linux/bug.h:5,
| from ./include/linux/cpumask.h:14,
| from ./arch/x86/include/asm/cpumask.h:5,
| from ./arch/x86/include/asm/msr.h:11,
| from ./arch/x86/include/asm/processor.h:22,
| from ./arch/x86/include/asm/timex.h:5,
| from ./include/linux/timex.h:65,
| from ./include/linux/time32.h:13,
| from ./include/linux/time.h:60,
| from ./include/linux/stat.h:19,
| from ./include/linux/module.h:13,
| from virt/lib/irqbypass.mod.c:1:
| ./include/linux/rcupdate.h: In function 'rcu_head_after_call_rcu':
| ./arch/x86/include/asm/bug.h:80:21: warning: declaration of 'f' shadows a parameter [-Wshadow]
| 80 | __auto_type f = BUGFLAG_WARNING|(flags); \
| | ^
| ./include/asm-generic/bug.h:106:17: note: in expansion of macro '__WARN_FLAGS'
| 106 | __WARN_FLAGS(BUGFLAG_ONCE | \
| | ^~~~~~~~~~~~
| ./include/linux/rcupdate.h:1007:9: note: in expansion of macro 'WARN_ON_ONCE'
| 1007 | WARN_ON_ONCE(func != (rcu_callback_t)~0L);
| | ^~~~~~~~~~~~
| In file included from ./include/linux/rbtree.h:24,
| from ./include/linux/mm_types.h:11,
| from ./include/linux/buildid.h:5,
| from ./include/linux/module.h:14,
| from virt/lib/irqbypass.mod.c:1:
| ./include/linux/rcupdate.h:1001:62: note: shadowed declaration is here
| 1001 | rcu_head_after_call_rcu(struct rcu_head *rhp, rcu_callback_t f)
| | ~~~~~~~~~~~~~~~^
This patch renames the variable from f to __flags (with two underscore
prefixes as suggested in the Linux kernel coding style [1]) in order
to prevent collisions.
[1] Linux kernel coding style, section 12) Macros, Enums and RTL,
paragraph 5) namespace collisions when defining local variables in
macros resembling functions
https://www.kernel.org/doc/html/v4.10/process/coding-style.html#macros-enums-and-rtl
fixes: bfb1a7c91fb7 ("x86/bug: Merge annotate_reachable() into
_BUG_FLAGS() asm")
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
arch/x86/include/asm/bug.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index bab883c0b6fe..66570e95af39 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -77,9 +77,9 @@ do { \
*/
#define __WARN_FLAGS(flags) \
do { \
- __auto_type f = BUGFLAG_WARNING|(flags); \
+ __auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin(); \
- _BUG_FLAGS(ASM_UD2, f, ASM_REACHABLE); \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE); \
instrumentation_end(); \
} while (0)
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags
2022-03-03 11:07 [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags Vincent Mailhol
@ 2022-03-04 19:18 ` Josh Poimboeuf
2022-03-04 19:26 ` Nick Desaulniers
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Josh Poimboeuf @ 2022-03-04 19:18 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
linux-kernel, H . Peter Anvin, Nick Desaulniers, Alexey Dobriyan
On Thu, Mar 03, 2022 at 08:07:55PM +0900, Vincent Mailhol wrote:
> This patch renames the variable from f to __flags (with two underscore
> prefixes as suggested in the Linux kernel coding style [1]) in order
> to prevent collisions.
>
> [1] Linux kernel coding style, section 12) Macros, Enums and RTL,
> paragraph 5) namespace collisions when defining local variables in
> macros resembling functions
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#macros-enums-and-rtl
Looks good to me.
>
> fixes: bfb1a7c91fb7 ("x86/bug: Merge annotate_reachable() into
> _BUG_FLAGS() asm")
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Fixes tag should be capitalized and in a single line, like:
Fixes: bfb1a7c91fb7 ("x86/bug: Merge annotate_reachable() into _BUG_FLAGS() asm")
Otherwise:
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
--
Josh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags
2022-03-03 11:07 [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags Vincent Mailhol
2022-03-04 19:18 ` Josh Poimboeuf
@ 2022-03-04 19:26 ` Nick Desaulniers
2022-03-05 3:57 ` Vincent MAILHOL
2022-03-17 6:57 ` [RESEND PATCH v2] " Vincent Mailhol
2022-03-24 2:37 ` [PATCH v3] " Vincent Mailhol
3 siblings, 1 reply; 7+ messages in thread
From: Nick Desaulniers @ 2022-03-04 19:26 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
linux-kernel, H . Peter Anvin, Alexey Dobriyan, Josh Poimboeuf
On Thu, Mar 3, 2022 at 3:08 AM Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
>
> The macro __WARN_FLAGS() uses a local variable named "f". This being a
> common name, there is a risk of shadowing other variables.
>
> For example:
>
> | In file included from ./include/linux/bug.h:5,
> | from ./include/linux/cpumask.h:14,
> | from ./arch/x86/include/asm/cpumask.h:5,
> | from ./arch/x86/include/asm/msr.h:11,
> | from ./arch/x86/include/asm/processor.h:22,
> | from ./arch/x86/include/asm/timex.h:5,
> | from ./include/linux/timex.h:65,
> | from ./include/linux/time32.h:13,
> | from ./include/linux/time.h:60,
> | from ./include/linux/stat.h:19,
> | from ./include/linux/module.h:13,
> | from virt/lib/irqbypass.mod.c:1:
> | ./include/linux/rcupdate.h: In function 'rcu_head_after_call_rcu':
> | ./arch/x86/include/asm/bug.h:80:21: warning: declaration of 'f' shadows a parameter [-Wshadow]
> | 80 | __auto_type f = BUGFLAG_WARNING|(flags); \
> | | ^
> | ./include/asm-generic/bug.h:106:17: note: in expansion of macro '__WARN_FLAGS'
> | 106 | __WARN_FLAGS(BUGFLAG_ONCE | \
> | | ^~~~~~~~~~~~
> | ./include/linux/rcupdate.h:1007:9: note: in expansion of macro 'WARN_ON_ONCE'
> | 1007 | WARN_ON_ONCE(func != (rcu_callback_t)~0L);
> | | ^~~~~~~~~~~~
> | In file included from ./include/linux/rbtree.h:24,
> | from ./include/linux/mm_types.h:11,
> | from ./include/linux/buildid.h:5,
> | from ./include/linux/module.h:14,
> | from virt/lib/irqbypass.mod.c:1:
> | ./include/linux/rcupdate.h:1001:62: note: shadowed declaration is here
> | 1001 | rcu_head_after_call_rcu(struct rcu_head *rhp, rcu_callback_t f)
> | | ~~~~~~~~~~~~~~~^
>
> This patch renames the variable from f to __flags (with two underscore
> prefixes as suggested in the Linux kernel coding style [1]) in order
> to prevent collisions.
>
> [1] Linux kernel coding style, section 12) Macros, Enums and RTL,
> paragraph 5) namespace collisions when defining local variables in
> macros resembling functions
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#macros-enums-and-rtl
Ah, thanks for the patch. I missed that coding style recommendation.
Might want to link to a newer (or evergreen) version of the docs
though:
https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
Not sure what happens when we start shadowing variables named __flags,
but maybe we cross that bridge if/when we get there.
Thanks for the fix!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags
2022-03-04 19:26 ` Nick Desaulniers
@ 2022-03-05 3:57 ` Vincent MAILHOL
0 siblings, 0 replies; 7+ messages in thread
From: Vincent MAILHOL @ 2022-03-05 3:57 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
linux-kernel, H . Peter Anvin, Alexey Dobriyan, Josh Poimboeuf
On Tue. 5 Mar 2022 à 04:26, Nick Desaulniers <ndesaulniers@google.com> wrote:
> On Thu, Mar 3, 2022 at 3:08 AM Vincent Mailhol
> <mailhol.vincent@wanadoo.fr> wrote:
> > The macro __WARN_FLAGS() uses a local variable named "f". This being a
> > common name, there is a risk of shadowing other variables.
> >
> > For example:
> >
> > | In file included from ./include/linux/bug.h:5,
> > | from ./include/linux/cpumask.h:14,
> > | from ./arch/x86/include/asm/cpumask.h:5,
> > | from ./arch/x86/include/asm/msr.h:11,
> > | from ./arch/x86/include/asm/processor.h:22,
> > | from ./arch/x86/include/asm/timex.h:5,
> > | from ./include/linux/timex.h:65,
> > | from ./include/linux/time32.h:13,
> > | from ./include/linux/time.h:60,
> > | from ./include/linux/stat.h:19,
> > | from ./include/linux/module.h:13,
> > | from virt/lib/irqbypass.mod.c:1:
> > | ./include/linux/rcupdate.h: In function 'rcu_head_after_call_rcu':
> > | ./arch/x86/include/asm/bug.h:80:21: warning: declaration of 'f' shadows a parameter [-Wshadow]
> > | 80 | __auto_type f = BUGFLAG_WARNING|(flags); \
> > | | ^
> > | ./include/asm-generic/bug.h:106:17: note: in expansion of macro '__WARN_FLAGS'
> > | 106 | __WARN_FLAGS(BUGFLAG_ONCE | \
> > | | ^~~~~~~~~~~~
> > | ./include/linux/rcupdate.h:1007:9: note: in expansion of macro 'WARN_ON_ONCE'
> > | 1007 | WARN_ON_ONCE(func != (rcu_callback_t)~0L);
> > | | ^~~~~~~~~~~~
> > | In file included from ./include/linux/rbtree.h:24,
> > | from ./include/linux/mm_types.h:11,
> > | from ./include/linux/buildid.h:5,
> > | from ./include/linux/module.h:14,
> > | from virt/lib/irqbypass.mod.c:1:
> > | ./include/linux/rcupdate.h:1001:62: note: shadowed declaration is here
> > | 1001 | rcu_head_after_call_rcu(struct rcu_head *rhp, rcu_callback_t f)
> > | | ~~~~~~~~~~~~~~~^
> >
> > This patch renames the variable from f to __flags (with two underscore
> > prefixes as suggested in the Linux kernel coding style [1]) in order
> > to prevent collisions.
> >
> > [1] Linux kernel coding style, section 12) Macros, Enums and RTL,
> > paragraph 5) namespace collisions when defining local variables in
> > macros resembling functions
> > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#macros-enums-and-rtl
Indeed! Will update this in v2 (and will also fix the Fixes: tag as
pointed by Josh).
> Ah, thanks for the patch. I missed that coding style recommendation.
> Might want to link to a newer (or evergreen) version of the docs
> though:
> https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
> Not sure what happens when we start shadowing variables named __flags,
> but maybe we cross that bridge if/when we get there.
Ack.
> Thanks for the fix!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RESEND PATCH v2] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags
2022-03-03 11:07 [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags Vincent Mailhol
2022-03-04 19:18 ` Josh Poimboeuf
2022-03-04 19:26 ` Nick Desaulniers
@ 2022-03-17 6:57 ` Vincent Mailhol
2022-03-24 2:37 ` [PATCH v3] " Vincent Mailhol
3 siblings, 0 replies; 7+ messages in thread
From: Vincent Mailhol @ 2022-03-17 6:57 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin
Cc: linux-kernel, Vincent Mailhol, Josh Poimboeuf, Nick Desaulniers
The macro __WARN_FLAGS() uses a local variable named "f". This being a
common name, there is a risk of shadowing other variables.
For example:
| In file included from ./include/linux/bug.h:5,
| from ./include/linux/cpumask.h:14,
| from ./arch/x86/include/asm/cpumask.h:5,
| from ./arch/x86/include/asm/msr.h:11,
| from ./arch/x86/include/asm/processor.h:22,
| from ./arch/x86/include/asm/timex.h:5,
| from ./include/linux/timex.h:65,
| from ./include/linux/time32.h:13,
| from ./include/linux/time.h:60,
| from ./include/linux/stat.h:19,
| from ./include/linux/module.h:13,
| from virt/lib/irqbypass.mod.c:1:
| ./include/linux/rcupdate.h: In function 'rcu_head_after_call_rcu':
| ./arch/x86/include/asm/bug.h:80:21: warning: declaration of 'f' shadows a parameter [-Wshadow]
| 80 | __auto_type f = BUGFLAG_WARNING|(flags); \
| | ^
| ./include/asm-generic/bug.h:106:17: note: in expansion of macro '__WARN_FLAGS'
| 106 | __WARN_FLAGS(BUGFLAG_ONCE | \
| | ^~~~~~~~~~~~
| ./include/linux/rcupdate.h:1007:9: note: in expansion of macro 'WARN_ON_ONCE'
| 1007 | WARN_ON_ONCE(func != (rcu_callback_t)~0L);
| | ^~~~~~~~~~~~
| In file included from ./include/linux/rbtree.h:24,
| from ./include/linux/mm_types.h:11,
| from ./include/linux/buildid.h:5,
| from ./include/linux/module.h:14,
| from virt/lib/irqbypass.mod.c:1:
| ./include/linux/rcupdate.h:1001:62: note: shadowed declaration is here
| 1001 | rcu_head_after_call_rcu(struct rcu_head *rhp, rcu_callback_t f)
| | ~~~~~~~~~~~~~~~^
This patch renames the variable from f to __flags (with two underscore
prefixes as suggested in the Linux kernel coding style [1]) in order
to prevent collisions.
[1] Linux kernel coding style, section 12) Macros, Enums and RTL,
paragraph 5) namespace collisions when defining local variables in
macros resembling functions
https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
Fixes: bfb1a7c91fb7 ("x86/bug: Merge annotate_reachable() into_BUG_FLAGS() asm")
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
Changelog:
v1 -> v2:
* Update the reference link to the kernel documentation to latest version.
* Do not break the Fixes: tag to a new line.
* Added the Acked-by and Reviewed-by tags.
---
arch/x86/include/asm/bug.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index bab883c0b6fe..66570e95af39 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -77,9 +77,9 @@ do { \
*/
#define __WARN_FLAGS(flags) \
do { \
- __auto_type f = BUGFLAG_WARNING|(flags); \
+ __auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin(); \
- _BUG_FLAGS(ASM_UD2, f, ASM_REACHABLE); \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE); \
instrumentation_end(); \
} while (0)
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags
2022-03-03 11:07 [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags Vincent Mailhol
` (2 preceding siblings ...)
2022-03-17 6:57 ` [RESEND PATCH v2] " Vincent Mailhol
@ 2022-03-24 2:37 ` Vincent Mailhol
2022-04-05 8:29 ` [tip: x86/urgent] x86/bug: Prevent shadowing in __WARN_FLAGS tip-bot2 for Vincent Mailhol
3 siblings, 1 reply; 7+ messages in thread
From: Vincent Mailhol @ 2022-03-24 2:37 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin
Cc: linux-kernel, adobriyan, jpoimboe, linux-sparse, llvm,
luc.vanoostenryck, nathan, peterz, Sasha Levin, Vincent Mailhol,
Charlemagne Lasse, Nick Desaulniers
The macro __WARN_FLAGS() uses a local variable named "f". This being a
common name, there is a risk of shadowing other variables.
For example, GCC would yield:
| In file included from ./include/linux/bug.h:5,
| from ./include/linux/cpumask.h:14,
| from ./arch/x86/include/asm/cpumask.h:5,
| from ./arch/x86/include/asm/msr.h:11,
| from ./arch/x86/include/asm/processor.h:22,
| from ./arch/x86/include/asm/timex.h:5,
| from ./include/linux/timex.h:65,
| from ./include/linux/time32.h:13,
| from ./include/linux/time.h:60,
| from ./include/linux/stat.h:19,
| from ./include/linux/module.h:13,
| from virt/lib/irqbypass.mod.c:1:
| ./include/linux/rcupdate.h: In function 'rcu_head_after_call_rcu':
| ./arch/x86/include/asm/bug.h:80:21: warning: declaration of 'f' shadows a parameter [-Wshadow]
| 80 | __auto_type f = BUGFLAG_WARNING|(flags); \
| | ^
| ./include/asm-generic/bug.h:106:17: note: in expansion of macro '__WARN_FLAGS'
| 106 | __WARN_FLAGS(BUGFLAG_ONCE | \
| | ^~~~~~~~~~~~
| ./include/linux/rcupdate.h:1007:9: note: in expansion of macro 'WARN_ON_ONCE'
| 1007 | WARN_ON_ONCE(func != (rcu_callback_t)~0L);
| | ^~~~~~~~~~~~
| In file included from ./include/linux/rbtree.h:24,
| from ./include/linux/mm_types.h:11,
| from ./include/linux/buildid.h:5,
| from ./include/linux/module.h:14,
| from virt/lib/irqbypass.mod.c:1:
| ./include/linux/rcupdate.h:1001:62: note: shadowed declaration is here
| 1001 | rcu_head_after_call_rcu(struct rcu_head *rhp, rcu_callback_t f)
| | ~~~~~~~~~~~~~~~^
For reference, sparse also warns about it, c.f. [1].
This patch renames the variable from f to __flags (with two underscore
prefixes as suggested in the Linux kernel coding style [2]) in order
to prevent collisions.
[1] https://lore.kernel.org/all/CAFGhKbyifH1a+nAMCvWM88TK6fpNPdzFtUXPmRGnnQeePV+1sw@mail.gmail.com/
[2] Linux kernel coding style, section 12) Macros, Enums and RTL,
paragraph 5) namespace collisions when defining local variables in
macros resembling functions
https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
CC: Charlemagne Lasse <charlemagnelasse@gmail.com>
Fixes: bfb1a7c91fb7 ("x86/bug: Merge annotate_reachable() into_BUG_FLAGS() asm")
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
Changelog:
v2 -> v3:
* add a mention that sparse is warning about this too.
c.f. https://lore.kernel.org/all/CAKwvOdmSV3Nse+tGMBXvN=QvnOs6-ODZRJB0OF5Pd6BVb-scFw@mail.gmail.com/
v1 -> v2:
* Update the reference link to the kernel documentation to latest version.
* Do not break the Fixes: tag to a new line.
* Added the Acked-by and Reviewed-by tags.
---
arch/x86/include/asm/bug.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index bab883c0b6fe..66570e95af39 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -77,9 +77,9 @@ do { \
*/
#define __WARN_FLAGS(flags) \
do { \
- __auto_type f = BUGFLAG_WARNING|(flags); \
+ __auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin(); \
- _BUG_FLAGS(ASM_UD2, f, ASM_REACHABLE); \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE); \
instrumentation_end(); \
} while (0)
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip: x86/urgent] x86/bug: Prevent shadowing in __WARN_FLAGS
2022-03-24 2:37 ` [PATCH v3] " Vincent Mailhol
@ 2022-04-05 8:29 ` tip-bot2 for Vincent Mailhol
0 siblings, 0 replies; 7+ messages in thread
From: tip-bot2 for Vincent Mailhol @ 2022-04-05 8:29 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vincent Mailhol, Peter Zijlstra (Intel),
Nick Desaulniers, Josh Poimboeuf, x86, linux-kernel
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 9ce02f0fc68326dd1f87a0a3a4c6ae7fdd39e6f6
Gitweb: https://git.kernel.org/tip/9ce02f0fc68326dd1f87a0a3a4c6ae7fdd39e6f6
Author: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
AuthorDate: Thu, 24 Mar 2022 11:37:42 +09:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Apr 2022 10:24:40 +02:00
x86/bug: Prevent shadowing in __WARN_FLAGS
The macro __WARN_FLAGS() uses a local variable named "f". This being a
common name, there is a risk of shadowing other variables.
For example, GCC would yield:
| In file included from ./include/linux/bug.h:5,
| from ./include/linux/cpumask.h:14,
| from ./arch/x86/include/asm/cpumask.h:5,
| from ./arch/x86/include/asm/msr.h:11,
| from ./arch/x86/include/asm/processor.h:22,
| from ./arch/x86/include/asm/timex.h:5,
| from ./include/linux/timex.h:65,
| from ./include/linux/time32.h:13,
| from ./include/linux/time.h:60,
| from ./include/linux/stat.h:19,
| from ./include/linux/module.h:13,
| from virt/lib/irqbypass.mod.c:1:
| ./include/linux/rcupdate.h: In function 'rcu_head_after_call_rcu':
| ./arch/x86/include/asm/bug.h:80:21: warning: declaration of 'f' shadows a parameter [-Wshadow]
| 80 | __auto_type f = BUGFLAG_WARNING|(flags); \
| | ^
| ./include/asm-generic/bug.h:106:17: note: in expansion of macro '__WARN_FLAGS'
| 106 | __WARN_FLAGS(BUGFLAG_ONCE | \
| | ^~~~~~~~~~~~
| ./include/linux/rcupdate.h:1007:9: note: in expansion of macro 'WARN_ON_ONCE'
| 1007 | WARN_ON_ONCE(func != (rcu_callback_t)~0L);
| | ^~~~~~~~~~~~
| In file included from ./include/linux/rbtree.h:24,
| from ./include/linux/mm_types.h:11,
| from ./include/linux/buildid.h:5,
| from ./include/linux/module.h:14,
| from virt/lib/irqbypass.mod.c:1:
| ./include/linux/rcupdate.h:1001:62: note: shadowed declaration is here
| 1001 | rcu_head_after_call_rcu(struct rcu_head *rhp, rcu_callback_t f)
| | ~~~~~~~~~~~~~~~^
For reference, sparse also warns about it, c.f. [1].
This patch renames the variable from f to __flags (with two underscore
prefixes as suggested in the Linux kernel coding style [2]) in order
to prevent collisions.
[1] https://lore.kernel.org/all/CAFGhKbyifH1a+nAMCvWM88TK6fpNPdzFtUXPmRGnnQeePV+1sw@mail.gmail.com/
[2] Linux kernel coding style, section 12) Macros, Enums and RTL,
paragraph 5) namespace collisions when defining local variables in
macros resembling functions
https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
Fixes: bfb1a7c91fb7 ("x86/bug: Merge annotate_reachable() into_BUG_FLAGS() asm")
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Link: https://lkml.kernel.org/r/20220324023742.106546-1-mailhol.vincent@wanadoo.fr
---
arch/x86/include/asm/bug.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index 4d20a29..aaf0cb0 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -78,9 +78,9 @@ do { \
*/
#define __WARN_FLAGS(flags) \
do { \
- __auto_type f = BUGFLAG_WARNING|(flags); \
+ __auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin(); \
- _BUG_FLAGS(ASM_UD2, f, ASM_REACHABLE); \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE); \
instrumentation_end(); \
} while (0)
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-04-05 10:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-03 11:07 [PATCH] x86/bug: __WARN_FLAGS: prevent shadowing by renaming local variable f to __flags Vincent Mailhol
2022-03-04 19:18 ` Josh Poimboeuf
2022-03-04 19:26 ` Nick Desaulniers
2022-03-05 3:57 ` Vincent MAILHOL
2022-03-17 6:57 ` [RESEND PATCH v2] " Vincent Mailhol
2022-03-24 2:37 ` [PATCH v3] " Vincent Mailhol
2022-04-05 8:29 ` [tip: x86/urgent] x86/bug: Prevent shadowing in __WARN_FLAGS tip-bot2 for Vincent Mailhol
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.