[PATCH] Division by zero

[Anna Zaks]

> On Sep 5, 2014, at 5:24 AM, Anders Rönnholm <Anders.Ronnholm at evidente.se> wrote:
> 
> Hi,
> 
> I feel that to change this checker and the null dereference check would take a large amount of time compared to what is gained, time which could be used more efficiently on other checkers.
> The null dereference check is already completed as path sensitive and works well.

We are talking about converting only the "check after division/dereference" (not regular div by zero or dereference checks) because these checks require all paths reasoning (See the "[cfe-dev] [RFC] Creating base class for 'Test after X' checkers" thread). The main win is speed (flow sensitive analyzes are algorithmically much simpler than the path sensitive ones), which also opens a possibility of converting this into a compiler warning.

I agree that it would not be a very easy task, but this is the right way to approach the problem.

> I don't know when we can deliver this as CFG-based (definitely not this year),  wouldn't it be better to have it like this now?
> 
> For a possible remake of this checker to a CFG-based one in the future, we would need more help from you on how to make them CFG-based. What parts of LiveVariables and DeadStoresChecker are related to our check? I guess just parts of the LiveVariables framework is needed.

DeadStoresChecker is an example of other flow sensitive checks. You would need to create a similar one for div by zero / dereference. 

> 
>> So, Anna brought up that the check as implemented is very nearly path-independent, i.e. it only depends on flow-sensitive properties of the CFG. The path-sensitivity is buying us very little; it catches this case:
>> 
>> int y = x;
>> int div = z / y;
>> if (x) { ...}
>> 
>> But also warns here, which doesn't necessarily make sense:
>> 
>> int foo(int x, int y, int z) {
>>       int div = z / y;
>>       if (x) return div;
>>       return 0;
>> }
>> 
>> foo(a, a, b); // only coincidentally the same symbol
>> 
>> What would you think about turning this (and/or the null dereference check) into a CFG-based check instead? We lose the first example (and cases where inlining would help), but fix the second, and very possibly speed up analysis. CFG analysis is also more capable of proving that something happens on all paths rather than just some, >since that's just propagating information along the graph.
> 
> I agree, this can in theory be a false positive but we believe it is an unlikely one. Other existing checkers in the Clang static analyser have the same problem. I don't have any proposal, but maybe a generic solution for such FP would be good. In the long run I think it would be nice to have full flow analysis in this checker so we can find deeper bugs that are not limited to a single basic block.
> 

Again, the main issue is the algorithmic performance win, not false positives.

> //Anders

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140905/2254aa55/attachment.html>