linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig
@ 2019-02-21  4:13 Masahiro Yamada
  2019-02-21  5:10 ` Nathan Chancellor
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Masahiro Yamada @ 2019-02-21  4:13 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nick Desaulniers, Nathan Chancellor, Arnd Bergmann,
	Masahiro Yamada, Steven Rostedt, linux-kernel, Michal Marek,
	Ingo Molnar

Since -Wmaybe-uninitialized was introduced by GCC 4.7, we have patched
various false positives:

 - commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized when building
   with -Os") turned off this option for -Os.

 - commit 815eb71e7149 ("Kbuild: disable 'maybe-uninitialized' warning
   for CONFIG_PROFILE_ALL_BRANCHES") turned off this option for
   CONFIG_PROFILE_ALL_BRANCHES

 - commit a76bcf557ef4 ("Kbuild: enable -Wmaybe-uninitialized warning
   for "make W=1"") turned off this option for GCC < 4.9
   Arnd provided more explanation in https://lkml.org/lkml/2017/3/14/903

I think this looks better by shifting the logic from Makefile to Kconfig.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Makefile             | 10 +++-------
 init/Kconfig         | 17 +++++++++++++++++
 kernel/trace/Kconfig |  1 +
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 1bb0535..b21aa2e3 100644
--- a/Makefile
+++ b/Makefile
@@ -656,17 +656,13 @@ KBUILD_CFLAGS	+= $(call cc-disable-warning, int-in-bool-context)
 
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
 KBUILD_CFLAGS	+= $(call cc-option,-Oz,-Os)
-KBUILD_CFLAGS	+= $(call cc-disable-warning,maybe-uninitialized,)
-else
-ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS	+= -O2 $(call cc-disable-warning,maybe-uninitialized,)
 else
 KBUILD_CFLAGS   += -O2
 endif
-endif
 
-KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
-			$(call cc-disable-warning,maybe-uninitialized,))
+ifdef CONFIG_CC_DISABLE_WARN_MAYBE_UNINITIALIZED
+KBUILD_CFLAGS   += -Wno-maybe-uninitialized
+endif
 
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
diff --git a/init/Kconfig b/init/Kconfig
index c9386a3..1f05a88 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -26,6 +26,22 @@ config CLANG_VERSION
 config CC_HAS_ASM_GOTO
 	def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
 
+config CC_HAS_WARN_MAYBE_UNINITIALIZED
+	def_bool $(cc-option,-Wmaybe-uninitialized)
+	help
+	  GCC >= 4.7 supports this option.
+
+config CC_DISABLE_WARN_MAYBE_UNINITIALIZED
+	bool
+	depends on CC_HAS_WARN_MAYBE_UNINITIALIZED
+	default CC_IS_GCC && GCC_VERSION < 40900  # unreliable for GCC < 4.9
+	help
+	  GCC's -Wmaybe-uninitialized is not reliable by definition.
+	  Lots of false positive warnings are produced in some cases.
+
+	  If this option is enabled, -Wno-maybe-uninitialzed is passed
+	  to the compiler to suppress maybe-uninitialized warnings.
+
 config CONSTRUCTORS
 	bool
 	depends on !UML
@@ -1113,6 +1129,7 @@ config CC_OPTIMIZE_FOR_PERFORMANCE
 
 config CC_OPTIMIZE_FOR_SIZE
 	bool "Optimize for size"
+	imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED  # avoid false positives
 	help
 	  Enabling this option will pass "-Os" instead of "-O2" to
 	  your compiler resulting in a smaller kernel.
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index fa8b1fe..8bd1d6d 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -370,6 +370,7 @@ config PROFILE_ANNOTATED_BRANCHES
 config PROFILE_ALL_BRANCHES
 	bool "Profile all if conditionals" if !FORTIFY_SOURCE
 	select TRACE_BRANCH_PROFILING
+	imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED  # avoid false positives
 	help
 	  This tracer profiles all branch conditions. Every if ()
 	  taken in the kernel is recorded whether it hit or miss.
-- 
2.7.4


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

* Re: [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig
  2019-02-21  4:13 [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig Masahiro Yamada
@ 2019-02-21  5:10 ` Nathan Chancellor
  2019-02-21  9:21 ` Arnd Bergmann
  2019-02-22  0:52 ` Nick Desaulniers
  2 siblings, 0 replies; 7+ messages in thread
From: Nathan Chancellor @ 2019-02-21  5:10 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Nick Desaulniers, Arnd Bergmann, Steven Rostedt,
	linux-kernel, Michal Marek, Ingo Molnar

On Thu, Feb 21, 2019 at 01:13:38PM +0900, Masahiro Yamada wrote:
> Since -Wmaybe-uninitialized was introduced by GCC 4.7, we have patched
> various false positives:
> 
>  - commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized when building
>    with -Os") turned off this option for -Os.
> 
>  - commit 815eb71e7149 ("Kbuild: disable 'maybe-uninitialized' warning
>    for CONFIG_PROFILE_ALL_BRANCHES") turned off this option for
>    CONFIG_PROFILE_ALL_BRANCHES
> 
>  - commit a76bcf557ef4 ("Kbuild: enable -Wmaybe-uninitialized warning
>    for "make W=1"") turned off this option for GCC < 4.9
>    Arnd provided more explanation in https://lkml.org/lkml/2017/3/14/903
> 
> I think this looks better by shifting the logic from Makefile to Kconfig.
> 

I agree!

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

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

> ---
> 
>  Makefile             | 10 +++-------
>  init/Kconfig         | 17 +++++++++++++++++
>  kernel/trace/Kconfig |  1 +
>  3 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1bb0535..b21aa2e3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -656,17 +656,13 @@ KBUILD_CFLAGS	+= $(call cc-disable-warning, int-in-bool-context)
>  
>  ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
>  KBUILD_CFLAGS	+= $(call cc-option,-Oz,-Os)
> -KBUILD_CFLAGS	+= $(call cc-disable-warning,maybe-uninitialized,)
> -else
> -ifdef CONFIG_PROFILE_ALL_BRANCHES
> -KBUILD_CFLAGS	+= -O2 $(call cc-disable-warning,maybe-uninitialized,)
>  else
>  KBUILD_CFLAGS   += -O2
>  endif
> -endif
>  
> -KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
> -			$(call cc-disable-warning,maybe-uninitialized,))
> +ifdef CONFIG_CC_DISABLE_WARN_MAYBE_UNINITIALIZED
> +KBUILD_CFLAGS   += -Wno-maybe-uninitialized
> +endif
>  
>  # Tell gcc to never replace conditional load with a non-conditional one
>  KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
> diff --git a/init/Kconfig b/init/Kconfig
> index c9386a3..1f05a88 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -26,6 +26,22 @@ config CLANG_VERSION
>  config CC_HAS_ASM_GOTO
>  	def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
>  
> +config CC_HAS_WARN_MAYBE_UNINITIALIZED
> +	def_bool $(cc-option,-Wmaybe-uninitialized)
> +	help
> +	  GCC >= 4.7 supports this option.
> +
> +config CC_DISABLE_WARN_MAYBE_UNINITIALIZED
> +	bool
> +	depends on CC_HAS_WARN_MAYBE_UNINITIALIZED
> +	default CC_IS_GCC && GCC_VERSION < 40900  # unreliable for GCC < 4.9
> +	help
> +	  GCC's -Wmaybe-uninitialized is not reliable by definition.
> +	  Lots of false positive warnings are produced in some cases.
> +
> +	  If this option is enabled, -Wno-maybe-uninitialzed is passed
> +	  to the compiler to suppress maybe-uninitialized warnings.
> +
>  config CONSTRUCTORS
>  	bool
>  	depends on !UML
> @@ -1113,6 +1129,7 @@ config CC_OPTIMIZE_FOR_PERFORMANCE
>  
>  config CC_OPTIMIZE_FOR_SIZE
>  	bool "Optimize for size"
> +	imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED  # avoid false positives
>  	help
>  	  Enabling this option will pass "-Os" instead of "-O2" to
>  	  your compiler resulting in a smaller kernel.
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index fa8b1fe..8bd1d6d 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -370,6 +370,7 @@ config PROFILE_ANNOTATED_BRANCHES
>  config PROFILE_ALL_BRANCHES
>  	bool "Profile all if conditionals" if !FORTIFY_SOURCE
>  	select TRACE_BRANCH_PROFILING
> +	imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED  # avoid false positives
>  	help
>  	  This tracer profiles all branch conditions. Every if ()
>  	  taken in the kernel is recorded whether it hit or miss.
> -- 
> 2.7.4
> 

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

* Re: [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig
  2019-02-21  4:13 [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig Masahiro Yamada
  2019-02-21  5:10 ` Nathan Chancellor
@ 2019-02-21  9:21 ` Arnd Bergmann
  2019-02-21  9:45   ` Masahiro Yamada
  2019-02-22  0:52 ` Nick Desaulniers
  2 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2019-02-21  9:21 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Nick Desaulniers, Nathan Chancellor,
	Steven Rostedt, Linux Kernel Mailing List, Michal Marek,
	Ingo Molnar

On Thu, Feb 21, 2019 at 5:14 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Since -Wmaybe-uninitialized was introduced by GCC 4.7, we have patched
> various false positives:
>
>  - commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized when building
>    with -Os") turned off this option for -Os.
>
>  - commit 815eb71e7149 ("Kbuild: disable 'maybe-uninitialized' warning
>    for CONFIG_PROFILE_ALL_BRANCHES") turned off this option for
>    CONFIG_PROFILE_ALL_BRANCHES
>
>  - commit a76bcf557ef4 ("Kbuild: enable -Wmaybe-uninitialized warning
>    for "make W=1"") turned off this option for GCC < 4.9
>    Arnd provided more explanation in https://lkml.org/lkml/2017/3/14/903
>
> I think this looks better by shifting the logic from Makefile to Kconfig.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

I like how this cleans up the logic and documents it better.
I've had my own plans to move this (and most other conditional warning
options) into a header file using the KBUILD_WARN() infrastructure, but
never got around to doing that. I still think that's what we want eventually,
but in the meantime your patch seems fine as well.

Also, I see that your patch keeps the existing logic, which is good,
but we may want to refine this a little afterwards:

- we may want to turn off -Wuninitialized for gcc-4.6 and gcc-4.7
  instead of turning off -Wmaybe-uninitialized as we do in the later
  compilers.

- the proposed CC_OPTIMIZE_FOR_DEBUGGING may need the
  same workaround as CC_OPTIMZE_FOR_SIZE.

- For clang, we may want to /not/ turn off -Wmaybe-uninitialized
  in all cases. I haven't tried build with with 'clang -Oz
  -Wmaybe-uninitialized', but if the number of false positives is small
  enough, we could decide to just leave that on.

      Arnd

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

* Re: [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig
  2019-02-21  9:21 ` Arnd Bergmann
@ 2019-02-21  9:45   ` Masahiro Yamada
  2019-02-21 13:16     ` Arnd Bergmann
  0 siblings, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2019-02-21  9:45 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux Kbuild mailing list, Nick Desaulniers, Nathan Chancellor,
	Steven Rostedt, Linux Kernel Mailing List, Michal Marek,
	Ingo Molnar

On Thu, Feb 21, 2019 at 6:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Feb 21, 2019 at 5:14 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > Since -Wmaybe-uninitialized was introduced by GCC 4.7, we have patched
> > various false positives:
> >
> >  - commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized when building
> >    with -Os") turned off this option for -Os.
> >
> >  - commit 815eb71e7149 ("Kbuild: disable 'maybe-uninitialized' warning
> >    for CONFIG_PROFILE_ALL_BRANCHES") turned off this option for
> >    CONFIG_PROFILE_ALL_BRANCHES
> >
> >  - commit a76bcf557ef4 ("Kbuild: enable -Wmaybe-uninitialized warning
> >    for "make W=1"") turned off this option for GCC < 4.9
> >    Arnd provided more explanation in https://lkml.org/lkml/2017/3/14/903
> >
> > I think this looks better by shifting the logic from Makefile to Kconfig.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>
> I like how this cleans up the logic and documents it better.
> I've had my own plans to move this (and most other conditional warning
> options) into a header file using the KBUILD_WARN() infrastructure, but
> never got around to doing that. I still think that's what we want eventually,
> but in the meantime your patch seems fine as well.
>
> Also, I see that your patch keeps the existing logic, which is good,
> but we may want to refine this a little afterwards:
>
> - we may want to turn off -Wuninitialized for gcc-4.6 and gcc-4.7
>   instead of turning off -Wmaybe-uninitialized as we do in the later
>   compilers.

Ah, right.

I can do that if it is worthwhile.

But, I'd rather want to raise the minimum compiler version
than taking care of old compilers.
(We will do it eventually.)


> - the proposed CC_OPTIMIZE_FOR_DEBUGGING may need the
>   same workaround as CC_OPTIMZE_FOR_SIZE.


I know this.

First, my patch took care of CC_OPTIMIZE_FOR_DEBUGGING as well.
But, CC_OPTIMIZE_FOR_DEBUGGING was rejected.
(https://patchwork.kernel.org/patch/10744541/)

So, I dropped this.


> - For clang, we may want to /not/ turn off -Wmaybe-uninitialized
>   in all cases. I haven't tried build with with 'clang -Oz
>   -Wmaybe-uninitialized', but if the number of false positives is small
>   enough, we could decide to just leave that on.


Does clang recognize -Wmaybe-uninitialized?

I am using Clang I built a few days ago.


masahiro@pug:~$ clang --version
clang version 9.0.0 (http://llvm.org/git/clang.git
b492883ea31e2046d5725126318911465bc1374f)
(http://llvm.org/git/llvm.git
03a15eec9ef44798cd5c57b4ed6a63e62b2c1db6)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/masahiro/toolchains/clang-latest/bin

masahiro@pug:~$ clang -x c /dev/null  -Wmaybe-uninitialized -c -o  /dev/null
warning: unknown warning option '-Wmaybe-uninitialized'; did you mean
      '-Wuninitialized'? [-Wunknown-warning-option]
1 warning generated.




>       Arnd



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig
  2019-02-21  9:45   ` Masahiro Yamada
@ 2019-02-21 13:16     ` Arnd Bergmann
  0 siblings, 0 replies; 7+ messages in thread
From: Arnd Bergmann @ 2019-02-21 13:16 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Nick Desaulniers, Nathan Chancellor,
	Steven Rostedt, Linux Kernel Mailing List, Michal Marek,
	Ingo Molnar

On Thu, Feb 21, 2019 at 10:45 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> On Thu, Feb 21, 2019 at 6:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Feb 21, 2019 at 5:14 AM Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> >
> > I like how this cleans up the logic and documents it better.
> > I've had my own plans to move this (and most other conditional warning
> > options) into a header file using the KBUILD_WARN() infrastructure, but
> > never got around to doing that. I still think that's what we want eventually,
> > but in the meantime your patch seems fine as well.
> >
> > Also, I see that your patch keeps the existing logic, which is good,
> > but we may want to refine this a little afterwards:
> >
> > - we may want to turn off -Wuninitialized for gcc-4.6 and gcc-4.7
> >   instead of turning off -Wmaybe-uninitialized as we do in the later
> >   compilers.
>
> Ah, right.
>
> I can do that if it is worthwhile.
>
> But, I'd rather want to raise the minimum compiler version
> than taking care of old compilers.
> (We will do it eventually.)

I was hoping that with gcc-4.6 being the minimum version, we could
actually get all supported compilers to produce clean builds on
all configurations, possibly at the cost of just turning off all
unreliable warning options on older compilers.

I don't mind regularly raising the minimum version, but I would
also wait several years before I'd consider gcc-4.9 a good
choice for the minimum, and all older versions have problems
here.

> Does clang recognize -Wmaybe-uninitialized?
>
> I am using Clang I built a few days ago.
>
>
> masahiro@pug:~$ clang --version
> clang version 9.0.0 (http://llvm.org/git/clang.git
> b492883ea31e2046d5725126318911465bc1374f)
> (http://llvm.org/git/llvm.git
> 03a15eec9ef44798cd5c57b4ed6a63e62b2c1db6)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> InstalledDir: /home/masahiro/toolchains/clang-latest/bin
>
> masahiro@pug:~$ clang -x c /dev/null  -Wmaybe-uninitialized -c -o  /dev/null
> warning: unknown warning option '-Wmaybe-uninitialized'; did you mean
>       '-Wuninitialized'? [-Wunknown-warning-option]
> 1 warning generated.

Ah, that's right. I also see -Wconditional-uninitialized and
-Wsometimes-uninitialized, but they are something different again.

       Arnd

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

* Re: [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig
  2019-02-21  4:13 [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig Masahiro Yamada
  2019-02-21  5:10 ` Nathan Chancellor
  2019-02-21  9:21 ` Arnd Bergmann
@ 2019-02-22  0:52 ` Nick Desaulniers
  2019-02-27 12:44   ` Masahiro Yamada
  2 siblings, 1 reply; 7+ messages in thread
From: Nick Desaulniers @ 2019-02-22  0:52 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Nathan Chancellor, Arnd Bergmann,
	Steven Rostedt, LKML, Michal Marek, Ingo Molnar

On Wed, Feb 20, 2019 at 8:14 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Since -Wmaybe-uninitialized was introduced by GCC 4.7, we have patched
> various false positives:
>
>  - commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized when building
>    with -Os") turned off this option for -Os.
>
>  - commit 815eb71e7149 ("Kbuild: disable 'maybe-uninitialized' warning
>    for CONFIG_PROFILE_ALL_BRANCHES") turned off this option for
>    CONFIG_PROFILE_ALL_BRANCHES
>
>  - commit a76bcf557ef4 ("Kbuild: enable -Wmaybe-uninitialized warning
>    for "make W=1"") turned off this option for GCC < 4.9
>    Arnd provided more explanation in https://lkml.org/lkml/2017/3/14/903
>
> I think this looks better by shifting the logic from Makefile to Kconfig.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

I verified that mainline+this patch did not regression arm64
allyesconfig clang build.
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Thanks for the patch! Would you mind adding a link tag to:
Link: https://github.com/ClangBuiltLinux/linux/issues/350

> ---
>
>  Makefile             | 10 +++-------
>  init/Kconfig         | 17 +++++++++++++++++
>  kernel/trace/Kconfig |  1 +
>  3 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 1bb0535..b21aa2e3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -656,17 +656,13 @@ KBUILD_CFLAGS     += $(call cc-disable-warning, int-in-bool-context)
>
>  ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
>  KBUILD_CFLAGS  += $(call cc-option,-Oz,-Os)
> -KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
> -else
> -ifdef CONFIG_PROFILE_ALL_BRANCHES
> -KBUILD_CFLAGS  += -O2 $(call cc-disable-warning,maybe-uninitialized,)
>  else
>  KBUILD_CFLAGS   += -O2
>  endif
> -endif
>
> -KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
> -                       $(call cc-disable-warning,maybe-uninitialized,))
> +ifdef CONFIG_CC_DISABLE_WARN_MAYBE_UNINITIALIZED
> +KBUILD_CFLAGS   += -Wno-maybe-uninitialized
> +endif
>
>  # Tell gcc to never replace conditional load with a non-conditional one
>  KBUILD_CFLAGS  += $(call cc-option,--param=allow-store-data-races=0)
> diff --git a/init/Kconfig b/init/Kconfig
> index c9386a3..1f05a88 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -26,6 +26,22 @@ config CLANG_VERSION
>  config CC_HAS_ASM_GOTO
>         def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
>
> +config CC_HAS_WARN_MAYBE_UNINITIALIZED
> +       def_bool $(cc-option,-Wmaybe-uninitialized)
> +       help
> +         GCC >= 4.7 supports this option.
> +
> +config CC_DISABLE_WARN_MAYBE_UNINITIALIZED
> +       bool
> +       depends on CC_HAS_WARN_MAYBE_UNINITIALIZED
> +       default CC_IS_GCC && GCC_VERSION < 40900  # unreliable for GCC < 4.9
> +       help
> +         GCC's -Wmaybe-uninitialized is not reliable by definition.
> +         Lots of false positive warnings are produced in some cases.
> +
> +         If this option is enabled, -Wno-maybe-uninitialzed is passed
> +         to the compiler to suppress maybe-uninitialized warnings.
> +
>  config CONSTRUCTORS
>         bool
>         depends on !UML
> @@ -1113,6 +1129,7 @@ config CC_OPTIMIZE_FOR_PERFORMANCE
>
>  config CC_OPTIMIZE_FOR_SIZE
>         bool "Optimize for size"
> +       imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED  # avoid false positives
>         help
>           Enabling this option will pass "-Os" instead of "-O2" to
>           your compiler resulting in a smaller kernel.
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index fa8b1fe..8bd1d6d 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -370,6 +370,7 @@ config PROFILE_ANNOTATED_BRANCHES
>  config PROFILE_ALL_BRANCHES
>         bool "Profile all if conditionals" if !FORTIFY_SOURCE
>         select TRACE_BRANCH_PROFILING
> +       imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED  # avoid false positives
>         help
>           This tracer profiles all branch conditions. Every if ()
>           taken in the kernel is recorded whether it hit or miss.
> --
> 2.7.4
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig
  2019-02-22  0:52 ` Nick Desaulniers
@ 2019-02-27 12:44   ` Masahiro Yamada
  0 siblings, 0 replies; 7+ messages in thread
From: Masahiro Yamada @ 2019-02-27 12:44 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Linux Kbuild mailing list, Nathan Chancellor, Arnd Bergmann,
	Steven Rostedt, LKML, Michal Marek, Ingo Molnar

On Fri, Feb 22, 2019 at 9:53 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Feb 20, 2019 at 8:14 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > Since -Wmaybe-uninitialized was introduced by GCC 4.7, we have patched
> > various false positives:
> >
> >  - commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized when building
> >    with -Os") turned off this option for -Os.
> >
> >  - commit 815eb71e7149 ("Kbuild: disable 'maybe-uninitialized' warning
> >    for CONFIG_PROFILE_ALL_BRANCHES") turned off this option for
> >    CONFIG_PROFILE_ALL_BRANCHES
> >
> >  - commit a76bcf557ef4 ("Kbuild: enable -Wmaybe-uninitialized warning
> >    for "make W=1"") turned off this option for GCC < 4.9
> >    Arnd provided more explanation in https://lkml.org/lkml/2017/3/14/903
> >
> > I think this looks better by shifting the logic from Makefile to Kconfig.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>
> I verified that mainline+this patch did not regression arm64
> allyesconfig clang build.
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> Thanks for the patch! Would you mind adding a link tag to:
> Link: https://github.com/ClangBuiltLinux/linux/issues/350

I added the tags, and applied to linux-kbuild.



> > ---
> >
> >  Makefile             | 10 +++-------
> >  init/Kconfig         | 17 +++++++++++++++++
> >  kernel/trace/Kconfig |  1 +
> >  3 files changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 1bb0535..b21aa2e3 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -656,17 +656,13 @@ KBUILD_CFLAGS     += $(call cc-disable-warning, int-in-bool-context)
> >
> >  ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
> >  KBUILD_CFLAGS  += $(call cc-option,-Oz,-Os)
> > -KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
> > -else
> > -ifdef CONFIG_PROFILE_ALL_BRANCHES
> > -KBUILD_CFLAGS  += -O2 $(call cc-disable-warning,maybe-uninitialized,)
> >  else
> >  KBUILD_CFLAGS   += -O2
> >  endif
> > -endif
> >
> > -KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
> > -                       $(call cc-disable-warning,maybe-uninitialized,))
> > +ifdef CONFIG_CC_DISABLE_WARN_MAYBE_UNINITIALIZED
> > +KBUILD_CFLAGS   += -Wno-maybe-uninitialized
> > +endif
> >
> >  # Tell gcc to never replace conditional load with a non-conditional one
> >  KBUILD_CFLAGS  += $(call cc-option,--param=allow-store-data-races=0)
> > diff --git a/init/Kconfig b/init/Kconfig
> > index c9386a3..1f05a88 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -26,6 +26,22 @@ config CLANG_VERSION
> >  config CC_HAS_ASM_GOTO
> >         def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
> >
> > +config CC_HAS_WARN_MAYBE_UNINITIALIZED
> > +       def_bool $(cc-option,-Wmaybe-uninitialized)
> > +       help
> > +         GCC >= 4.7 supports this option.
> > +
> > +config CC_DISABLE_WARN_MAYBE_UNINITIALIZED
> > +       bool
> > +       depends on CC_HAS_WARN_MAYBE_UNINITIALIZED
> > +       default CC_IS_GCC && GCC_VERSION < 40900  # unreliable for GCC < 4.9
> > +       help
> > +         GCC's -Wmaybe-uninitialized is not reliable by definition.
> > +         Lots of false positive warnings are produced in some cases.
> > +
> > +         If this option is enabled, -Wno-maybe-uninitialzed is passed
> > +         to the compiler to suppress maybe-uninitialized warnings.
> > +
> >  config CONSTRUCTORS
> >         bool
> >         depends on !UML
> > @@ -1113,6 +1129,7 @@ config CC_OPTIMIZE_FOR_PERFORMANCE
> >
> >  config CC_OPTIMIZE_FOR_SIZE
> >         bool "Optimize for size"
> > +       imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED  # avoid false positives
> >         help
> >           Enabling this option will pass "-Os" instead of "-O2" to
> >           your compiler resulting in a smaller kernel.
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index fa8b1fe..8bd1d6d 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -370,6 +370,7 @@ config PROFILE_ANNOTATED_BRANCHES
> >  config PROFILE_ALL_BRANCHES
> >         bool "Profile all if conditionals" if !FORTIFY_SOURCE
> >         select TRACE_BRANCH_PROFILING
> > +       imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED  # avoid false positives
> >         help
> >           This tracer profiles all branch conditions. Every if ()
> >           taken in the kernel is recorded whether it hit or miss.
> > --
> > 2.7.4
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Best Regards
Masahiro Yamada

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

end of thread, other threads:[~2019-02-27 12:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-21  4:13 [PATCH] kbuild: compute false-positive -Wmaybe-uninitialized cases in Kconfig Masahiro Yamada
2019-02-21  5:10 ` Nathan Chancellor
2019-02-21  9:21 ` Arnd Bergmann
2019-02-21  9:45   ` Masahiro Yamada
2019-02-21 13:16     ` Arnd Bergmann
2019-02-22  0:52 ` Nick Desaulniers
2019-02-27 12:44   ` Masahiro Yamada

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).