From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
linux-kbuild@vger.kernel.org, Michal Marek <mmarek@suse.cz>,
linux-arch@vger.kernel.org, inux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os
Date: Sat, 16 Mar 2013 08:56:14 +0000 [thread overview]
Message-ID: <1363424174.2459.15.camel@dabdike> (raw)
In-Reply-To: <201303151943.45980.arnd@arndb.de>
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
WARNING: multiple messages have this Message-ID (diff)
From: James.Bottomley@HansenPartnership.com (James Bottomley)
To: linux-arm-kernel@lists.infradead.org
Subject: Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os
Date: Sat, 16 Mar 2013 08:56:14 +0000 [thread overview]
Message-ID: <1363424174.2459.15.camel@dabdike> (raw)
In-Reply-To: <201303151943.45980.arnd@arndb.de>
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
next prev parent reply other threads:[~2013-03-16 8:56 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-15 14:55 Fwd: [PATCH] Turn off -Wmaybe-uninitialized when building with -Os Arnd Bergmann
2013-03-15 14:55 ` Arnd Bergmann
2013-03-15 14:55 ` Arnd Bergmann
2013-03-15 18:12 ` Russell King - ARM Linux
2013-03-15 18:12 ` Russell King - ARM Linux
2013-03-15 19:43 ` Arnd Bergmann
2013-03-15 19:43 ` Arnd Bergmann
2013-03-16 8:56 ` James Bottomley [this message]
2013-03-16 8:56 ` James Bottomley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1363424174.2459.15.camel@dabdike \
--to=james.bottomley@hansenpartnership.com \
--cc=arnd@arndb.de \
--cc=inux-kernel@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=mmarek@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.