Date: Wed, 9 Sep 2020 09:44:29 -0400 From: Mark Johnston <markj@freebsd.org> To: Andriy Gapon <avg@freebsd.org> Cc: Michal Meloun <meloun.michal@gmail.com>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: uninitialized variables [Was: svn commit: r365445 - head/sys/cam/mmc] Message-ID: <20200909134429.GA65588@raichu> In-Reply-To: <6b18b5ef-a743-3d5e-8dd2-24640614ec88@FreeBSD.org> References: <202009080546.0885kAgk006783@repo.freebsd.org> <34826ee7-12a9-d309-1fee-cd2e95744603@FreeBSD.org> <67be7fa5-30dd-b7ee-1076-9c29195d83d3@gmail.com> <20200908124848.GB66031@raichu> <6b18b5ef-a743-3d5e-8dd2-24640614ec88@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 09, 2020 at 08:49:01AM +0300, Andriy Gapon wrote:
> On 08/09/2020 15:48, Mark Johnston wrote:
> > I observed the same thing recently as well: the compiler catches
> > uninitialized variables only in simple cases. In my case, any uses of
> > goto within the function seemed to silence the warning, even if they
> > appeared after the uninitialized reference.
>
> I am running a kernel build now with this addition (for clang):
> CWARNEXTRA+= -Wconditional-uninitialized -Wno-error-conditional-uninitialized
>
> It produces a ton of warnings.
> Some of them are probably false positives, but some look quite reasonable.
It has a lot of trouble with code patterns of the form:
for (i = 0; i < 100; i++) {
val = foo();
}
if (val != 0) /* may be uninitialized!!1 */
bar();
or
if (foo == bar)
val = baz();
<some other stuff>
if (foo == bar && val == 3)
<some stuff>
The second example makes some sense to me since it's hard to prove that
foo == bar will not change between the first and second evaluations.
> E.g.:
> sys/cam/cam_periph.c:314:19: warning: variable 'p_drv' may be uninitialized when
> used here [-Wconditional-uninitialized]
> TAILQ_REMOVE(&(*p_drv)->units, periph, unit_links);
>
> Indeed, there is a conditional 'goto failure' before a first assignment to p_drv
> and the line is after the label. So, maybe the situation is impossible, but it
> is reasonable to warn about it.
>
> But the number of false positives (and "possible but impossible" situations) is
> too overwhelming.
Yeah. I looked at maybe 30 warnings (out of hundreds) this morning
and they were all false positives. KMSAN will provide a new tool for
finding such bugs, but they will only be detected at runtime.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200909134429.GA65588>