Fixed #1751 (Undefined Behavior: Signed integer overflow)

lib/checkother.cpp

@@ -59,6 +59,69 @@ bool astIsFloat(const Token *tok, bool unknown)
     return unknown;
 }
 
+static bool astGetSizeSign(const Settings *settings, const Token *tok, int *size, char *sign)
+{
+    if (!tok)
+        return false;
+    if (tok->isArithmeticalOp()) {
+        if (!astGetSizeSign(settings, tok->astOperand1(), size, sign))
+            return false;
+        return !tok->astOperand2() || astGetSizeSign(settings, tok->astOperand2(), size, sign);
+    }
+    if (tok->isNumber() && MathLib::isInt(tok->str())) {
+        if (tok->str().find("L") != std::string::npos)
+            return false;
+        MathLib::bigint value = MathLib::toLongNumber(tok->str());
+        int sz;
+        if (value >= -(1<<7) && value <= (1<<7)-1)
+            sz = 8;
+        else if (value >= -(1<<15) && value <= (1<<15)-1)
+            sz = 16;
+        else if (value >= -(1LL<<31) && value <= (1LL<<31)-1)
+            sz = 32;
+        else
+            return false;
+        if (sz < 8 * settings->sizeof_int)
+            sz = 8 * settings->sizeof_int;
+        if (*size < sz)
+            *size = sz;
+        if (tok->str().find('U') != std::string::npos)
+            *sign = 'u';
+        if (*sign != 'u')
+            *sign = 's';
+        return true;
+    }
+    if (tok->isName()) {
+        const Variable *var = tok->variable();
+        if (!var)
+            return false;
+        int sz = 0;
+        for (const Token *type = var->typeStartToken(); type; type = type->next()) {
+            if (type->str() == "*")
+                return false;  // <- FIXME: handle pointers
+            if (Token::Match(type, "char|short|int")) {
+                sz = 8 * settings->sizeof_int;
+                if (type->isUnsigned())
+                    *sign = 'u';
+                else if (*sign != 'u')
+                    *sign = 's';
+            } else if (Token::Match(type, "float|double|long")) {
+                return false;
+            } else {
+                // TODO: try to lookup type info in library
+            }
+            if (type == var->typeEndToken())
+                break;
+        }
+        if (sz == 0)
+            return false;
+        if (*size < sz)
+            *size = sz;
+        return true;
+    }
+    return false;
+}
+
 static bool isConstExpression(const Token *tok, const std::set<std::string> &constFunctions)
 {
     if (!tok)
@@ -2726,6 +2789,59 @@ void CheckOther::tooBigBitwiseShiftError(const Token *tok, int lhsbits, const Va
     reportError(callstack, rhsbits.condition ? Severity::warning : Severity::error, "shiftTooManyBits", errmsg.str(), rhsbits.inconclusive);
 }
 
+//---------------------------------------------------------------------------
+// Checking for integer overflow
+//---------------------------------------------------------------------------
+
+void CheckOther::checkIntegerOverflow()
+{
+    // unknown sizeof(int) => can't run this checker
+    if (_settings->platformType == Settings::Unspecified)
+        return;
+
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * scope = symbolDatabase->functionScopes[i];
+        for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
+            if (!tok->isArithmeticalOp())
+                continue;
+
+            // get size and sign of result..
+            int  size = 0;
+            char sign = 0;
+            if (!astGetSizeSign(_settings, tok, &size, &sign))
+                continue;
+            if (sign != 's')  // only signed integer overflow is UB
+                continue;
+
+            // max int value according to platform settings.
+            const MathLib::bigint maxint = (1LL << 8 * (_settings->sizeof_int - 1)) - 1;
+
+            // is there a overflow result value
+            const ValueFlow::Value *value = tok->getValueGE(maxint + 1, _settings);
+            if (!value)
+                value = tok->getValueLE(-maxint - 2, _settings);
+            if (value)
+                integerOverflowError(tok, *value);
+        }
+    }
+}
+
+void CheckOther::integerOverflowError(const Token *tok, const ValueFlow::Value &value)
+{
+    const std::string expr(tok ? tok->expressionString() : "");
+    const std::string cond(value.condition ?
+                           ". See condition at line " + MathLib::toString(value.condition->linenr()) + "." :
+                           "");
+
+    reportError(tok,
+                value.condition ? Severity::warning : Severity::error,
+                "integerOverflow",
+                "Signed integer overflow for expression '"+expr+"'"+cond,
+                value.inconclusive);
+}
+
 //---------------------------------------------------------------------------
 // Check for incompletely filled buffers.
 //---------------------------------------------------------------------------

lib/checkother.h

@@ -103,6 +103,7 @@ public:
         checkOther.checkRedundantCopy();
         checkOther.checkNegativeBitwiseShift();
         checkOther.checkTooBigBitwiseShift();
+        checkOther.checkIntegerOverflow();
         checkOther.checkSuspiciousEqualityComparison();
         checkOther.checkComparisonFunctionIsAlwaysTrueOrFalse();
     }
@@ -222,6 +223,9 @@ public:
     /** @brief %Check for bitwise shift with too big right operand */
     void checkTooBigBitwiseShift();
 
+    /** @brief %Check for integer overflow */
+    void checkIntegerOverflow();
+
     /** @brief %Check for buffers that are filled incompletely with memset and similar functions */
     void checkIncompleteArrayFill();
 
@@ -291,6 +295,7 @@ private:
     void doubleCloseDirError(const Token *tok, const std::string &varname);
     void negativeBitwiseShiftError(const Token *tok);
     void tooBigBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits);
+    void integerOverflowError(const Token *tok, const ValueFlow::Value &value);
     void redundantCopyError(const Token *tok, const std::string &varname);
     void incompleteArrayFillError(const Token* tok, const std::string& buffer, const std::string& function, bool boolean);
     void varFuncNullUBError(const Token *tok);
@@ -310,6 +315,7 @@ private:
         c.invalidPointerCastError(0, "float", "double", false);
         c.negativeBitwiseShiftError(0);
         c.tooBigBitwiseShiftError(0, 32, ValueFlow::Value(64));
+        c.integerOverflowError(0, ValueFlow::Value(1LL<<32));
         c.checkPipeParameterSizeError(0, "varname", "dimension");
 
         //performance

test/testother.cpp

@@ -150,6 +150,8 @@ private:
         TEST_CASE(checkNegativeShift);
         TEST_CASE(checkTooBigShift);
 
+        TEST_CASE(checkIntegerOverflow);
+
         TEST_CASE(incompleteArrayFill);
 
         TEST_CASE(redundantVarAssignment);
@@ -5400,6 +5402,29 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void checkIntegerOverflow() {
+        Settings settings;
+        settings.platform(Settings::Unix32);
+
+        check("int foo(int x) {\n"
+              "   if (x==123456) {}\n"
+              "   return x * x;\n"
+              "}","test.cpp",false,false,false,true,&settings);
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Signed integer overflow for expression 'x*x'. See condition at line 2.\n", errout.str());
+
+        check("int foo(int x) {\n"
+              "   if (x==123456) {}\n"
+              "   return -123456 * x;\n"
+              "}","test.cpp",false,false,false,true,&settings);
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Signed integer overflow for expression '-123456*x'. See condition at line 2.\n", errout.str());
+
+        check("int foo(int x) {\n"
+              "   if (x==123456) {}\n"
+              "   return 123456U * x;\n"
+              "}","test.cpp",false,false,false,true,&settings);
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void incompleteArrayFill() {
         check("void f() {\n"
               "    int a[5];\n"