Fixed #6158 (New check: dangerous sign conversion)

Makefile

@@ -137,6 +137,7 @@ LIBOBJ =      $(SRCDIR)/check.o \
               $(SRCDIR)/checksizeof.o \
               $(SRCDIR)/checkstl.o \
               $(SRCDIR)/checkstring.o \
+              $(SRCDIR)/checktype.o \
               $(SRCDIR)/checkuninitvar.o \
               $(SRCDIR)/checkunusedfunctions.o \
               $(SRCDIR)/checkunusedvar.o \
@@ -211,6 +212,7 @@ TESTOBJ =     test/options.o \
               test/testtimer.o \
               test/testtoken.o \
               test/testtokenize.o \
+              test/testtype.o \
               test/testuninitvar.o \
               test/testunusedfunctions.o \
               test/testunusedprivfunc.o \
@@ -340,6 +342,9 @@ $(SRCDIR)/checkstl.o: lib/checkstl.cpp lib/cxx11emu.h lib/checkstl.h lib/config.
 $(SRCDIR)/checkstring.o: lib/checkstring.cpp lib/cxx11emu.h lib/checkstring.h lib/config.h lib/check.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/path.h lib/standards.h lib/timer.h lib/symboldatabase.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o $(SRCDIR)/checkstring.o $(SRCDIR)/checkstring.cpp
 
+$(SRCDIR)/checktype.o: lib/checktype.cpp lib/cxx11emu.h lib/checktype.h lib/config.h lib/check.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/path.h lib/standards.h lib/timer.h lib/symboldatabase.h
+	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o $(SRCDIR)/checktype.o $(SRCDIR)/checktype.cpp
+
 $(SRCDIR)/checkuninitvar.o: lib/checkuninitvar.cpp lib/cxx11emu.h lib/checkuninitvar.h lib/config.h lib/check.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/path.h lib/standards.h lib/timer.h lib/executionpath.h lib/checknullpointer.h lib/symboldatabase.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o $(SRCDIR)/checkuninitvar.o $(SRCDIR)/checkuninitvar.cpp
 
@@ -556,6 +561,9 @@ test/testtoken.o: test/testtoken.cpp lib/cxx11emu.h test/testsuite.h lib/errorlo
 test/testtokenize.o: test/testtokenize.cpp lib/cxx11emu.h test/testsuite.h lib/errorlogger.h lib/config.h lib/suppressions.h test/redirect.h lib/library.h lib/path.h lib/mathlib.h lib/token.h lib/valueflow.h lib/tokenize.h lib/tokenlist.h lib/settings.h lib/standards.h lib/timer.h lib/preprocessor.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testtokenize.o test/testtokenize.cpp
 
+test/testtype.o: test/testtype.cpp lib/cxx11emu.h lib/preprocessor.h lib/config.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/symboldatabase.h lib/token.h lib/valueflow.h lib/mathlib.h lib/checkother.h lib/check.h lib/settings.h lib/library.h lib/path.h lib/standards.h lib/timer.h test/testsuite.h test/redirect.h test/testutils.h
+	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testtype.o test/testtype.cpp
+
 test/testuninitvar.o: test/testuninitvar.cpp lib/cxx11emu.h lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checkuninitvar.h lib/check.h lib/token.h lib/valueflow.h lib/mathlib.h lib/settings.h lib/library.h lib/path.h lib/standards.h lib/timer.h test/testsuite.h test/redirect.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testuninitvar.o test/testuninitvar.cpp
 

lib/checkother.cpp

@@ -59,69 +59,6 @@ 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,124 +2663,6 @@ void CheckOther::negativeBitwiseShiftError(const Token *tok)
     reportError(tok, Severity::error, "shiftNegative", "Shifting by a negative value is undefined behaviour");
 }
 
-//---------------------------------------------------------------------------
-// Checking for shift by too many bits
-//---------------------------------------------------------------------------
-
-void CheckOther::checkTooBigBitwiseShift()
-{
-    // 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->str() != "<<" && tok->str() != ">>")
-                continue;
-
-            if (!tok->astOperand1() || !tok->astOperand2())
-                continue;
-
-            // get number of bits of lhs
-            const Variable *var = tok->astOperand1()->variable();
-            if (!var)
-                continue;
-            int lhsbits = 0;
-            for (const Token *type = var->typeStartToken(); type; type = type->next()) {
-                if (Token::Match(type,"char|short|int") && !type->isLong()) {
-                    lhsbits = _settings->sizeof_int * 8;
-                    break;
-                }
-                if (type == var->typeEndToken())
-                    break;
-            }
-            if (lhsbits == 0)
-                continue;
-
-            // Get biggest rhs value. preferably a value which doesn't have 'condition'.
-            const ValueFlow::Value *value = tok->astOperand2()->getValueGE(lhsbits, _settings);
-            if (!value)
-                continue;
-            if (value->condition && !_settings->isEnabled("warning"))
-                continue;
-            if (value->inconclusive && !_settings->inconclusive)
-                continue;
-            tooBigBitwiseShiftError(tok, lhsbits, *value);
-        }
-    }
-}
-
-void CheckOther::tooBigBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits)
-{
-    std::list<const Token*> callstack;
-    callstack.push_back(tok);
-    if (rhsbits.condition)
-        callstack.push_back(rhsbits.condition);
-    std::ostringstream errmsg;
-    errmsg << "Shifting " << lhsbits << "-bit value by " << rhsbits.intvalue << " bits is undefined behaviour";
-    if (rhsbits.condition)
-        errmsg << ". See condition at line " << rhsbits.condition->linenr() << ".";
-    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;
-
-    // max int value according to platform settings.
-    const MathLib::bigint maxint = (1LL << (8 * _settings->sizeof_int - 1)) - 1;
-
-    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;
-
-            // 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)
-                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;
-
-            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

@@ -102,8 +102,6 @@ public:
         checkOther.checkDoubleFree();
         checkOther.checkRedundantCopy();
         checkOther.checkNegativeBitwiseShift();
-        checkOther.checkTooBigBitwiseShift();
-        checkOther.checkIntegerOverflow();
         checkOther.checkSuspiciousEqualityComparison();
         checkOther.checkComparisonFunctionIsAlwaysTrueOrFalse();
     }
@@ -220,12 +218,6 @@ public:
     /** @brief %Check for bitwise shift with negative right operand */
     void checkNegativeBitwiseShift();
 
-    /** @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();
 
@@ -294,8 +286,6 @@ private:
     void SuspiciousSemicolonError(const Token *tok);
     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);
@@ -314,8 +304,6 @@ private:
         c.doubleFreeError(0, "varname");
         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
@@ -378,7 +366,6 @@ private:
                "* provide wrong dimensioned array to pipe() system command (--std=posix)\n"
                "* cast the return values of getc(),fgetc() and getchar() to character and compare it to EOF\n"
                "* invalid input values for functions\n"
-               "* bitwise shift by too many bits\n"
 
                // warning
                "* either division by zero or useless condition\n"

lib/checktype.cpp

@@ -0,0 +1,280 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+//---------------------------------------------------------------------------
+#include "checktype.h"
+#include "mathlib.h"
+#include "symboldatabase.h"
+
+#include <stack>
+//---------------------------------------------------------------------------
+
+// Register this check class (by creating a static instance of it)
+namespace {
+    CheckType instance;
+}
+
+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;
+}
+
+//---------------------------------------------------------------------------
+// Checking for shift by too many bits
+//---------------------------------------------------------------------------
+
+void CheckType::checkTooBigBitwiseShift()
+{
+    // 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->str() != "<<" && tok->str() != ">>")
+                continue;
+
+            if (!tok->astOperand1() || !tok->astOperand2())
+                continue;
+
+            // get number of bits of lhs
+            const Variable *var = tok->astOperand1()->variable();
+            if (!var)
+                continue;
+            int lhsbits = 0;
+            for (const Token *type = var->typeStartToken(); type; type = type->next()) {
+                if (Token::Match(type,"char|short|int") && !type->isLong()) {
+                    lhsbits = _settings->sizeof_int * 8;
+                    break;
+                }
+                if (type == var->typeEndToken())
+                    break;
+            }
+            if (lhsbits == 0)
+                continue;
+
+            // Get biggest rhs value. preferably a value which doesn't have 'condition'.
+            const ValueFlow::Value *value = tok->astOperand2()->getValueGE(lhsbits, _settings);
+            if (!value)
+                continue;
+            if (value->condition && !_settings->isEnabled("warning"))
+                continue;
+            if (value->inconclusive && !_settings->inconclusive)
+                continue;
+            tooBigBitwiseShiftError(tok, lhsbits, *value);
+        }
+    }
+}
+
+void CheckType::tooBigBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits)
+{
+    std::list<const Token*> callstack;
+    callstack.push_back(tok);
+    if (rhsbits.condition)
+        callstack.push_back(rhsbits.condition);
+    std::ostringstream errmsg;
+    errmsg << "Shifting " << lhsbits << "-bit value by " << rhsbits.intvalue << " bits is undefined behaviour";
+    if (rhsbits.condition)
+        errmsg << ". See condition at line " << rhsbits.condition->linenr() << ".";
+    reportError(callstack, rhsbits.condition ? Severity::warning : Severity::error, "shiftTooManyBits", errmsg.str(), rhsbits.inconclusive);
+}
+
+//---------------------------------------------------------------------------
+// Checking for integer overflow
+//---------------------------------------------------------------------------
+
+void CheckType::checkIntegerOverflow()
+{
+    // unknown sizeof(int) => can't run this checker
+    if (_settings->platformType == Settings::Unspecified)
+        return;
+
+    // max int value according to platform settings.
+    const MathLib::bigint maxint = (1LL << (8 * _settings->sizeof_int - 1)) - 1;
+
+    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;
+
+            // 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)
+                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;
+
+            integerOverflowError(tok, *value);
+        }
+    }
+}
+
+void CheckType::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);
+}
+
+//---------------------------------------------------------------------------
+// Checking for sign conversion when operand can be negative
+//---------------------------------------------------------------------------
+
+void CheckType::checkSignConversion()
+{
+    if (!_settings->isEnabled("warning"))
+        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;
+
+            int size = 0;
+            char sign = 0;
+            if (!astGetSizeSign(_settings, tok, &size, &sign))
+                continue;
+            
+            if (sign != 'u')
+                continue;
+
+            // Check if there are signed operands that can be negative..
+            std::stack<const Token *> tokens;
+            tokens.push(tok);
+            while (!tokens.empty()) {
+                const Token *tok1 = tokens.top();
+                tokens.pop();
+                if (!tok1)
+                    continue;
+                if (tok1->str() == "(")
+                    continue; // Todo: properly handle casts, function calls, etc
+                const Variable *var = tok1->variable();
+                if (var && tok1->getValueLE(-1,_settings)) {
+                    bool signedvar = false;
+                    for (const Token *type = var->typeStartToken();;type = type->next()) {
+                        if (type->isSigned()) {
+                            signedvar = true;
+                            break;
+                        }
+                        if (type == var->typeEndToken())
+                            break;
+                    }
+                    if (signedvar) {
+                        signConversionError(tok1);
+                        break;
+                    }
+                }
+                tokens.push(tok1->astOperand1());
+                tokens.push(tok1->astOperand2());
+            }
+        }
+    }
+}
+
+void CheckType::signConversionError(const Token *tok)
+{
+    const std::string varname(tok ? tok->str() : "var");
+    
+    reportError(tok,
+                Severity::warning,
+                "signConversion",
+                "Suspicious code: sign conversion of " + varname + " in calculation, even though " + varname + " can have a negative value");
+}

lib/checktype.h

@@ -0,0 +1,98 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+//---------------------------------------------------------------------------
+#ifndef checktypeH
+#define checktypeH
+//---------------------------------------------------------------------------
+
+#include "config.h"
+#include "check.h"
+
+/// @addtogroup Checks
+/// @{
+
+
+/** @brief Various small checks */
+
+class CPPCHECKLIB CheckType : public Check {
+public:
+    /** @brief This constructor is used when registering the CheckClass */
+    CheckType() : Check(myName()) {
+    }
+
+    /** @brief This constructor is used when running checks. */
+    CheckType(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
+        : Check(myName(), tokenizer, settings, errorLogger) {
+    }
+
+    /** @brief Run checks against the normal token list */
+    void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
+        (void)tokenizer;
+        (void)settings;
+        (void)errorLogger;
+    }
+
+    /** @brief Run checks against the simplified token list */
+    void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
+        CheckType checkType(tokenizer, settings, errorLogger);
+        checkType.checkTooBigBitwiseShift();
+        checkType.checkIntegerOverflow();
+        checkType.checkSignConversion();
+    }
+
+    /** @brief %Check for bitwise shift with too big right operand */
+    void checkTooBigBitwiseShift();
+
+    /** @brief %Check for integer overflow */
+    void checkIntegerOverflow();
+    
+    /** @brief %Check for dangerous sign conversion */
+    void checkSignConversion();
+
+private:
+    bool isUnsigned(const Variable *var) const;
+    static bool isSigned(const Variable *var);
+
+    // Error messages..
+    void tooBigBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits);
+    void integerOverflowError(const Token *tok, const ValueFlow::Value &value);
+    void signConversionError(const Token *tok);
+
+    void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
+        CheckType c(0, settings, errorLogger);
+        c.tooBigBitwiseShiftError(0, 32, ValueFlow::Value(64));
+        c.integerOverflowError(0, ValueFlow::Value(1LL<<32));
+        c.signConversionError(0);
+    }
+
+    static std::string myName() {
+        return "Type";
+    }
+
+    std::string classInfo() const {
+        return "Type checks\n"
+               "* bitwise shift by too many bits (only enabled when --platform is used)\n"
+               "* signed integer overflow (only enabled when --platform is used)\n"
+               "* dangerous sign conversion, when signed value can be negative\n";
+    }
+};
+/// @}
+//---------------------------------------------------------------------------
+#endif // checktypeH

lib/lib.pri

@@ -28,6 +28,7 @@ HEADERS += $${BASEPATH}check.h \
            $${BASEPATH}checksizeof.h \
            $${BASEPATH}checkstl.h \
            $${BASEPATH}checkstring.h \
+           $${BASEPATH}checktype.h \
            $${BASEPATH}checkuninitvar.h \
            $${BASEPATH}checkunusedfunctions.h \
            $${BASEPATH}checkunusedvar.h \
@@ -72,6 +73,7 @@ SOURCES += $${BASEPATH}check.cpp \
            $${BASEPATH}checksizeof.cpp \
            $${BASEPATH}checkstl.cpp \
            $${BASEPATH}checkstring.cpp \
+           $${BASEPATH}checktype.cpp \
            $${BASEPATH}checkuninitvar.cpp \
            $${BASEPATH}checkunusedfunctions.cpp \
            $${BASEPATH}checkunusedvar.cpp \

test/testfiles.pri

@@ -48,6 +48,7 @@ SOURCES += $${BASEPATH}/test64bit.cpp \
            $${BASEPATH}/testtimer.cpp \
            $${BASEPATH}/testtoken.cpp \
            $${BASEPATH}/testtokenize.cpp \
+           $${BASEPATH}/testtype.cpp \
            $${BASEPATH}/testuninitvar.cpp \
            $${BASEPATH}/testunusedfunctions.cpp \
            $${BASEPATH}/testunusedprivfunc.cpp \

test/testother.cpp

@@ -149,9 +149,6 @@ private:
         TEST_CASE(checkRedundantCopy);
 
         TEST_CASE(checkNegativeShift);
-        TEST_CASE(checkTooBigShift);
-
-        TEST_CASE(checkIntegerOverflow);
 
         TEST_CASE(incompleteArrayFill);
 
@@ -5398,44 +5395,6 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
-    void checkTooBigShift() {
-        Settings settings;
-        settings.platform(Settings::Unix32);
-
-        check("int foo(int x) {\n"
-              "   return x << 32;\n"
-              "}","test.cpp",false,false,false,true,&settings);
-        ASSERT_EQUALS("[test.cpp:2]: (error) Shifting 32-bit value by 32 bits is undefined behaviour\n", errout.str());
-
-        check("int foo(int x) {\n"
-              "   return x << 2;\n"
-              "}","test.cpp",false,false,false,true,&settings);
-        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"

test/testtype.cpp

@@ -0,0 +1,117 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "preprocessor.h"
+#include "tokenize.h"
+#include "symboldatabase.h"
+#include "checktype.h"
+#include "testsuite.h"
+#include "testutils.h"
+#include <sstream>
+#include <tinyxml2.h>
+
+extern std::ostringstream errout;
+
+class TestType : public TestFixture {
+public:
+    TestType() : TestFixture("TestType") {
+    }
+
+private:
+
+
+    void run() {
+        TEST_CASE(checkTooBigShift);
+        TEST_CASE(checkIntegerOverflow);
+        TEST_CASE(signConversion);
+    }
+
+    void check(const char code[], Settings* settings = 0) {
+        // Clear the error buffer..
+        errout.str("");
+
+        if (!settings) {
+            static Settings _settings;
+            settings = &_settings;
+        }
+        settings->addEnabled("warning");
+
+        // Tokenize..
+        Tokenizer tokenizer(settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        // Check..
+        CheckType checkType(&tokenizer, settings, this);
+        checkType.runChecks(&tokenizer, settings, this);
+        const std::string str1(tokenizer.tokens()->stringifyList(0,true));
+        tokenizer.simplifyTokenList2();
+        const std::string str2(tokenizer.tokens()->stringifyList(0,true));
+        if (str1 != str2)
+            warn(("Unsimplified code in test case\nstr1="+str1+"\nstr2="+str2).c_str());
+        checkType.runSimplifiedChecks(&tokenizer, settings, this);
+    }
+
+    void checkTooBigShift() {
+        Settings settings;
+        settings.platform(Settings::Unix32);
+
+        check("int foo(int x) {\n"
+              "   return x << 32;\n"
+              "}",&settings);
+        ASSERT_EQUALS("[test.cpp:2]: (error) Shifting 32-bit value by 32 bits is undefined behaviour\n", errout.str());
+
+        check("int foo(int x) {\n"
+              "   return x << 2;\n"
+              "}",&settings);
+        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"
+              "}",&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"
+              "}",&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"
+              "}",&settings);
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void signConversion() {
+        check("unsigned int f1(signed int x, unsigned int y) {"
+              "  return x * y;\n"
+              "}\n"
+              "void f2() { f1(-4,4); }");
+        ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", errout.str());
+    }
+};
+
+REGISTER_TEST(TestType)