On 09/09/2020 16:44, Mark Johnston wrote:
> 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.

I also noted the first pattern as the most common source of false positives.
So, it seems that we cannot have what we want.
Without -Wconditional-uninitialized clang is too conservative, with the option
it's too "loose".

I seem to recall that compilers used to be better than that.
But maybe it's just false memories ("there used to be more snow in the winter",
etc).

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


-- 
Andriy Gapon
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to