Fixed #3327 (printf with std::string as parameter)

lib/checkother.cpp

@@ -1230,13 +1230,35 @@ void CheckOther::invalidScanfError(const Token *tok)
 }
 
 //---------------------------------------------------------------------------
-//    printf("%u", "xyz"); // Wrong argument type. TODO.
+//    printf("%u", "xyz"); // Wrong argument type
 //    printf("%u%s", 1); // Too few arguments
 //    printf("", 1); // Too much arguments
 //---------------------------------------------------------------------------
+static bool isComplexType(const Variable* var, const Token* varTypeTok)
+{
+    if (var->type())
+        return(true);
+
+    static std::set<std::string> knownTypes;
+    if (knownTypes.empty()) {
+        knownTypes.insert("struct"); // If a type starts with the struct keyword, its a complex type
+        knownTypes.insert("string");
+    }
+
+    if (varTypeTok->str() == "std")
+        varTypeTok = varTypeTok->tokAt(2);
+    return(knownTypes.find(varTypeTok->str()) != knownTypes.end() && !var->isPointer() && !var->isArray());
+}
+
+static bool isKnownType(const Variable* var, const Token* varTypeTok)
+{
+    return(varTypeTok->isStandardType() || varTypeTok->next()->isStandardType() || isComplexType(var, varTypeTok));
+}
 
 void CheckOther::checkWrongPrintfScanfArguments()
 {
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
         if (!tok->isName()) continue;
 
@@ -1286,6 +1308,7 @@ void CheckOther::checkWrongPrintfScanfArguments()
         bool scan = Token::Match(tok, "sscanf|fscanf|scanf");
         unsigned int numFormat = 0;
         bool percent = false;
+        const Token* argListTok2 = argListTok;
         for (std::string::iterator i = formatString.begin(); i != formatString.end(); ++i) {
             if (*i == '%') {
                 percent = !percent;
@@ -1293,6 +1316,8 @@ void CheckOther::checkWrongPrintfScanfArguments()
                 while (i != formatString.end()) {
                     if (*i == ']') {
                         numFormat++;
+                        if (argListTok)
+                            argListTok = argListTok->nextArgument();
                         percent = false;
                         break;
                     }
@@ -1308,8 +1333,11 @@ void CheckOther::checkWrongPrintfScanfArguments()
                     if (*i == '*') {
                         if (scan)
                             _continue = true;
-                        else
+                        else {
                             numFormat++;
+                            if (argListTok)
+                                argListTok = argListTok->nextArgument();
+                        }
                     }
                     ++i;
                 }
@@ -1318,18 +1346,80 @@ void CheckOther::checkWrongPrintfScanfArguments()
                 if (_continue)
                     continue;
 
-                if (*i != 'm') // %m is a non-standard extension that requires no parameter
+                if (*i != 'm') { // %m is a non-standard extension that requires no parameter
                     numFormat++;
 
-                // TODO: Perform type checks
+                    // Perform type checks
+                    if (_settings->isEnabled("style") && Token::Match(argListTok, "%any% ,|)")) { // We can currently only check the type of arguments matching this simple pattern.
+                        const Variable* variableInfo = symbolDatabase->getVariableFromVarId(argListTok->varId());
+                        const Token* varTypeTok = variableInfo ? variableInfo->typeStartToken() : NULL;
+                        if (varTypeTok && varTypeTok->str() == "static")
+                            varTypeTok = varTypeTok->next();
+
+                        if (scan && varTypeTok) {
+                            if ((!variableInfo->isPointer() && !variableInfo->isArray()) || varTypeTok->str() == "const")
+                                invalidScanfArgTypeError(tok, tok->str(), numFormat);
+                        } else if (!scan) {
+                            switch (*i) {
+                            case 's':
+                                if (variableInfo && !Token::Match(argListTok, "%str%") && isKnownType(variableInfo, varTypeTok) && (!variableInfo->isPointer() && !variableInfo->isArray()))
+                                    invalidPrintfArgTypeError_s(tok, numFormat);
+                                break;
+                            case 'n':
+                                if ((varTypeTok && isKnownType(variableInfo, varTypeTok) && ((!variableInfo->isPointer() && !variableInfo->isArray()) || varTypeTok->str() == "const")) || Token::Match(argListTok, "%str%"))
+                                    invalidPrintfArgTypeError_n(tok, numFormat);
+                                break;
+                            case 'c':
+                            case 'd':
+                            case 'i':
+                            case 'u':
+                            case 'x':
+                            case 'X':
+                            case 'o':
+                                if (varTypeTok && varTypeTok->str() == "const")
+                                    varTypeTok = varTypeTok->next();
+                                if ((varTypeTok && isKnownType(variableInfo, varTypeTok) && !Token::Match(varTypeTok, "unsigned|signed| bool|short|long|int|char|size_t|unsigned|signed") && !variableInfo->isPointer() && !variableInfo->isArray()))
+                                    invalidPrintfArgTypeError_int(tok, numFormat, *i);
+                                else if (Token::Match(argListTok, "%str%"))
+                                    invalidPrintfArgTypeError_int(tok, numFormat, *i);
+                                break;
+                            case 'p':
+                                if (varTypeTok && varTypeTok->str() == "const")
+                                    varTypeTok = varTypeTok->next();
+                                if (varTypeTok && isKnownType(variableInfo, varTypeTok) && !Token::Match(varTypeTok, "unsigned|signed| short|long|int|size_t|unsigned|signed") && !variableInfo->isPointer() && !variableInfo->isArray())
+                                    invalidPrintfArgTypeError_p(tok, numFormat);
+                                else if (Token::Match(argListTok, "%str%"))
+                                    invalidPrintfArgTypeError_p(tok, numFormat);
+                                break;
+                            case 'e':
+                            case 'E':
+                            case 'f':
+                            case 'g':
+                            case 'G':
+                                if (varTypeTok && varTypeTok->str() == "const")
+                                    varTypeTok = varTypeTok->next();
+                                if (varTypeTok && (isKnownType(variableInfo, varTypeTok) && !Token::Match(varTypeTok, "float|double") || variableInfo->isPointer() || variableInfo->isArray()))
+                                    invalidPrintfArgTypeError_float(tok, numFormat, *i);
+                                else if (Token::Match(argListTok, "%str%"))
+                                    invalidPrintfArgTypeError_float(tok, numFormat, *i);
+                                break;
+                            default:
+                                break;
+                            }
+                        }
+                    }
+
+                    if (argListTok)
+                        argListTok = argListTok->nextArgument(); // Find next argument
+                }
             }
         }
 
         // Count printf/scanf parameters..
         unsigned int numFunction = 0;
-        while (argListTok) {
+        while (argListTok2) {
             numFunction++;
-            argListTok = argListTok->nextArgument(); // Find next argument
+            argListTok2 = argListTok2->nextArgument(); // Find next argument
         }
 
         // Mismatching number of parameters => warning
@@ -1356,7 +1446,44 @@ void CheckOther::wrongPrintfScanfArgumentsError(const Token* tok,
            << numFunction
            << " are given";
 
-    reportError(tok, severity, "wrongPrintfScanfArgs", errmsg.str());
+    reportError(tok, severity, "wrongPrintfScanfArgNum", errmsg.str());
+}
+
+void CheckOther::invalidScanfArgTypeError(const Token* tok, const std::string &functionName, unsigned int numFormat)
+{
+    std::ostringstream errmsg;
+    errmsg << functionName << " argument no. " << numFormat << ": requires non-const pointers or arrays as arguments";
+    reportError(tok, Severity::warning, "invalidScanfArgType", errmsg.str());
+}
+void CheckOther::invalidPrintfArgTypeError_s(const Token* tok, unsigned int numFormat)
+{
+    std::ostringstream errmsg;
+    errmsg << "%s in format string (no. " << numFormat << ") requires a char* given in the argument list";
+    reportError(tok, Severity::warning, "invalidPrintfArgType_s", errmsg.str());
+}
+void CheckOther::invalidPrintfArgTypeError_n(const Token* tok, unsigned int numFormat)
+{
+    std::ostringstream errmsg;
+    errmsg << "%n in format string (no. " << numFormat << ") requires a pointer to an non-const integer given in the argument list";
+    reportError(tok, Severity::warning, "invalidPrintfArgType_n", errmsg.str());
+}
+void CheckOther::invalidPrintfArgTypeError_p(const Token* tok, unsigned int numFormat)
+{
+    std::ostringstream errmsg;
+    errmsg << "%p in format string (no. " << numFormat << ") requires an integer or pointer given in the argument list";
+    reportError(tok, Severity::warning, "invalidPrintfArgType_p", errmsg.str());
+}
+void CheckOther::invalidPrintfArgTypeError_int(const Token* tok, unsigned int numFormat, char c)
+{
+    std::ostringstream errmsg;
+    errmsg << "%" << c << " in format string (no. " << numFormat << ") requires an integer given in the argument list";
+    reportError(tok, Severity::warning, "invalidPrintfArgType_int", errmsg.str());
+}
+void CheckOther::invalidPrintfArgTypeError_float(const Token* tok, unsigned int numFormat, char c)
+{
+    std::ostringstream errmsg;
+    errmsg << "%" << c << " in format string (no. " << numFormat << ") requires a floating point number given in the argument list";
+    reportError(tok, Severity::warning, "invalidPrintfArgType_float", errmsg.str());
 }
 
 //---------------------------------------------------------------------------

lib/checkother.h

@@ -179,6 +179,12 @@ public:
                                         const std::string &function,
                                         unsigned int numFormat,
                                         unsigned int numFunction);
+    void invalidScanfArgTypeError(const Token* tok, const std::string &functionName, unsigned int numFormat);
+    void invalidPrintfArgTypeError_s(const Token* tok, unsigned int numFormat);
+    void invalidPrintfArgTypeError_n(const Token* tok, unsigned int numFormat);
+    void invalidPrintfArgTypeError_p(const Token* tok, unsigned int numFormat);
+    void invalidPrintfArgTypeError_int(const Token* tok, unsigned int numFormat, char c);
+    void invalidPrintfArgTypeError_float(const Token* tok, unsigned int numFormat, char c);
 
     /** @brief %Check for assigning to the same variable twice in a switch statement*/
     void checkRedundantAssignmentInSwitch();
@@ -357,6 +363,12 @@ public:
         c.comparisonOfBoolExpressionWithIntError(0);
         c.SuspiciousSemicolonError(0);
         c.wrongPrintfScanfArgumentsError(0,"printf",3,2);
+        c.invalidScanfArgTypeError(0, "scanf", 1);
+        c.invalidPrintfArgTypeError_s(0, 1);
+        c.invalidPrintfArgTypeError_n(0, 1);
+        c.invalidPrintfArgTypeError_p(0, 1);
+        c.invalidPrintfArgTypeError_int(0, 1, 'u');
+        c.invalidPrintfArgTypeError_float(0, 1, 'f');
         c.cctypefunctionCallError(0, "funname", "value");
     }
 

test/testother.cpp

@@ -2098,10 +2098,7 @@ private:
               "    snprintf(str,10,\"%u%s\");\n"
               "    sprintf(string1, \"%-*.*s\", 32, string2);\n" // #3364
               "    sprintf(string1, \"%*\", 32);\n" // #3364
-              "}\n",
+              "}");
-              "test.cpp",
-              true
-             );
         ASSERT_EQUALS("[test.cpp:2]: (error) printf format string has 1 parameters but only 0 are given\n"
                       "[test.cpp:3]: (error) printf format string has 2 parameters but only 1 are given\n"
                       "[test.cpp:4]: (error) printf format string has 3 parameters but only 2 are given\n"
@@ -2115,10 +2112,7 @@ private:
               "    printf(\"\", 0);\n"
               "    printf(\"%u\", 123, bar());\n"
               "    printf(\"%u%s\", 0, bar(), 43123);\n"
-              "}\n",
+              "}");
-              "test.cpp",
-              true
-             );
         ASSERT_EQUALS("[test.cpp:2]: (warning) printf format string has 0 parameters but 1 are given\n"
                       "[test.cpp:3]: (warning) printf format string has 1 parameters but 2 are given\n"
                       "[test.cpp:4]: (warning) printf format string has 2 parameters but 3 are given\n", errout.str());
@@ -2135,11 +2129,83 @@ private:
               "    fprintf(stderr, \"error: %m\n\");\n" // #3339
               "    printf(\"string: %.*s\n\", len, string);\n" // #3311
               "    fprintf(stderr, \"%*cText.\n\", indent, ' ');\n" // #3313
-              "}\n",
+              "}");
-              "test.cpp",
-              true
-             );
         ASSERT_EQUALS("", errout.str());
+
+        check("void foo(char* s, const char* s2, std::string s3, int i) {\n"
+              "    printf(\"%s%s\", s, s2);\n"
+              "    printf(\"%s\", i);\n"
+              "    printf(\"%i%s\", i, i);\n"
+              "    printf(\"%s\", s3);\n"
+              "    printf(\"%s\", \"s4\");\n"
+              "    printf(\"%u\", s);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) %s in format string (no. 1) requires a char* given in the argument list\n"
+                      "[test.cpp:4]: (warning) %s in format string (no. 2) requires a char* given in the argument list\n"
+                      "[test.cpp:5]: (warning) %s in format string (no. 1) requires a char* given in the argument list\n", errout.str());
+
+        check("void foo(const int* cpi, const int ci, int i, int* pi, std::string s) {\n"
+              "    printf(\"%n\", cpi);\n"
+              "    printf(\"%n\", ci);\n"
+              "    printf(\"%n\", i);\n"
+              "    printf(\"%n\", pi);\n"
+              "    printf(\"%n\", s);\n"
+              "    printf(\"%n\", \"s4\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n"
+                      "[test.cpp:3]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n"
+                      "[test.cpp:4]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n"
+                      "[test.cpp:6]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n"
+                      "[test.cpp:7]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n", errout.str());
+
+        check("class foo {};\n"
+              "void foo(const int* cpi, foo f, bar b, bar* bp, double d) {\n"
+              "    printf(\"%i\", f);\n"
+              "    printf(\"%c\", \"s4\");\n"
+              "    printf(\"%o\", d);\n"
+              "    printf(\"%i\", cpi);\n"
+              "    printf(\"%u\", b);\n"
+              "    printf(\"%u\", bp);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) %i in format string (no. 1) requires an integer given in the argument list\n"
+                      "[test.cpp:4]: (warning) %c in format string (no. 1) requires an integer given in the argument list\n"
+                      "[test.cpp:5]: (warning) %o in format string (no. 1) requires an integer given in the argument list\n", errout.str());
+
+        check("class foo {};\n"
+              "void foo(const int* cpi, foo f, bar b, bar* bp, char c) {\n"
+              "    printf(\"%p\", f);\n"
+              "    printf(\"%p\", c);\n"
+              "    printf(\"%p\", bp);\n"
+              "    printf(\"%p\", cpi);\n"
+              "    printf(\"%p\", b);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) %p in format string (no. 1) requires an integer or pointer given in the argument list\n"
+                      "[test.cpp:4]: (warning) %p in format string (no. 1) requires an integer or pointer given in the argument list\n", errout.str());
+
+        check("class foo {};\n"
+              "void foo(const int* cpi, foo f, bar b, bar* bp, double d) {\n"
+              "    printf(\"%e\", f);\n"
+              "    printf(\"%E\", \"s4\");\n"
+              "    printf(\"%f\", cpi);\n"
+              "    printf(\"%G\", bp);\n"
+              "    printf(\"%f\", d);\n"
+              "    printf(\"%f\", b);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) %e in format string (no. 1) requires a floating point number given in the argument list\n"
+                      "[test.cpp:4]: (warning) %E in format string (no. 1) requires a floating point number given in the argument list\n"
+                      "[test.cpp:5]: (warning) %f in format string (no. 1) requires a floating point number given in the argument list\n"
+                      "[test.cpp:6]: (warning) %G in format string (no. 1) requires a floating point number given in the argument list\n", errout.str());
+
+        check("class foo;\n"
+              "void foo(foo f) {\n"
+              "    printf(\"%u\", f);\n"
+              "    printf(\"%f\", f);\n"
+              "    printf(\"%p\", f);\n"
+              "}");
+        TODO_ASSERT_EQUALS("[test.cpp:3]: (warning) %u in format string (no. 1) requires an integer given in the argument list\n"
+                           "[test.cpp:4]: (warning) %f in format string (no. 1) requires an integer given in the argument list\n"
+                           "[test.cpp:5]: (warning) %p in format string (no. 1) requires an integer given in the argument list\n", "", errout.str());
+
     }
 
     void trac1132() {