[PATCH] Division by zero

[Jordan Rose]

Hi, Anders. I have some high-level concerns about this checker as it's structured now. First (and most mundanely), the checker is conflating values and locations. A particular symbol in the analyzer can never change, so there's no reason to ever have to remove something from the DivZeroMap because of invalidation.

int x = foo(); // The value stored in 'x' is some symbol '$foo'
int y = 5 / x; // use $foo
x = bar(); // The value stored in 'x' is now some symbol '$bar', but '$foo' hasn't changed.

On the flip side, I'm concerned that values never leave the map. You could clean up some of them with a checkDeadSymbols callback, but that's still a large amount of state for values that may have been checked quite a while ago. In particular, this will result in false positives if a value is used in one function and then checked in another.

(The opposite order hit us so much when dealing with C++ support that we coined a term for it: "inlined defensive checks". This occurs when one function accepts null pointers and so does a check, the caller knows the value is never null but doesn't actually assert it, and then the value is used. We ended up having to silence all bugs where the first null check came from an inlined function, which definitely through out some true positives with the false ones.)

What do you think?
Jordan

P.S. If your map entries always map to "true", you might as well use a set. ;-)


On Apr 1, 2014, at 7:45 , Anders Rönnholm <Anders.Ronnholm at evidente.se> wrote:

> Hi,
> 
> I have a new patch i'd like to get reviewed for a different kind of division by zero than the already existing one. 
> 
> This patch check for division by variable that is later compared against 0. Either the comparison is useless or there is division by zero.
> 
> It catches scenarios like these:
> 
> void f(int y) {
> x = 100 / y;
> if (y == 0) {}
> }
> 
> //Anders<divzero2.diff>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

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