Added check for invalid pointer casts (#1255)

Detect sign extension problems when variable is a reference (#3637)
Refactorizations:
- Tokenizer::getFiles returns a reference instead of a pointer, because its guaranteed that no nullpointer is returned
- Remove signed/unsigned in one step for "%type% signed|unsigned"
- Fixed recently introduced compiler warning in symboldatabase.cpp

lib/check.h

@@ -161,8 +161,8 @@ private:
         }
 
         ErrorLogger::ErrorMessage errmsg(locationList, severity, msg, id, inconclusive);
-        if (_tokenizer && _tokenizer->getFiles() && !_tokenizer->getFiles()->empty())
+        if (_tokenizer && !_tokenizer->getFiles().empty())
-            errmsg.file0 = _tokenizer->getFiles()->at(0);
+            errmsg.file0 = _tokenizer->getFiles()[0];
         if (_errorLogger)
             _errorLogger->reportErr(errmsg);
         else

lib/checkbufferoverrun.cpp

@@ -1319,7 +1319,7 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable()
         if (tok->previous() && (!tok->previous()->isName() && !Token::Match(tok->previous(), "[;{}]")))
             continue;
 
-        _errorLogger->reportProgress(_tokenizer->getFiles()->front(),
+        _errorLogger->reportProgress(_tokenizer->getFiles().front(),
                                      "Check (BufferOverrun::checkGlobalAndLocalVariable)",
                                      tok->progressValue());
 

lib/checkother.cpp

@@ -322,6 +322,97 @@ void CheckOther::cstyleCastError(const Token *tok)
     reportError(tok, Severity::style, "cstyleCast", "C-style pointer casting");
 }
 
+//---------------------------------------------------------------------------
+// float* f; double* d = (double*)f; <-- Pointer cast to a type with an incompatible binary data representation
+//---------------------------------------------------------------------------
+
+static std::string analyzeType(const Token* tok)
+{
+    if (tok->str() == "double")
+        if (tok->isLong())
+            return "long double";
+        else
+            return "double";
+    if (tok->str() == "float")
+        return "float";
+    if (Token::Match(tok, "unsigned| int|long|short|char|size_t"))
+        return "integer";
+    return "";
+}
+
+void CheckOther::invalidPointerCast()
+{
+    if (!_settings->isEnabled("style") && !_settings->isEnabled("portability"))
+        return;
+
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        const Token* toTok = 0;
+        const Token* nextTok = 0;
+        // Find cast
+        if (Token::Match(tok, "( const| %type% const| * )") || Token::Match(tok, "( const| %type% %type% const| * )")) {
+            toTok = tok->next();
+            nextTok = tok->link()->next();
+            if (nextTok->str() == "(")
+                nextTok = nextTok->next();
+        } else if (Token::Match(tok, "reinterpret_cast < const| %type% const| * > (") || Token::Match(tok, "reinterpret_cast < const| %type% %type% const| * > (")) {
+            nextTok = tok->tokAt(5);
+            while (nextTok->str() != "(")
+                nextTok = nextTok->next();
+            nextTok = nextTok->next();
+            toTok = tok->tokAt(2);
+        }
+        if (toTok && toTok->str() == "const")
+            toTok = toTok->next();
+
+        if (!nextTok || !toTok || !toTok->isStandardType())
+            continue;
+
+        // Find casted variable
+        unsigned int varid = 0;
+        bool allocation = false;
+        bool ref = false;
+        if (Token::Match(nextTok, "new %type%"))
+            allocation = true;
+        else if (Token::Match(nextTok, "%var% !!["))
+            varid = nextTok->varId();
+        else if (Token::Match(nextTok, "& %var%") && !Token::Match(nextTok->tokAt(2), "(|[")) {
+            varid = nextTok->next()->varId();
+            ref = true;
+        }
+
+        const Token* fromTok = 0;
+
+        if (allocation) {
+            fromTok = nextTok->next();
+        } else {
+            const Variable* var = symbolDatabase->getVariableFromVarId(varid);
+            if (!var || (!ref && !var->isPointer() && !var->isArray()) || (ref && (var->isPointer() || var->isArray())))
+                continue;
+            fromTok = var->typeStartToken();
+        }
+
+        while (Token::Match(fromTok, "static|const"))
+            fromTok = fromTok->next();
+        if (!fromTok->isStandardType())
+            continue;
+
+        std::string fromType = analyzeType(fromTok);
+        std::string toType = analyzeType(toTok);
+        if (fromType != toType && !fromType.empty() && !toType.empty() && (toType != "integer" || _settings->isEnabled("portability")))
+            invalidPointerCastError(tok, fromType, toType);
+    }
+}
+
+void CheckOther::invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to)
+{
+    if (to == "integer") // If we cast something to int*, this can be useful to play with its binary data representation
+        reportError(tok, Severity::portability, "invalidPointerCast", "Casting from " + from + "* to integer* is not portable due to different binary data representations on different platforms");
+    else
+        reportError(tok, Severity::warning, "invalidPointerCast", "Casting between " + from + "* and " + to + "* which have an incompatible binary data representation");
+}
+
 //---------------------------------------------------------------------------
 // fflush(stdin) <- fflush only applies to output streams in ANSI C
 //---------------------------------------------------------------------------
@@ -1944,7 +2035,7 @@ void CheckOther::passedByValueError(const Token *tok, const std::string &parname
 //---------------------------------------------------------------------------
 static bool isChar(const Variable* var)
 {
-    return(var && !var->isPointer() && !var->isArray() && (var->typeEndToken()->str() == "char" || (var->typeEndToken()->previous() && var->typeEndToken()->previous()->str() == "char")));
+    return(var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "static| const| char"));
 }
 
 static bool isSignedChar(const Variable* var)
@@ -1982,11 +2073,8 @@ void CheckOther::checkCharVariable()
 
             // is the result stored in a short|int|long?
             const Variable *var = symbolDatabase->getVariableFromVarId(tok->next()->varId());
-            if (!(var && Token::Match(var->typeEndToken(), "short|int|long")))
+            if (var && Token::Match(var->typeStartToken(), "static| const| short|int|long") && !var->isPointer() && !var->isArray())
-                continue;
+                charBitOpError(tok->tokAt(4)); // This is an error..
-
-            // This is an error..
-            charBitOpError(tok->tokAt(4));
         }
 
         else if (Token::Match(tok, "[;{}] %var% = %any% [&|] ( * %var% ) ;")) {
@@ -1999,11 +2087,8 @@ void CheckOther::checkCharVariable()
 
             // is the result stored in a short|int|long?
             var = symbolDatabase->getVariableFromVarId(tok->next()->varId());
-            if (!(var && Token::Match(var->typeEndToken(), "short|int|long")))
+            if (var && Token::Match(var->typeStartToken(), "static| const| short|int|long") && !var->isPointer() && !var->isArray())
-                continue;
+                charBitOpError(tok->tokAt(4)); // This is an error..
-
-            // This is an error..
-            charBitOpError(tok->tokAt(4));
         }
     }
 }

lib/checkother.h

@@ -51,6 +51,7 @@ public:
 
         // Coding style checks
         checkOther.warningOldStylePointerCast();
+        checkOther.invalidPointerCast();
         checkOther.checkUnsignedDivision();
         checkOther.checkCharVariable();
         checkOther.strPlusChar();
@@ -120,6 +121,9 @@ public:
     /** @brief Are there C-style pointer casts in a c++ file? */
     void warningOldStylePointerCast();
 
+    /** @brief Check for pointer casts to a type with an incompatible binary data representation */
+    void invalidPointerCast();
+
     /**
      * @brief Invalid function usage (invalid radix / overlapping data)
      *
@@ -263,6 +267,7 @@ public:
 
     // Error messages..
     void cstyleCastError(const Token *tok);
+    void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to);
     void dangerousUsageStrtolError(const Token *tok);
     void sprintfOverlappingDataError(const Token *tok, const std::string &varname);
     void udivError(const Token *tok, bool inconclusive);
@@ -325,6 +330,7 @@ public:
         c.sizeofForNumericParameterError(0);
         c.coutCerrMisusageError(0, "cout");
         c.doubleFreeError(0, "varname");
+        c.invalidPointerCastError(0, "float", "double");
 
         // style/warning
         c.cstyleCastError(0);
@@ -396,6 +402,7 @@ public:
 
                // style
                "* C-style pointer cast in cpp file\n"
+               "* casting between incompatible pointer types\n"
                "* redundant if\n"
                "* bad usage of the function 'strtol'\n"
                "* [[CheckUnsignedDivision|unsigned division]]\n"

lib/cppcheck.cpp

@@ -333,8 +333,8 @@ void CppCheck::checkFile(const std::string &code, const char FileName[])
         }
 
         // Update the _dependencies..
-        if (_tokenizer.getFiles()->size() >= 2)
+        if (_tokenizer.getFiles().size() >= 2)
-            _dependencies.insert(_tokenizer.getFiles()->begin()+1, _tokenizer.getFiles()->end());
+            _dependencies.insert(_tokenizer.getFiles().begin()+1, _tokenizer.getFiles().end());
 
         // call all "runChecks" in all registered Check classes
         for (std::list<Check *>::iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) {

lib/symboldatabase.cpp

@@ -551,12 +551,12 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
         for (std::list<Scope::FriendInfo>::iterator i = it->friendList.begin(); i != it->friendList.end(); ++i) {
             for (std::list<Scope>::iterator j = scopeList.begin(); j != scopeList.end(); ++j) {
                 // check scope for match
-                const Scope *scope = j->findQualifiedScope(i->name);
+                scope = const_cast<Scope*>(j->findQualifiedScope(i->name));
 
                 // found match?
                 if (scope && scope->isClassOrStruct()) {
                     // set found scope
-                    i->scope = const_cast<Scope *>(scope);
+                    i->scope = scope;
                     break;
                 }
             }

lib/tokenize.cpp

@@ -82,9 +82,9 @@ const Token *Tokenizer::tokens() const
 }
 
 
-const std::vector<std::string> *Tokenizer::getFiles() const
+const std::vector<std::string>& Tokenizer::getFiles() const
 {
-    return &_files;
+    return _files;
 }
 
 //---------------------------------------------------------------------------
@@ -5522,12 +5522,13 @@ void Tokenizer::simplifyStdType()
     for (Token *tok = _tokens; tok; tok = tok->next()) {
         // long unsigned => unsigned long
         if (Token::Match(tok, "char|short|int|long|__int8|__int16|__int32|__int64 unsigned|signed")) {
-            std::string temp = tok->str();
+            bool isUnsigned = tok->next()->str() == "unsigned";
-            tok->str(tok->next()->str());
+            tok->deleteNext();
-            tok->next()->str(temp);
+            tok->isUnsigned(isUnsigned);
+            tok->isSigned(!isUnsigned);
         }
 
-        if (!Token::Match(tok, "unsigned|signed|char|short|int|long|__int8|__int16|__int32|__int64"))
+        else if (!Token::Match(tok, "unsigned|signed|char|short|int|long|__int8|__int16|__int32|__int64"))
             continue;
 
         // check if signed or unsigned specified
@@ -7397,7 +7398,7 @@ void Tokenizer::simplifyEnum()
             }
 
             if (enumType) {
-                const std::string pattern(className.empty() ? "" : (className + " :: " + enumType->str()).c_str());
+                const std::string pattern(className.empty() ? std::string("") : (className + " :: " + enumType->str()));
 
                 // count { and } for tok2
                 int level = 0;

lib/tokenize.h

@@ -150,7 +150,7 @@ public:
      * Simplify '* & ( %var% ) =' or any combination of '* &' and '()'
      * parentheses around '%var%' to '%var% ='
      */
-    void simplifyMulAndParens(void);
+    void simplifyMulAndParens();
 
     /**
      * Get parameter name of function
@@ -181,7 +181,7 @@ public:
      * The first filename is the filename for the sourcefile
      * @return vector with filenames
      */
-    const std::vector<std::string> *getFiles() const;
+    const std::vector<std::string>& getFiles() const;
 
     /**
      * Get function token by function name

test/testcharvar.cpp

@@ -37,6 +37,7 @@ private:
         TEST_CASE(array_index_2);
         TEST_CASE(bitop1);
         TEST_CASE(bitop2);
+        TEST_CASE(bitop3);
         TEST_CASE(return1);
         TEST_CASE(assignChar);
         TEST_CASE(and03);
@@ -124,6 +125,13 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void bitop3() {
+        check("void f(int& i, char& c) {\n"
+              "    i &= c;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) When using char variables in bit operations, sign extension can generate unexpected results.\n", errout.str());
+    }
+
     void return1() {
         check("void foo()\n"
               "{\n"

test/testother.cpp

@@ -65,6 +65,7 @@ private:
         TEST_CASE(varScope12);      // variable usage in inner loop
 
         TEST_CASE(oldStylePointerCast);
+        TEST_CASE(invalidPointerCast);
 
         TEST_CASE(dangerousStrolUsage);
 
@@ -780,6 +781,85 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void checkInvalidPointerCast(const char code[], bool portability = false) {
+        // Clear the error buffer..
+        errout.str("");
+
+        Settings settings;
+        settings.addEnabled("style");
+        if (portability)
+            settings.addEnabled("portability");
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        CheckOther checkOtherCpp(&tokenizer, &settings, this);
+        checkOtherCpp.invalidPointerCast();
+    }
+
+
+    void invalidPointerCast() {
+        checkInvalidPointerCast("void test() {\n"
+                                "    float *f = new float[10];\n"
+                                "    delete [] (double*)f;\n"
+                                "    delete [] (long double const*)(new float[10]);\n"
+                                "}");
+        TODO_ASSERT_EQUALS("[test.cpp:3]: (warning) Casting between float* and double* which have an incompatible binary data representation\n"
+                           "[test.cpp:4]: (warning) Casting between float* and long double* which have an incompatible binary data representation\n",
+                           "[test.cpp:3]: (warning) Casting between float* and double* which have an incompatible binary data representation\n"
+                           "[test.cpp:4]: (warning) Casting between float* and double* which have an incompatible binary data representation\n", errout.str());
+
+        checkInvalidPointerCast("void test(const float* f) {\n"
+                                "    double *d = (double*)f;\n"
+                                "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Casting between float* and double* which have an incompatible binary data representation\n", errout.str());
+
+        checkInvalidPointerCast("void test(double* d1) {\n"
+                                "    long double *ld = (long double*)d1;\n"
+                                "    double *d2 = (double*)ld;\n"
+                                "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Casting between double* and long double* which have an incompatible binary data representation\n"
+                      "[test.cpp:3]: (warning) Casting between long double* and double* which have an incompatible binary data representation\n", errout.str());
+
+        checkInvalidPointerCast("char* test(int* i) {\n"
+                                "    long double *d = (long double*)(i);\n"
+                                "    double *d = (double*)(i);\n"
+                                "    float *f = reinterpret_cast<float*>(i);\n"
+                                "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Casting between integer* and long double* which have an incompatible binary data representation\n"
+                      "[test.cpp:3]: (warning) Casting between integer* and double* which have an incompatible binary data representation\n"
+                      "[test.cpp:4]: (warning) Casting between integer* and float* which have an incompatible binary data representation\n", errout.str());
+
+        checkInvalidPointerCast("float* test(unsigned int* i) {\n"
+                                "    return (float*)i;\n"
+                                "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Casting between integer* and float* which have an incompatible binary data representation\n", errout.str());
+
+        checkInvalidPointerCast("float* test(unsigned int* i) {\n"
+                                "    return (float*)i[0];\n"
+                                "}");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInvalidPointerCast("float* test(double& d) {\n"
+                                "    return (float*)&d;\n"
+                                "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Casting between double* and float* which have an incompatible binary data representation\n", errout.str());
+
+
+        checkInvalidPointerCast("long long* test(float* f) {\n"
+                                "    return (long long*)f;\n"
+                                "}", false);
+        ASSERT_EQUALS("", errout.str());
+
+        checkInvalidPointerCast("long long* test(float* f, char* c) {\n"
+                                "    foo((long long*)f);\n"
+                                "    return reinterpret_cast<long long*>(c);\n"
+                                "}", true);
+        ASSERT_EQUALS("[test.cpp:2]: (portability) Casting from float* to integer* is not portable due to different binary data representations on different platforms\n", errout.str());
+    }
+
     void dangerousStrolUsage() {
         {
             sprintfUsage("int f(const char *num)\n"