linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os
@ 2013-03-15 14:55 Arnd Bergmann
  2013-03-15 18:12 ` Russell King - ARM Linux
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2013-03-15 14:55 UTC (permalink / raw)
  To: linux-arm-kernel

I forgot to put the linux-kbuild list on Cc, that is probably the
best place to discuss this patch.

----------  Forwarded Message  ----------

Subject: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os
Date: Thursday 14 March 2013
From: Arnd Bergmann <arnd@arndb.de>
To: linux-arch at vger.kernel.org, linux-arm-kernel at lists.infradead.org, linux-kernel at vger.kernel.org

gcc-4.7 and higher add a lot of false positive warnings about
potential uses of uninitialized warnings, but only when optimizing
for size (-Os). This is the default when building allyesconfig,
which turns on CONFIG_CC_OPTIMIZE_FOR_SIZE.

In order to avoid getting a lot of patches that initialize such
variables and accidentally hide real errors along the way, let's
just turn off this warning on the respective gcc versions
when building with size optimizations. The -Wmaybe-uninitialized
option was introduced in the same gcc version (4.7) that is now
causing the false positives, so there is no effect on older compilers.

A side effect is that when building with CONFIG_CC_OPTIMIZE_FOR_SIZE,
we might now see /fewer/ warnings about possibly uninitialized
warnings than with -O2, but that is still much better than seeing
warnings known to be bogus.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: stable at vger.kernel.org
--
I'd like to merge this for 3.9 and also for the stable kernels,
if people agree this is a good idea.

diff --git a/Makefile b/Makefile
index 10fb6c7..caea2d1 100644
--- a/Makefile
+++ b/Makefile
@@ -570,7 +570,7 @@ endif # $(dot-config)
 all: vmlinux
 
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS	+= -Os
+KBUILD_CFLAGS	+= -Os $(call cc-disable-warning,maybe-uninitialized,)
 else
 KBUILD_CFLAGS	+= -O2
 endif

--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-------------------------------------------------------

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

* Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os
  2013-03-15 14:55 Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os Arnd Bergmann
@ 2013-03-15 18:12 ` Russell King - ARM Linux
  2013-03-15 19:43   ` Arnd Bergmann
  0 siblings, 1 reply; 4+ messages in thread
From: Russell King - ARM Linux @ 2013-03-15 18:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Mar 15, 2013 at 02:55:38PM +0000, Arnd Bergmann wrote:
> I forgot to put the linux-kbuild list on Cc, that is probably the
> best place to discuss this patch.
> 
> ----------  Forwarded Message  ----------
> 
> Subject: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os
> Date: Thursday 14 March 2013
> From: Arnd Bergmann <arnd@arndb.de>
> To: linux-arch at vger.kernel.org, linux-arm-kernel at lists.infradead.org, linux-kernel at vger.kernel.org
> 
> gcc-4.7 and higher add a lot of false positive warnings about
> potential uses of uninitialized warnings, but only when optimizing
> for size (-Os). This is the default when building allyesconfig,
> which turns on CONFIG_CC_OPTIMIZE_FOR_SIZE.
> 
> In order to avoid getting a lot of patches that initialize such
> variables and accidentally hide real errors along the way, let's
> just turn off this warning on the respective gcc versions
> when building with size optimizations. The -Wmaybe-uninitialized
> option was introduced in the same gcc version (4.7) that is now
> causing the false positives, so there is no effect on older compilers.
> 
> A side effect is that when building with CONFIG_CC_OPTIMIZE_FOR_SIZE,
> we might now see /fewer/ warnings about possibly uninitialized
> warnings than with -O2, but that is still much better than seeing
> warnings known to be bogus.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: stable at vger.kernel.org
> --
> I'd like to merge this for 3.9 and also for the stable kernels,
> if people agree this is a good idea.

I think I replied to your previous version recently asking whether
this affects real uninitialized variables too.

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

* Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os
  2013-03-15 18:12 ` Russell King - ARM Linux
@ 2013-03-15 19:43   ` Arnd Bergmann
  2013-03-16  8:56     ` James Bottomley
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2013-03-15 19:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 15 March 2013, Russell King - ARM Linux wrote:
> On Fri, Mar 15, 2013 at 02:55:38PM +0000, Arnd Bergmann wrote:

> > I'd like to merge this for 3.9 and also for the stable kernels,
> > if people agree this is a good idea.
> 
> I think I replied to your previous version recently asking whether
> this affects real uninitialized variables too.

If gcc can prove that there is a code path in which the variable is
used uninitialized, it will still warn with this patch, since we are
leaving -Wuninitialized enabled but only disable -Wmaybe-uninitilized.
There are obviously some cases where gcc correctly warns today but
cannot prove whether or not this is actually possible. I don't have
any data about how often we'd see one or the other, but I would expect
the first one to be more common.

We'd also still see all valid warnings with the Kconfig default of
building with -O2 rather than -Os, and as gcc gets smarter over time,
it should show more of the real bugs with -Wuninitialized.

I think the real trade-off is that not applying this patch will cause
more patches to get merged that add bogus initializations, which
definitely prevent gcc from warning about a real uninitialized
variable bug in that function again. I have done some of those
patches myself in the past, but it always feels really wrong to
do those.

	Arnd

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

* Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os
  2013-03-15 19:43   ` Arnd Bergmann
@ 2013-03-16  8:56     ` James Bottomley
  0 siblings, 0 replies; 4+ messages in thread
From: James Bottomley @ 2013-03-16  8:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 2013-03-15 at 19:43 +0000, Arnd Bergmann wrote:
> On Friday 15 March 2013, Russell King - ARM Linux wrote:
> > On Fri, Mar 15, 2013 at 02:55:38PM +0000, Arnd Bergmann wrote:
> 
> > > I'd like to merge this for 3.9 and also for the stable kernels,
> > > if people agree this is a good idea.
> > 
> > I think I replied to your previous version recently asking whether
> > this affects real uninitialized variables too.
> 
> If gcc can prove that there is a code path in which the variable is
> used uninitialized, it will still warn with this patch, since we are
> leaving -Wuninitialized enabled but only disable -Wmaybe-uninitilized.
> There are obviously some cases where gcc correctly warns today but
> cannot prove whether or not this is actually possible. I don't have
> any data about how often we'd see one or the other, but I would expect
> the first one to be more common.
> 
> We'd also still see all valid warnings with the Kconfig default of
> building with -O2 rather than -Os, and as gcc gets smarter over time,
> it should show more of the real bugs with -Wuninitialized.
> 
> I think the real trade-off is that not applying this patch will cause
> more patches to get merged that add bogus initializations, which
> definitely prevent gcc from warning about a real uninitialized
> variable bug in that function again. I have done some of those
> patches myself in the past, but it always feels really wrong to
> do those.

I always reject any set variable to zero (or mark it uninitialised) just
because gcc warns patches precisely because they would hide future
errors; all the checkers we care about have a false positive matching
system now.  The thing this would cut down on is the number of newbie "I
compiled the kernel myself and this fixes the warning I found" type
patches, which I do see as a net benefit.

James

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

end of thread, other threads:[~2013-03-16  8:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-15 14:55 Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os Arnd Bergmann
2013-03-15 18:12 ` Russell King - ARM Linux
2013-03-15 19:43   ` Arnd Bergmann
2013-03-16  8:56     ` James Bottomley

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