| Message ID | 201303151455.38917.arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@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@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. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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