From 965a034afd314c5111672f210d58d4e4ea59ac7c Mon Sep 17 00:00:00 2001
From: Alexander Mai <amai@users.sf.net>
Date: Sat, 1 Aug 2015 18:42:17 +0200
Subject: [PATCH] Fix some more false positives on zerodiv: error should be
 issued if type of epxression is known to be integral

---
 lib/checkother.cpp | 45 ++++++++++++++++++++++++++++++++++++++++++---
 test/testother.cpp | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index b796f8ee9..51340ca74 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -32,6 +32,46 @@ namespace {
     CheckOther instance;
 }
 
+bool astIsIntegral(const Token *tok, bool unknown)
+{
+    // TODO: handle arrays
+    if (tok->isNumber())
+        return MathLib::isInt(tok->str());
+
+    if (tok->isName()) {
+        if (tok->variable())
+            return tok->variable()->isIntegralType();
+
+        return unknown;
+    }
+    if (tok->str() == "(") {
+        // cast
+        if (Token::Match(tok, "( const| float|double )"))
+            return false;
+
+        // Function call
+        if (tok->previous()->function()) {
+            if (Token::Match(tok->previous()->function()->retDef, "float|double"))
+                return false;
+            else if (Token::Match(tok->previous()->function()->retDef, "bool|char|short|int|long"))
+                return true;
+        }
+
+        if (tok->strAt(-1) == "sizeof")
+            return true;
+
+        return unknown;
+    }
+
+    if (tok->astOperand2() && (tok->str() == "." || tok->str() == "::"))
+        return astIsIntegral(tok->astOperand2(), unknown);
+
+    if (tok->astOperand1() && tok->str() != "?")
+        return astIsIntegral(tok->astOperand1(), unknown);
+
+    return unknown;
+}
+
 bool astIsFloat(const Token *tok, bool unknown)
 {
     // TODO: handle arrays
@@ -1799,7 +1839,7 @@ void CheckOther::checkZeroDivision()
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
         if (!Token::Match(tok, "[/%]") || !tok->astOperand1() || !tok->astOperand2())
             continue;
-        if (astIsFloat(tok,false))
+        if (!astIsIntegral(tok,false))
             continue;
         if (tok->astOperand1()->isNumber()) {
             if (MathLib::isFloat(tok->astOperand1()->str()))
@@ -1807,10 +1847,9 @@ void CheckOther::checkZeroDivision()
         } else if (tok->astOperand1()->isName()) {
             if (tok->astOperand1()->variable() && !tok->astOperand1()->variable()->isIntegralType())
                 continue;
-        } else {
+        } else if (!tok->astOperand1()->isArithmeticalOp()) {
             continue;
         }
-
         // Value flow..
         const ValueFlow::Value *value = tok->astOperand2()->getValue(0LL);
         if (!value)
diff --git a/test/testother.cpp b/test/testother.cpp
index 08f41cb09..ee97f1186 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -299,18 +299,33 @@ private:
               "{\n"
               "   long a = b / 0x0;\n"
               "}");
+        ASSERT_EQUALS("", errout.str());
+        check("void f(long b)\n"
+              "{\n"
+              "   long a = b / 0x0;\n"
+              "}");
         ASSERT_EQUALS("[test.cpp:3]: (error) Division by zero.\n", errout.str());
 
         check("void f()\n"
               "{\n"
               "   long a = b / 0L;\n"
               "}");
+        ASSERT_EQUALS("", errout.str());
+        check("void f(long b)\n"
+              "{\n"
+              "   long a = b / 0L;\n"
+              "}");
         ASSERT_EQUALS("[test.cpp:3]: (error) Division by zero.\n", errout.str());
 
         check("void f()\n"
               "{\n"
               "   long a = b / 0ul;\n"
               "}");
+        ASSERT_EQUALS("", errout.str());
+        check("void f(long b)\n"
+              "{\n"
+              "   long a = b / 0ul;\n"
+              "}");
         ASSERT_EQUALS("[test.cpp:3]: (error) Division by zero.\n", errout.str());
 
         // Don't warn about floating points (gcc doesn't warn either)
@@ -333,6 +348,11 @@ private:
               "{ { {\n"
               "   long a = b / 0;\n"
               "} } }");
+        ASSERT_EQUALS("", errout.str());
+        check("void f(long b)\n"
+              "{ { {\n"
+              "   long a = b / 0;\n"
+              "} } }");
         ASSERT_EQUALS("[test.cpp:3]: (error) Division by zero.\n", errout.str());
     }
 
@@ -341,14 +361,25 @@ private:
               "{ { {\n"
               "   int a = b % 0;\n"
               "} } }");
+        ASSERT_EQUALS("", errout.str());
+        check("void f(int b)\n"
+              "{ { {\n"
+              "   int a = b % 0;\n"
+              "} } }");
         ASSERT_EQUALS("[test.cpp:3]: (error) Division by zero.\n", errout.str());
     }
 
     void zeroDiv7() {
+        // unknown types for x and y --> do not warn
         check("void f() {\n"
               "  int a = x/2*3/0;\n"
               "  int b = y/2*3%0;\n"
               "}");
+        ASSERT_EQUALS("", errout.str());
+        check("void f(int x, int y) {\n"
+              "  int a = x/2*3/0;\n"
+              "  int b = y/2*3%0;\n"
+              "}");
         ASSERT_EQUALS("[test.cpp:2]: (error) Division by zero.\n"
                       "[test.cpp:3]: (error) Division by zero.\n", errout.str());
     }
@@ -482,12 +513,12 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
 
-        // ?:
+        // Unknown types for b and c --> do not warn
         check("int f(int d) {\n"
               "  int r = (a?b:c) / d;\n"
               "  if (d == 0) {}\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'd==0' is redundant or there is division by zero at line 2.\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
 
         check("int f(int a) {\n"
               "  int r = a ? 1 / a : 0;\n"