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

[Abramo Bagnara]

Il 31/10/2012 18:11, David Blaikie ha scritto:
> 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?

Of course we don't need a command line option, but an option settable
from clang library (e.g. in LangOptions).

Once this has been done it become natural and savy to interface it to a
clang command line options (although we are interested in that only as
clang compiler users and not for our tools)

About cross-TU analysis, it is not strictly needed for the unreachable
code check. Of course it increases precision, but is not mandatory.

The point is that when we rely on info obtained from clang we need to be
sure they does not contains false negative or positives due to
statistical assumptions or arbitrary heuristics (as in the case of a
perfectly reachable default presented as never executed shown in the
mail that has originated this thread).

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

-- 
Abramo Bagnara

BUGSENG srl - http://bugseng.com
mailto:abramo.bagnara at bugseng.com