Fix issue with functions pointers

Author: pfultz2

lib/checkio.cpp

@@ -1554,7 +1554,7 @@ CheckIOImpl::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings &setti
         while (top->astParent() && top->astParent()->str() != "," && top->astParent() != arg->previous())
             top = top->astParent();
         const ValueType *valuetype = top->argumentType();
-        if (valuetype && valuetype->type >= ValueType::Type::BOOL) {
+        if (valuetype && valuetype->isPrimitive()) {
             typeToken = tempToken = new Token(top);
             if (valuetype->pointer && valuetype->constness & 1) {
                 tempToken->str("const");
@@ -1583,7 +1583,10 @@ CheckIOImpl::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings &setti
             else if (valuetype->type == ValueType::LONGDOUBLE) {
                 tempToken->str("double");
                 tempToken->isLong(true);
-            }
+            } else if (valuetype->type == ValueType::FUNCTION)
+                tempToken->str("function");
+            else if (valuetype->type == ValueType::MEMBER_POINTER)
+                tempToken->str("member-pointer");
             if (valuetype->isIntegral()) {
                 if (valuetype->sign == ValueType::Sign::UNSIGNED)
                     tempToken->isUnsigned(true);

lib/symboldatabase.cpp

@@ -7363,13 +7363,19 @@ static ValueType memberPointerType(const ValueType& member)
 }
 
 // The "(" that opens the parenthesized declarator of a pointer or reference to a
-// function ("int ( * f ) ( int )"), nullptr when var is not declared that way
-static const Token* functionDeclarator(const Variable& var)
+// function in the declaration whose return type starts at the given token
+// ("int ( * f ) ( int )"), nullptr when there is no such declarator
+static const Token* functionDeclarator(const Token* returnType)
 {
-    const Token* tok = var.nameToken() ? var.nameToken()->previous() : nullptr;
-    while (Token::Match(tok, "*|&|&&|const|volatile"))
-        tok = tok->previous();
-    return Token::simpleMatch(tok, "(") ? tok : nullptr;
+    for (const Token* tok = returnType; tok; tok = tok->next()) {
+        if (Token::Match(tok, "( *|&|&&") && Token::simpleMatch(tok->link(), ") ("))
+            return tok;
+        if (tok->str() == "<" && tok->link())
+            tok = tok->link();
+        else if (!Token::Match(tok, "%name%|*|&|&&|::"))
+            return nullptr;
+    }
+    return nullptr;
 }
 
 void SymbolDatabase::setValueType(Token* tok, const ValueType& valuetype, const SourceLocation &loc)
@@ -7509,12 +7515,11 @@ void SymbolDatabase::setValueType(Token* tok, const ValueType& valuetype, const
     }
     // a call through a pointer or reference to a function has the return type of the
     // function: the declared type before the parenthesized declarator "int ( * f ) ( int )"
-    if (Token::Match(parent->tokAt(-1), "%name% (") && parent->astOperand1() == tok && valuetype.type == ValueType::Type::FUNCTION &&
-        valuetype.pointer <= 1 && tok->variable()) {
-        const Variable* const var = tok->variable();
-        const Token* const declarator = functionDeclarator(*var);
+    if (parent->str() == "(" && parent->astOperand1() == tok && !parent->isCast() && valuetype.type == ValueType::Type::FUNCTION &&
+        valuetype.pointer <= 1 && valuetype.returnTypeToken) {
+        const Token* const declarator = functionDeclarator(valuetype.returnTypeToken);
         ValueType returnType;
-        if (declarator && parsedecl(var->typeStartToken(), &returnType, mDefaultSignedness, mSettings, declarator))
+        if (declarator && parsedecl(valuetype.returnTypeToken, &returnType, mDefaultSignedness, mSettings, declarator))
             setValueType(parent, returnType);
         return;
     }
@@ -7972,6 +7977,7 @@ static const Token* parsedecl(const Token* type,
     const int pointer0 = valuetype->pointer;
     while (Token::Match(type->previous(), "%name%") && !endsWith(type->strAt(-1), ':'))
         type = type->previous();
+    const Token* const typeStart = type;
     valuetype->sign = ValueType::Sign::UNKNOWN_SIGN;
     if (!valuetype->typeScope && !valuetype->smartPointerType)
         valuetype->type = ValueType::Type::UNKNOWN_TYPE;
@@ -8008,6 +8014,7 @@ static const Token* parsedecl(const Token* type,
                 function.type = ValueType::Type::FUNCTION;
                 function.bits = valuetype->bits;
                 function.pointer = pointer0;
+                function.returnTypeToken = typeStart;
                 if (type->strAt(1) == "&")
                     function.reference = Reference::LValue;
                 else if (type->strAt(1) == "&&")
@@ -8032,6 +8039,7 @@ static const Token* parsedecl(const Token* type,
             valuetype->constness += vt2->constness;
             valuetype->pointer += vt2->pointer;
             valuetype->reference = vt2->reference;
+            valuetype->returnTypeToken = vt2->returnTypeToken;
             type = type->linkAt(1)->next();
             continue;
         }
@@ -8095,6 +8103,7 @@ static const Token* parsedecl(const Token* type,
             valuetype->constness = vt->constness;
             valuetype->volatileness = vt->volatileness;
             valuetype->originalTypeName = vt->originalTypeName;
+            valuetype->returnTypeToken = vt->returnTypeToken;
             const bool hasConst = Token::simpleMatch(type->previous(), "const");
             const bool hasVolatile = Token::simpleMatch(type->previous(), "volatile");
             while (Token::Match(type, "%name%|*|&|&&|::") && !type->variable()) {

lib/symboldatabase.h

@@ -1230,6 +1230,9 @@ class CPPCHECKLIB Scope {
 class CPPCHECKLIB ValueType {
 public:
     enum Sign : std::uint8_t { UNKNOWN_SIGN, SIGNED, UNSIGNED } sign = UNKNOWN_SIGN;
+    /** The order matters: the types before VOID are class types that may have user defined
+     * constructors, assignment or operators. VOID and the types after it are built in types
+     * with trivial copy and move, the ones from BOOL on are arithmetic (isIntegral, isFloat). */
     enum Type : std::uint8_t {
         UNKNOWN_TYPE,
         POD,
@@ -1238,9 +1241,9 @@ class CPPCHECKLIB ValueType {
         SMART_POINTER,
         CONTAINER,
         ITERATOR,
+        VOID,
         FUNCTION,       ///< a function; a pointer to it ("int (*)(int)") is counted in pointer
         MEMBER_POINTER, ///< a pointer to a data member ("int S::*"), counted in pointer
-        VOID,
         BOOL,
         CHAR,
         SHORT,
@@ -1267,6 +1270,8 @@ class CPPCHECKLIB ValueType {
     ///< container
     const Token* containerTypeToken{}; ///< The container type token. the template argument token that defines the
     ///< container element type.
+    const Token* returnTypeToken{}; ///< FUNCTION: the first token of the return type in the declaration of the
+    ///< function ("int ( * f ) ( int )"), the type of a call is parsed from it
     std::string originalTypeName;    ///< original type name as written in the source code. eg. this might be "uint8_t"
     ///< when type is CHAR.
     ErrorPath debugPath; ///< debug path to the type
@@ -1306,8 +1311,9 @@ class CPPCHECKLIB ValueType {
     static MatchResult matchParameter(const ValueType *call, const ValueType *func);
     static MatchResult matchParameter(const ValueType *call, const Variable *callVar, const Variable *funcVar);
 
+    /** a built in type that is not void: arithmetic (or a pointer to it), a pointer to a function or to a member */
     bool isPrimitive() const {
-        return (type >= ValueType::Type::BOOL);
+        return (type >= ValueType::Type::BOOL || type == ValueType::Type::FUNCTION || type == ValueType::Type::MEMBER_POINTER);
     }
 
     bool isIntegral() const {

test/testio.cpp

@@ -5001,6 +5001,21 @@ class TestIO : public TestFixture {
               "    printf(\"%i\", fp());\n"
               "}\n");
         ASSERT_EQUALS("", errout_str());
+
+        check("struct S { int a; };\n" // a pointer to a function or to a member is a pointer
+              "void test(int (*cb)(int), int S::*pm) {\n"
+              "    auto acb = cb;\n"
+              "    printf(\"%d %d %d %p %p\", cb, pm, acb, cb, acb);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4:5]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'function *'. [invalidPrintfArgType_sint]\n"
+                      "[test.cpp:4:5]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'member-pointer *'. [invalidPrintfArgType_sint]\n"
+                      "[test.cpp:4:5]: (warning) %d in format string (no. 3) requires 'int' but the argument type is 'function *'. [invalidPrintfArgType_sint]\n", errout_str());
+
+        check("void test(char* (*cs)(int)) {\n" // a call through a copy of the pointer has the return type of the function
+              "    auto acs = cs;\n"
+              "    printf(\"%d %s\", acs(1), (*cs)(2));\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3:5]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'char *'. [invalidPrintfArgType_sint]\n", errout_str());
     }
 
     void testPrintf0WithSuffix() { // ticket #7069

test/testleakautovar.cpp

@@ -712,6 +712,15 @@ class TestLeakAutoVar : public TestFixture {
               "    p->f();\n"
               "}\n", dinit(CheckOptions, $.cpp = true));
         ASSERT_EQUALS("[test.cpp:9:1]: (error) Memory leak: p [memleak]\n", errout_str());
+
+        check("struct S { int (*cb)(int); };\n" // a pointer to a function or to a member is a primitive member
+              "struct T { int S::*pm; };\n"
+              "void f() {\n"
+              "    S* s = new S;\n"
+              "    T* t = new T;\n"
+              "}\n", dinit(CheckOptions, $.cpp = true));
+        ASSERT_EQUALS("[test.cpp:6:1]: (error) Memory leak: s [memleak]\n"
+                      "[test.cpp:6:1]: (error) Memory leak: t [memleak]\n", errout_str());
     }
 
     void realloc1() {

test/testother.cpp

@@ -13136,6 +13136,16 @@ class TestOther : public TestFixture {
               "    g(*w);\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:5:10]: (style) Variable 'w' can be declared as pointer to const [constVariablePointer]\n", errout_str());
+
+        check("struct S { int a; };\n" // moving a pointer to a function or to a member copies it
+              "void h(int (*)(int));\n"
+              "int f(int (*cb)(int), const S& s, int S::*pm) {\n"
+              "    int (*c)(int) = std::move(cb);\n"
+              "    int S::*q = std::move(pm);\n"
+              "    h(std::move(cb));\n"
+              "    return cb(1) + c(2) + s.*pm + s.*q;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
     }
 
     void moveAndAddressOf() {

test/testsymboldatabase.cpp

@@ -10739,6 +10739,8 @@ class TestSymbolDatabase : public TestFixture {
                       "    auto acb = cb; auto apm = &S::a;\n"
                       "    g(cb); g(rf); g(*pcb); g(cbs[0]); g(pa); g(pmf); g(pm); g(&S::a); g(&S::s); g(&s.a); g(p); g(acb); g(apm);\n"
                       "    g(cb(1)); g(rf(2)); g(pf(3));\n"
+                      "    g(acb(4)); g((*cb)(5)); g(cbs[0](6)); g((*pcb)(7));\n"
+                      "    decltype(cb) dcb = cb; g(dcb(8));\n"
                       "}\n");
         ASSERT(db != nullptr);
         const Token* const tokens = tokenizer.tokens();
@@ -10759,6 +10761,12 @@ class TestSymbolDatabase : public TestFixture {
         ASSERT_EQUALS("signed int", argumentType(tokens, "g ( cb ( 1 ) )"));
         ASSERT_EQUALS("signed int", argumentType(tokens, "g ( rf ( 2 ) )"));
         ASSERT_EQUALS("signed int *", argumentType(tokens, "g ( pf ( 3 ) )"));
+        // also when the pointer is not the declared variable: a copy, a dereference, an element
+        ASSERT_EQUALS("signed int", argumentType(tokens, "g ( acb ( 4 ) )"));
+        ASSERT_EQUALS("signed int", argumentType(tokens, "g ( ( * cb ) ( 5 ) )"));
+        ASSERT_EQUALS("signed int", argumentType(tokens, "g ( cbs [ 0 ] ( 6 ) )"));
+        ASSERT_EQUALS("signed int", argumentType(tokens, "g ( ( * pcb ) ( 7 ) )"));
+        ASSERT_EQUALS("signed int", argumentType(tokens, "g ( dcb ( 8 ) )"));
     }
 
     // the bit count of the ValueType of the first token after start that matches pattern: