[PATCH] Division by zero

Anders Rönnholm Anders.Ronnholm at evidente.se
Fri Sep 5 05:24:06 PDT 2014


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

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

//Anders



More information about the cfe-commits mailing list