[PATCH] Division by zero

Anders Rönnholm Anders.Ronnholm at evidente.se
Thu Jun 5 05:16:55 PDT 2014


Hi Jordan,

New patch and some comments.

+inline void inline_func3(int x) {
+  var = 77 / x;
+}
+void ok_inline(int x) {
+  var = 77 / x;
+  inline_func3(x);
+  if (x==0){}
+}

This is an example where we should still be warning. Why aren't we?

It doesn't warn here because blockid is in the value and not the key as it should. I have made a set of symbol-block-frame as you suggested and that fixes this problem.


+// RUN: %clang_cc1 -analyze -analyzer-checker=core.DivideZero2 -analyzer-output=text -verify %s

We should never be running path-sensitive checks without the rest of the core checkers.

There is a small problem here. It looks like the DivideZero checker makes the assumtion that because it can't see that a variable is zero then it's not zero so all variables in our tests have the range reg_$0<x> : { [-2147483648, -1], [1, 2147483647] }. I have removed the range check in my checker to make it work otherwise i would have to change the DivideZero checker.


+  const ZeroState *ZS = C.getState()->get<DivZeroMap>(SR);
+  return (ZS && *ZS == ZeroState(C.getBlockID(), C.getStackFrame()));

...now I'm wondering about the wisdom of using a map for this. What if the same symbol is constrained in two stack frames? Would it make more sense to keep around a set of symbol-block-frame triples, rather than a map from symbols to block/frame pairs?

Or can this not happen because a value can only be used once along a certain path before it is constrained to be non-zero?

Changed to a set of symbol-block-frame triples to make the above example work.


+def DivZeroChecker2 : Checker<"DivideZero2">,
+  HelpText<"Check for division by variable that is later compared against 0. Either the comparison is useless or there is division by zero.">,
+  DescFile<"DivZeroChecker2.cpp">;
+

I'm not sure this is really a core checker, since it's safe to run the analyzer without it. But we don't have a category for "useful C checks that should be on by default" yet. Anton was looking at reorganizing the categories, but the problem is that everything outside of "alpha" is something people expect to be able to rely on when using scan-build.

Any ideas for this particular checker, or for the general issue?

I ok with having it as an alpha, or something else. I just put it in core since the other DivideZero checker is there.

//Anders
-------------- next part --------------
A non-text attachment was scrubbed...
Name: divzero2.diff
Type: text/x-patch
Size: 17925 bytes
Desc: divzero2.diff
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140605/c2176daa/attachment.bin>


More information about the cfe-commits mailing list