* [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS @ 2018-10-17 0:08 Leonardo Brás 2018-10-17 4:32 ` [Lkcamp] " Helen Koike 2018-10-17 8:11 ` Borislav Petkov 0 siblings, 2 replies; 16+ messages in thread From: Leonardo Brás @ 2018-10-17 0:08 UTC (permalink / raw) To: lkcamp Cc: Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, Masahiro Yamada, Michal Marek, linux-kernel, linux-kbuild Adds -Wshadow=local on KBUILD_HOSTCFLAGS to show shadow warnings on tools built for HOST. Signed-off-by: Leonardo Brás <leobras.c@gmail.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e8b599b4dcde..fb0a9ac195e7 100644 --- a/Makefile +++ b/Makefile @@ -360,7 +360,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) HOSTCC = gcc HOSTCXX = g++ -KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ +KBUILD_HOSTCFLAGS := -Wall -Wshadow=local -Wmissing-prototypes -Wstrict-prototypes -O2 \ -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \ $(HOSTCFLAGS) KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) -- 2.19.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Lkcamp] [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-17 0:08 [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS Leonardo Brás @ 2018-10-17 4:32 ` Helen Koike 2018-10-17 23:18 ` Leonardo Bras 2018-10-17 8:11 ` Borislav Petkov 1 sibling, 1 reply; 16+ messages in thread From: Helen Koike @ 2018-10-17 4:32 UTC (permalink / raw) To: Leonardo Brás, lkcamp Cc: x86, linux-kbuild, Matthew Wilcox, linux-kernel, Masahiro Yamada, Ingo Molnar, Borislav Petkov, Andy Lutomirski, H. Peter Anvin, Michal Marek, Thomas Gleixner Hi Leonardo, Thanks for the patch. On 10/16/18 9:08 PM, Leonardo Brás wrote: > Adds -Wshadow=local on KBUILD_HOSTCFLAGS to show shadow warnings > on tools built for HOST. > > Signed-off-by: Leonardo Brás <leobras.c@gmail.com> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index e8b599b4dcde..fb0a9ac195e7 100644 > --- a/Makefile > +++ b/Makefile > @@ -360,7 +360,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) > > HOSTCC = gcc > HOSTCXX = g++ > -KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > +KBUILD_HOSTCFLAGS := -Wall -Wshadow=local -Wmissing-prototypes -Wstrict-prototypes -O2 \ > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \ > $(HOSTCFLAGS) > KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) > I believe this should be the last patch on this series and not the first one to avoid commits in between where we have those warnings. Regards Helen ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Lkcamp] [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-17 4:32 ` [Lkcamp] " Helen Koike @ 2018-10-17 23:18 ` Leonardo Bras 0 siblings, 0 replies; 16+ messages in thread From: Leonardo Bras @ 2018-10-17 23:18 UTC (permalink / raw) To: helen Cc: lkcamp, x86, linux-kbuild, Matthew Wilcox, linux-kernel, Masahiro Yamada, Ingo Molnar, Borislav Petkov, Andy Lutomirski, H. Peter Anvin, Michal Marek, Thomas Gleixner Hello Helen, On Wed, Oct 17, 2018 at 1:32 AM Helen Koike <helen@koikeco.de> wrote: > > Hi Leonardo, > > Thanks for the patch. > > On 10/16/18 9:08 PM, Leonardo Brás wrote: > > Adds -Wshadow=local on KBUILD_HOSTCFLAGS to show shadow warnings > > on tools built for HOST. > > > > Signed-off-by: Leonardo Brás <leobras.c@gmail.com> > > --- > > Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index e8b599b4dcde..fb0a9ac195e7 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -360,7 +360,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) > > > > HOSTCC = gcc > > HOSTCXX = g++ > > -KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > > +KBUILD_HOSTCFLAGS := -Wall -Wshadow=local -Wmissing-prototypes -Wstrict-prototypes -O2 \ > > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \ > > $(HOSTCFLAGS) > > KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) > > > > I believe this should be the last patch on this series and not the first > one to avoid commits in between where we have those warnings. > You are right, I will change the order for v2. Thanks! > Regards > Helen Regards, Leonardo ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-17 0:08 [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS Leonardo Brás 2018-10-17 4:32 ` [Lkcamp] " Helen Koike @ 2018-10-17 8:11 ` Borislav Petkov 2018-10-17 8:21 ` Masahiro Yamada 1 sibling, 1 reply; 16+ messages in thread From: Borislav Petkov @ 2018-10-17 8:11 UTC (permalink / raw) To: Leonardo Brás Cc: lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Masahiro Yamada, Michal Marek, linux-kernel, linux-kbuild On Tue, Oct 16, 2018 at 09:08:09PM -0300, Leonardo Brás wrote: > Adds -Wshadow=local on KBUILD_HOSTCFLAGS to show shadow warnings > on tools built for HOST. > > Signed-off-by: Leonardo Brás <leobras.c@gmail.com> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index e8b599b4dcde..fb0a9ac195e7 100644 > --- a/Makefile > +++ b/Makefile > @@ -360,7 +360,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) > > HOSTCC = gcc > HOSTCXX = g++ > -KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > +KBUILD_HOSTCFLAGS := -Wall -Wshadow=local -Wmissing-prototypes -Wstrict-prototypes -O2 \ > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \ > $(HOSTCFLAGS) > KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) > -- You might wanna take a look at scripts/Makefile.extrawarn which already has -Wshadow. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-17 8:11 ` Borislav Petkov @ 2018-10-17 8:21 ` Masahiro Yamada 2018-10-17 8:31 ` Borislav Petkov 2018-10-18 0:36 ` Leonardo Bras 0 siblings, 2 replies; 16+ messages in thread From: Masahiro Yamada @ 2018-10-17 8:21 UTC (permalink / raw) To: Borislav Petkov Cc: Leonardo Brás, lkcamp, willy6545, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, Michal Marek, Linux Kernel Mailing List, Linux Kbuild mailing list On Wed, Oct 17, 2018 at 5:11 PM Borislav Petkov <bp@alien8.de> wrote: > > On Tue, Oct 16, 2018 at 09:08:09PM -0300, Leonardo Brás wrote: > > Adds -Wshadow=local on KBUILD_HOSTCFLAGS to show shadow warnings > > on tools built for HOST. > > > > Signed-off-by: Leonardo Brás <leobras.c@gmail.com> > > --- > > Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index e8b599b4dcde..fb0a9ac195e7 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -360,7 +360,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) > > > > HOSTCC = gcc > > HOSTCXX = g++ > > -KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > > +KBUILD_HOSTCFLAGS := -Wall -Wshadow=local -Wmissing-prototypes -Wstrict-prototypes -O2 \ > > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \ > > $(HOSTCFLAGS) > > KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) > > -- > > You might wanna take a look at scripts/Makefile.extrawarn which already > has -Wshadow. scripts/Makefile.extrawarn provides options for the target compiler (CC), whereas this patch adds -Wshadow=local for the host compiler (HOSTCC). -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-17 8:21 ` Masahiro Yamada @ 2018-10-17 8:31 ` Borislav Petkov 2018-10-18 0:40 ` Leonardo Bras 2018-10-18 0:36 ` Leonardo Bras 1 sibling, 1 reply; 16+ messages in thread From: Borislav Petkov @ 2018-10-17 8:31 UTC (permalink / raw) To: Masahiro Yamada Cc: Leonardo Brás, lkcamp, willy6545, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, Michal Marek, Linux Kernel Mailing List, Linux Kbuild mailing list On Wed, Oct 17, 2018 at 05:21:01PM +0900, Masahiro Yamada wrote: > scripts/Makefile.extrawarn provides options for the target compiler (CC), > whereas this patch adds -Wshadow=local for the host compiler (HOSTCC). Aha, this wants to fix shadowing for the host tools, ok. Next question: why not -Wshadow simply? Also, -Wshadow for the target compiler is an extra warning (W=2). Why is it enabled by default here? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-17 8:31 ` Borislav Petkov @ 2018-10-18 0:40 ` Leonardo Bras 2018-10-18 9:16 ` Borislav Petkov 0 siblings, 1 reply; 16+ messages in thread From: Leonardo Bras @ 2018-10-18 0:40 UTC (permalink / raw) To: Borislav Petkov Cc: Masahiro Yamada, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Michal Marek, linux-kernel, linux-kbuild Hello Boris, > Next question: why not -Wshadow simply? Good question. I can change it to -Wshadow and fix stuff for v2. > > Also, -Wshadow for the target compiler is an extra warning (W=2). Why is > it enabled by default here? The idea was to put it as default and fix all the shadowing warnings. What do you think? I am open to suggestions. Thank you, Leonardo Bras > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-18 0:40 ` Leonardo Bras @ 2018-10-18 9:16 ` Borislav Petkov 2018-10-18 16:39 ` Masahiro Yamada 0 siblings, 1 reply; 16+ messages in thread From: Borislav Petkov @ 2018-10-18 9:16 UTC (permalink / raw) To: Leonardo Bras Cc: Masahiro Yamada, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Michal Marek, linux-kernel, linux-kbuild On Wed, Oct 17, 2018 at 09:40:53PM -0300, Leonardo Bras wrote: > The idea was to put it as default and fix all the shadowing warnings. > What do you think? I am open to suggestions. That's Masahiro's call. In the rest of the kernel, those warnings are behind the W=2 switch - i.e., not enabled by default. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-18 9:16 ` Borislav Petkov @ 2018-10-18 16:39 ` Masahiro Yamada 2018-10-18 16:50 ` Borislav Petkov 2018-10-19 11:28 ` David Laight 0 siblings, 2 replies; 16+ messages in thread From: Masahiro Yamada @ 2018-10-18 16:39 UTC (permalink / raw) To: Borislav Petkov Cc: Leonardo Brás, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, Michal Marek, Linux Kernel Mailing List, Linux Kbuild mailing list On Thu, Oct 18, 2018 at 6:18 PM Borislav Petkov <bp@alien8.de> wrote: > > On Wed, Oct 17, 2018 at 09:40:53PM -0300, Leonardo Bras wrote: > > The idea was to put it as default and fix all the shadowing warnings. > > What do you think? I am open to suggestions. > > That's Masahiro's call. In the rest of the kernel, those warnings are behind > the W=2 switch - i.e., not enabled by default. It is not realistic to enable this warning option by default. Even -Wshadow=local emits tons of warnings. (More with -Wshadow) The problem of this flag is, it is false positive in macro expansions. For example, I think the following is a legitimate case. In file included from ./arch/arm64/include/asm/cputype.h:126:0, from ./arch/arm64/include/asm/cache.h:19, from ./include/linux/cache.h:6, from ./include/linux/printk.h:9, from ./include/linux/kernel.h:14, from ./include/linux/bitmap.h:10, from arch/arm64/kernel/fpsimd.c:20: arch/arm64/kernel/fpsimd.c: In function ‘sve_kernel_enable’: ./arch/arm64/include/asm/sysreg.h:707:6: warning: declaration of ‘__val’ shadows a previous local [-Wshadow=compatible-local] u64 __val; \ ^ ./arch/arm64/include/asm/sysreg.h:717:20: note: in definition of macro ‘write_sysreg’ u64 __val = (u64)(v); \ ^ arch/arm64/kernel/fpsimd.c:713:15: note: in expansion of macro ‘read_sysreg’ write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1); ^~~~~~~~~~~ ./arch/arm64/include/asm/sysreg.h:717:6: note: shadowed declaration is here u64 __val = (u64)(v); \ ^ arch/arm64/kernel/fpsimd.c:713:2: note: in expansion of macro ‘write_sysreg’ write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1); ^~~~~~~~~~~~ -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-18 16:39 ` Masahiro Yamada @ 2018-10-18 16:50 ` Borislav Petkov 2018-10-19 2:41 ` Masahiro Yamada 2018-10-19 11:28 ` David Laight 1 sibling, 1 reply; 16+ messages in thread From: Borislav Petkov @ 2018-10-18 16:50 UTC (permalink / raw) To: Masahiro Yamada Cc: Leonardo Brás, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, Michal Marek, Linux Kernel Mailing List, Linux Kbuild mailing list On Fri, Oct 19, 2018 at 01:39:21AM +0900, Masahiro Yamada wrote: > It is not realistic to enable this warning option by default. I believe the question is whether to enable that warning by default in KBUILD_HOSTCFLAGS. Enabling it by default for the target kernel is of course, too noisy. That's why it is hidden behind W=2 there. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-18 16:50 ` Borislav Petkov @ 2018-10-19 2:41 ` Masahiro Yamada 2018-10-19 2:50 ` Leonardo Bras 2018-10-19 8:08 ` Borislav Petkov 0 siblings, 2 replies; 16+ messages in thread From: Masahiro Yamada @ 2018-10-19 2:41 UTC (permalink / raw) To: Borislav Petkov Cc: Leonardo Brás, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, Michal Marek, Linux Kernel Mailing List, Linux Kbuild mailing list On Fri, Oct 19, 2018 at 1:53 AM Borislav Petkov <bp@alien8.de> wrote: > > On Fri, Oct 19, 2018 at 01:39:21AM +0900, Masahiro Yamada wrote: > > It is not realistic to enable this warning option by default. > > I believe the question is whether to enable that warning by default in > KBUILD_HOSTCFLAGS. Enabling it by default for the target kernel is of > course, too noisy. That's why it is hidden behind W=2 there. Sorry, I misunderstood the question. The difference about the noisiness between CC and HOSTCC is just comes from the amount of source code. Adding -Wshadow to KBUILD_HOSTCFLAGS emits another warning in Kconfig. Of course, it is easy to fix. But, I just started to think this option is a kind of harsh... -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-19 2:41 ` Masahiro Yamada @ 2018-10-19 2:50 ` Leonardo Bras 2018-10-19 8:08 ` Borislav Petkov 1 sibling, 0 replies; 16+ messages in thread From: Leonardo Bras @ 2018-10-19 2:50 UTC (permalink / raw) To: Masahiro Yamada Cc: Borislav Petkov, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Michal Marek, linux-kernel, linux-kbuild On Thu, Oct 18, 2018 at 11:42 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Adding -Wshadow to KBUILD_HOSTCFLAGS emits another warning in Kconfig. > Of course, it is easy to fix. For v2, I already replaced '-Wshadow=local' for '-Wshadow' and fixed this warning. > But, I just started to think this option is a kind of harsh... The v2 it's almost done. You think it will be useful, or should I drop it? Regards, Leonardo Bras ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-19 2:41 ` Masahiro Yamada 2018-10-19 2:50 ` Leonardo Bras @ 2018-10-19 8:08 ` Borislav Petkov 1 sibling, 0 replies; 16+ messages in thread From: Borislav Petkov @ 2018-10-19 8:08 UTC (permalink / raw) To: Masahiro Yamada Cc: Leonardo Brás, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, Michal Marek, Linux Kernel Mailing List, Linux Kbuild mailing list On Fri, Oct 19, 2018 at 11:41:31AM +0900, Masahiro Yamada wrote: > Adding -Wshadow to KBUILD_HOSTCFLAGS emits another warning in Kconfig. > Of course, it is easy to fix. > But, I just started to think this option is a kind of harsh... What is more, if we added -Wshadow to KBUILD_HOSTCFLAGS, then there'll be a difference in build options between host and target kernel in that the host kernel build will be stricter wrt shadowing. Thus, it is a maintainer decision, IMHO. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-18 16:39 ` Masahiro Yamada 2018-10-18 16:50 ` Borislav Petkov @ 2018-10-19 11:28 ` David Laight 2018-10-28 16:54 ` Masahiro Yamada 1 sibling, 1 reply; 16+ messages in thread From: David Laight @ 2018-10-19 11:28 UTC (permalink / raw) To: 'Masahiro Yamada', Borislav Petkov Cc: Leonardo Brás, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, Michal Marek, Linux Kernel Mailing List, Linux Kbuild mailing list From: Masahiro Yamada > Sent: 18 October 2018 17:39 > > On Thu, Oct 18, 2018 at 6:18 PM Borislav Petkov <bp@alien8.de> wrote: > > > > On Wed, Oct 17, 2018 at 09:40:53PM -0300, Leonardo Bras wrote: > > > The idea was to put it as default and fix all the shadowing warnings. > > > What do you think? I am open to suggestions. > > > > That's Masahiro's call. In the rest of the kernel, those warnings are behind > > the W=2 switch - i.e., not enabled by default. > > > It is not realistic to enable this warning option by default. > Even -Wshadow=local emits tons of warnings. > (More with -Wshadow) The question is, how many of them are actual bugs. IMHO -Wshadow is a good idea. > The problem of this flag is, > it is false positive in macro expansions. Right, but macro expansions inside macro definitions could accidentally use the same local variable - leading to choas. > For example, I think the following is a legitimate case. ... > arch/arm64/kernel/fpsimd.c:713:2: note: in expansion of macro ‘write_sysreg’ > write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1); > ^~~~~~~~~~~~ Easily fixed by using different named temporaries in the two macros. There probably aren't that many macro pairs where that happens. Especially since many are now inlined functions. It might be that a small number of changes get rid of most of the warnings. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-19 11:28 ` David Laight @ 2018-10-28 16:54 ` Masahiro Yamada 0 siblings, 0 replies; 16+ messages in thread From: Masahiro Yamada @ 2018-10-28 16:54 UTC (permalink / raw) To: David.Laight Cc: Borislav Petkov, Leonardo Brás, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, Michal Marek, Linux Kernel Mailing List, Linux Kbuild mailing list On Fri, Oct 19, 2018 at 8:28 PM David Laight <David.Laight@aculab.com> wrote: > > From: Masahiro Yamada > > Sent: 18 October 2018 17:39 > > > > On Thu, Oct 18, 2018 at 6:18 PM Borislav Petkov <bp@alien8.de> wrote: > > > > > > On Wed, Oct 17, 2018 at 09:40:53PM -0300, Leonardo Bras wrote: > > > > The idea was to put it as default and fix all the shadowing warnings. > > > > What do you think? I am open to suggestions. > > > > > > That's Masahiro's call. In the rest of the kernel, those warnings are behind > > > the W=2 switch - i.e., not enabled by default. > > > > > > It is not realistic to enable this warning option by default. > > Even -Wshadow=local emits tons of warnings. > > (More with -Wshadow) > > The question is, how many of them are actual bugs. > IMHO -Wshadow is a good idea. > > > The problem of this flag is, > > it is false positive in macro expansions. > > Right, but macro expansions inside macro definitions could accidentally > use the same local variable - leading to choas. I do not think so. The macro definitions are surrounded by { ... } so that local variables are properly separated from the outside world. > > For example, I think the following is a legitimate case. > ... > > arch/arm64/kernel/fpsimd.c:713:2: note: in expansion of macro ‘write_sysreg’ > > write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1); > > ^~~~~~~~~~~~ > > Easily fixed by using different named temporaries in the two macros. > There probably aren't that many macro pairs where that happens. > Especially since many are now inlined functions. But, theoretically, any arbitrary macros could be used in pairs. This means a new constraint where a local variable name must be unique, it means 'local variable' is not literally 'local'. I'd like to use short names such as 'x', 'tmp', etc. for local variables. > It might be that a small number of changes get rid of most of the warnings. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS 2018-10-17 8:21 ` Masahiro Yamada 2018-10-17 8:31 ` Borislav Petkov @ 2018-10-18 0:36 ` Leonardo Bras 1 sibling, 0 replies; 16+ messages in thread From: Leonardo Bras @ 2018-10-18 0:36 UTC (permalink / raw) To: Masahiro Yamada Cc: Borislav Petkov, lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Michal Marek, linux-kernel, linux-kbuild On Wed, Oct 17, 2018 at 5:21 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Wed, Oct 17, 2018 at 5:11 PM Borislav Petkov <bp@alien8.de> wrote: > > > > On Tue, Oct 16, 2018 at 09:08:09PM -0300, Leonardo Brás wrote: > > > Adds -Wshadow=local on KBUILD_HOSTCFLAGS to show shadow warnings > > > on tools built for HOST. > > > > > > Signed-off-by: Leonardo Brás <leobras.c@gmail.com> > > > --- > > > Makefile | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/Makefile b/Makefile > > > index e8b599b4dcde..fb0a9ac195e7 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -360,7 +360,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) > > > > > > HOSTCC = gcc > > > HOSTCXX = g++ > > > -KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > > > +KBUILD_HOSTCFLAGS := -Wall -Wshadow=local -Wmissing-prototypes -Wstrict-prototypes -O2 \ > > > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \ > > > $(HOSTCFLAGS) > > > KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) > > > -- > > > > You might wanna take a look at scripts/Makefile.extrawarn which already > > has -Wshadow. > > > scripts/Makefile.extrawarn provides options for the target compiler (CC), > whereas this patch adds -Wshadow=local for the host compiler (HOSTCC). > > Thanks for helping, :) Leonardo Bras > > -- > Best Regards > Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-10-28 16:55 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-17 0:08 [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS Leonardo Brás 2018-10-17 4:32 ` [Lkcamp] " Helen Koike 2018-10-17 23:18 ` Leonardo Bras 2018-10-17 8:11 ` Borislav Petkov 2018-10-17 8:21 ` Masahiro Yamada 2018-10-17 8:31 ` Borislav Petkov 2018-10-18 0:40 ` Leonardo Bras 2018-10-18 9:16 ` Borislav Petkov 2018-10-18 16:39 ` Masahiro Yamada 2018-10-18 16:50 ` Borislav Petkov 2018-10-19 2:41 ` Masahiro Yamada 2018-10-19 2:50 ` Leonardo Bras 2018-10-19 8:08 ` Borislav Petkov 2018-10-19 11:28 ` David Laight 2018-10-28 16:54 ` Masahiro Yamada 2018-10-18 0:36 ` Leonardo Bras
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).