[RESEND PATCH 0/5] gcov fixes and maybe-uninitialized warnings

[RESEND PATCH 0/5] gcov fixes and maybe-uninitialized warnings

[Arnd Bergmann]

Hi Michal,

This is a resend of a series I originally sent back in February, but
unfortunately I never heard back from you. Could you apply these
patches for v4.7?

[PATCH 1/5] Kbuild: change CC_OPTIMIZE_FOR_SIZE definition
[PATCH 2/5] Kbuild: disable 'maybe-uninitialized' warning for
[PATCH 3/5] gcov: disable for COMPILE_TEST
[PATCH 4/5] gcov: disable tree-loop-im to reduce stack usage
[PATCH 5/5] gcov: disable -Wmaybe-uninitialized warning

I've marked patch 4/5 for stable backports, please decide for
yourself if you want to send it for v4.6 right away, it's
not overly urgent but it does fix real bugs unlike the others
that just address the warning levels.

	Arnd
----
Original description:

This series tries to address two related problems:

* getting better "variable may be used uninitialized" warnings
  in allmodconfig and randconfig builds

* improving GCOV_PROFILE support

Surprisingly, these two are related, so I have a single series that
I hope to get merged through the Kbuild tree.

I have built many thousands of ARM randconfig kernels and created
patches for every single warning and error I found, and submitted
most of them for inclusion. The "may be used uninitialized" warnings
are both the most annoying and the most helpful ones that gcc
gives us, so this tries to make them more useful, including three
steps:

1. Limit the false positives as much as we can: Aside from
   CC_OPTIMIZE_FOR_SIZE, three other options (UBSAN_SANITIZE_ALL,
   PROFILE_ALL_BRANCHES and GCOV_PROFILE_ALL) confuse the
   compiler (in versions 4.9 through 5.3, and to a lesser degree
   on older versions) so we get a lot of extra warnings. We
   already disable the warnings for CC_OPTIMIZE_FOR_SIZE, and
   this series also disables them for the other two, which are
   rarely used in practice but do show up in randconfig builds
   all the time.
2. Ensure we actually see them on 'allmodconfig' builds: Today they
   are disabled, because CC_OPTIMIZE_FOR_SIZE disables them.
   UBSAN_SANITIZE_ALL and PROFILE_ALL_BRANCHES are defined in
   a way that they don't get turned on for allmodconfig, and
   we need to do the samem for GCOV_PROFILE_ALL and
   CC_OPTIMIZE_FOR_SIZE.
3. Fix all known such warnings: The warnings should stick out,
   so we can fix them once they first appear. Today they often
   get ignored because of all the false positives.

For GCOV support, another problem showed up, resulting in an increased
risk for kernel stack overflow, aside from getting a number of
warnings about the stack size. I'm also including a patch to
address that here.

[PATCH 1/5] Kbuild: change CC_OPTIMIZE_FOR_SIZE definition

[Arnd Bergmann]

CC_OPTIMIZE_FOR_SIZE disables the often useful -Wmaybe-unused warning,
because that causes a ridiculous amount of false positives when combined
with -Os.

This means a lot of warnings don't show up in testing by the developers
that should see them with an 'allmodconfig' kernel that has
CC_OPTIMIZE_FOR_SIZE enabled, but only later in randconfig builds
that don't.

This changes the Kconfig logic around CC_OPTIMIZE_FOR_SIZE to make
it a 'choice' statement defaulting to CC_OPTIMIZE_FOR_PERFORMANCE
that gets added for this purpose. The allmodconfig and allyesconfig
kernels now default to -O2 with the maybe-unused warning enabled.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 init/Kconfig | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index 0c666408a8b6..d1b766bf60c1 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1284,6 +1284,17 @@ source "usr/Kconfig"
 
 endif
 
+choice
+	prompt "Compiler optimization level"
+	default CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE
+
+config CC_OPTIMIZE_FOR_PERFORMANCE
+	bool "Optimize for performance"
+	help
+	  This is the default optimization level for the kernel, building
+	  with the "-O2" compiler flag for best performance and most
+	  helpful compile-time warnings.
+
 config CC_OPTIMIZE_FOR_SIZE
 	bool "Optimize for size"
 	help
@@ -1292,6 +1303,8 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+endchoice
+
 config SYSCTL
 	bool
 
-- 
2.7.0

[PATCH 2/5] Kbuild: disable 'maybe-uninitialized' warning for CONFIG_PROFILE_ALL_BRANCHES

[Arnd Bergmann]

CONFIG_PROFILE_ALL_BRANCHES confuses gcc-5.x to the degree that it prints
incorrect warnings about a lot of variables that it thinks can be used
uninitialized, e.g.:

i2c/busses/i2c-diolan-u2c.c: In function 'diolan_usb_xfer':
i2c/busses/i2c-diolan-u2c.c:391:16: warning: 'byte' may be used uninitialized in this function
iio/gyro/itg3200_core.c: In function 'itg3200_probe':
iio/gyro/itg3200_core.c:213:6: warning: 'val' may be used uninitialized in this function
leds/leds-lp55xx-common.c: In function 'lp55xx_update_bits':
leds/leds-lp55xx-common.c:350:6: warning: 'tmp' may be used uninitialized in this function
misc/bmp085.c: In function 'show_pressure':
misc/bmp085.c:363:10: warning: 'pressure' may be used uninitialized in this function
power/ds2782_battery.c: In function 'ds2786_get_capacity':
power/ds2782_battery.c:214:17: warning: 'raw' may be used uninitialized in this function

These are all false positives that either rob someone's time when trying
to figure out whether they are real, or they get people to send wrong
patches to shut up the warnings.

Nobody normally wants to run a CONFIG_PROFILE_ALL_BRANCHES kernel in
production, so disabling the whole class of warnings for this configuration
has no serious downsides either.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Steven Rostedt <rostedtgoodmis.org>
---
 Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index c5a3db5aebd1..e595d1f626b8 100644
--- a/Makefile
+++ b/Makefile
@@ -624,7 +624,11 @@ KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
 KBUILD_CFLAGS	+= -Os $(call cc-disable-warning,maybe-uninitialized,)
 else
-KBUILD_CFLAGS	+= -O2
+ifdef CONFIG_PROFILE_ALL_BRANCHES
+KBUILD_CFLAGS	+= -O2 $(call cc-disable-warning,maybe-uninitialized,)
+else
+KBUILD_CFLAGS   += -O2
+endif
 endif
 
 # Tell gcc to never replace conditional load with a non-conditional one
-- 
2.7.0

[PATCH 3/5] gcov: disable for COMPILE_TEST

[Arnd Bergmann]

Enabling gcov is counterproductive to compile testing: it significantly
increases the kernel image size, compile time, and it produces lots
of false positive "may be used uninitialized" warnings as the result
of missed optimizations.

This is in line with how UBSAN_SANITIZE_ALL and PROFILE_ALL_BRANCHES
work, both of which have similar problems.

With an ARM allmodconfig kernel, I see the build time drop from
283 minutes CPU time to 225 minutes, and the vmlinux size drops
from 43MB to 26MB.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
---
 kernel/gcov/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index c92e44855ddd..1276aabaab55 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -37,6 +37,7 @@ config ARCH_HAS_GCOV_PROFILE_ALL
 
 config GCOV_PROFILE_ALL
 	bool "Profile entire Kernel"
+	depends on !COMPILE_TEST
 	depends on GCOV_KERNEL
 	depends on ARCH_HAS_GCOV_PROFILE_ALL
 	default n
-- 
2.7.0

[PATCH 4/5] gcov: disable tree-loop-im to reduce stack usage

[Arnd Bergmann]

Enabling CONFIG_GCOV_PROFILE_ALL produces us a lot of warnings like

lib/lz4/lz4hc_compress.c: In function 'lz4_compresshcctx':
lib/lz4/lz4hc_compress.c:514:1: warning: the frame size of 1504 bytes is larger than 1024 bytes [-Wframe-larger-than=]

After some investigation, I found that this behavior started with gcc-4.9,
and opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69702.
A suggested workaround for it is to use the -fno-tree-loop-im
flag that turns off one of the optimization stages in gcc, so the
code runs a little slower but does not use excessive amounts
of stack.

We could make this conditional on the gcc version, but I could not
find an easy way to do this in Kbuild and the benefit would be
fairly small, given that most of the gcc version in production are
affected now.

I'm marking this for 'stable' backports because it addresses a bug
with code generation in gcc that exists in all kernel versions
with the affected gcc releases.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e595d1f626b8..d74b38e70d6f 100644
--- a/Makefile
+++ b/Makefile
@@ -364,7 +364,7 @@ AFLAGS_MODULE   =
 LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
-CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage
+CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
 CFLAGS_KCOV	= -fsanitize-coverage=trace-pc
 
 
-- 
2.7.0

[PATCH 5/5] gcov: disable -Wmaybe-uninitialized warning

[Arnd Bergmann]

When gcov profiling is enabled, we see a lot of spurious warnings about
possibly uninitialized variables being used:

arch/arm/mm/dma-mapping.c: In function 'arm_coherent_iommu_map_page':
arch/arm/mm/dma-mapping.c:1085:16: warning: 'start' may be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/clk/st/clk-flexgen.c: In function 'st_of_flexgen_setup':
drivers/clk/st/clk-flexgen.c:323:9: warning: 'num_parents' may be used uninitialized in this function [-Wmaybe-uninitialized]
kernel/cgroup.c: In function 'cgroup_mount':
kernel/cgroup.c:2119:11: warning: 'root' may be used uninitialized in this function [-Wmaybe-uninitialized]

All of these are false positives, so it seems better to just disable
the warnings whenever GCOV is enabled. Most users don't enable GCOV,
and based on a prior patch, it is now also disabled for 'allmodconfig'
builds, so there should be no downsides of doing this.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d74b38e70d6f..b5c67617a98d 100644
--- a/Makefile
+++ b/Makefile
@@ -364,7 +364,7 @@ AFLAGS_MODULE   =
 LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
-CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im -Wno-maybe-uninitialized
 CFLAGS_KCOV	= -fsanitize-coverage=trace-pc
 
 
-- 
2.7.0

Re: [RESEND PATCH 0/5] gcov fixes and maybe-uninitialized warnings

[Michal Marek]

On Mon, Apr 25, 2016 at 05:35:26PM +0200, Arnd Bergmann wrote:
> Hi Michal,
> 
> This is a resend of a series I originally sent back in February, but
> unfortunately I never heard back from you. Could you apply these
> patches for v4.7?
> 
> [PATCH 1/5] Kbuild: change CC_OPTIMIZE_FOR_SIZE definition
> [PATCH 2/5] Kbuild: disable 'maybe-uninitialized' warning for
> [PATCH 3/5] gcov: disable for COMPILE_TEST
> [PATCH 4/5] gcov: disable tree-loop-im to reduce stack usage
> [PATCH 5/5] gcov: disable -Wmaybe-uninitialized warning

Applied now, sorry for the delay.

Michal

Re: [RESEND PATCH 0/5] gcov fixes and maybe-uninitialized warnings

[Arnd Bergmann]

On Tuesday 10 May 2016 17:14:37 Michal Marek wrote:
> On Mon, Apr 25, 2016 at 05:35:26PM +0200, Arnd Bergmann wrote:
> > Hi Michal,
> > 
> > This is a resend of a series I originally sent back in February, but
> > unfortunately I never heard back from you. Could you apply these
> > patches for v4.7?
> > 
> > [PATCH 1/5] Kbuild: change CC_OPTIMIZE_FOR_SIZE definition
> > [PATCH 2/5] Kbuild: disable 'maybe-uninitialized' warning for
> > [PATCH 3/5] gcov: disable for COMPILE_TEST
> > [PATCH 4/5] gcov: disable tree-loop-im to reduce stack usage
> > [PATCH 5/5] gcov: disable -Wmaybe-uninitialized warning
> 
> Applied now, sorry for the delay.

Great, thanks!

	Arnd