[cfe-dev] [cfe-commits] False positive for -Wunreachable-code

David Blaikie dblaikie at gmail.com
Wed Oct 31 10:11:20 PDT 2012


On Wed, Oct 31, 2012 at 12:22 AM, Abramo Bagnara
<abramo.bagnara at bugseng.com> wrote:
> Il 30/10/2012 19:19, David Blaikie ha scritto:
>>>>
>>>> Your definition of "false positive" differs from mine/ours. Hopefully
>>>> my description above helps describe the motivation here.
>>>
>>> Yes, my definition of false positive is different, see:
>>>
>>> http://en.wikipedia.org/wiki/Type_I_and_type_II_errors#False_positive_error
>>>
>>> "is the default unreachable"? ("is there a wolf near the herd?")
>>>
>>> If the message says "warning: will never be executed
>>> [-Wunreachable-code]" ("Wolf, wolf!") then we have a false positive.
>>>
>>> The idea that a warning is a false positive if it has not found a bug in
>>> the code is rather weird... do you know any warning that is able to
>>> always find a bug without knowing the programmer intentions?
>>
>> Generally, no. They make local assumptions so to make a best effort at
>> finding bugs. Part of that is also to avoid finding non-bugs because
>> doing so creates a burden on developers that may eventually eclipse
>> the benefit they gain from the warning.
>>
>> We're not trying to write warnings to teach people about the semantics
>> of their code - they can read books about that. They're meant to be
>> actionable for a good reason, not just to indicate that the developer
>> read & understood the diagnostic message & then went on their merry
>> way.
>>
>>> That apart, of course if we want a compilation switch that deviates from
>>> the standard and says that an enum typed expression cannot have any
>>> value different from enum constants specified we can do that, but really
>>> we want that?
>>
>> Not really, no - the compiler implements the standard. For the purpose
>> of diagnostics we might be interested in adding an attribute to enum
>> types or variables that could indicate whether that variable or
>> variables of that type are intended to contain an/all values in the
>> representable range (to cause us to do things like diagnose the code
>> after the switch or in the default of a covered switch as reachable,
>> etc).
>
> In the long term this seems indeed the right way to handle that, but I
> believe that more useful (and in our work of software verifier tools
> writer indeed mandatory)

Sorry, I don't quite understand this point. If you're implementing a
verifier on top of Clang then it'll need cross-TU analysis anyway &
shouldn't need to make these assumptions (ideally it could prove that
the enum use is safe/never leaks the possibility of lying outside the
enumerable values but inside the representable range). This might call
for some API tweaks to the CFGBuilder so that it can take information
your cross-TU analysis has proven about certain enums, but I don't see
how a Clang command line option would relate to development of
software verifier tools. Could you explain in more detail?

For the Clang Static Analyzer (which currently doesn't do cross-TU
analysis as far as I know) & the compiler itself, these seem like
reasonable best-effort heuristics given partial information.


> is to add a global option (disabled by default)
> to avoid statistical assumption about programmer intention.
>
> (i.e. the option says (among other things): "as the standard does not
> limit enum typed values to enum constants listed in enum declaration,
> then this assumption, although if true for 90% of cases, is not taken
> for granted)
>
> This would not harm in any way the use of clang warnings as a way to
> identify locations that are bug with a probability > 90%, while in the
> same time makes clang a valid tool to find code weakness according to
> language standard semantics and to suggest proper defensive programming
> technique whenever they are needed.
>
> Is this a viable path to get the best of the two worlds?
>
> --
> Abramo Bagnara
>
> BUGSENG srl - http://bugseng.com
> mailto:abramo.bagnara at bugseng.com



More information about the cfe-dev mailing list