Skip to content

Commit 8effc6c

Browse files
pfultz2pfultz2
authored
Fix 14937: False positive: inconclusive nullPointerRedundantCheck with thunk (#8755)
1 parent bc88bec commit 8effc6c

4 files changed

Lines changed: 136 additions & 11 deletions

File tree

‎lib/symboldatabase.cpp‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5861,7 +5861,10 @@ bool Scope::hasInlineOrLambdaFunction(const Token** tokStart, bool onlyInline) c
58615861
});
58625862
}
58635863

5864-
void Scope::findFunctionInBase(const Token* tok, size_t args, std::vector<const Function *> & matches) const
5864+
void Scope::findFunctionInBase(const std::string& name,
5865+
const Token* tok,
5866+
size_t args,
5867+
std::vector<const Function*>& matches) const
58655868
{
58665869
if (isClassOrStruct() && definedType && !definedType->derivedFrom.empty()) {
58675870
const std::vector<Type::BaseInfo> &derivedFrom = definedType->derivedFrom;
@@ -5871,7 +5874,7 @@ void Scope::findFunctionInBase(const Token* tok, size_t args, std::vector<const
58715874
if (base->classScope == this) // Ticket #5120, #5125: Recursive class; tok should have been found already
58725875
continue;
58735876

5874-
auto range = base->classScope->functionMap.equal_range(tok->str());
5877+
auto range = base->classScope->functionMap.equal_range(name);
58755878
for (auto it = range.first; it != range.second; ++it) {
58765879
const Function *func = it->second;
58775880
if (func->isDestructor() && !Token::simpleMatch(tok->tokAt(-1), "~"))
@@ -5882,7 +5885,7 @@ void Scope::findFunctionInBase(const Token* tok, size_t args, std::vector<const
58825885
}
58835886
}
58845887

5885-
base->classScope->findFunctionInBase(tok, args, matches);
5888+
base->classScope->findFunctionInBase(name, tok, args, matches);
58865889
}
58875890
}
58885891
}
@@ -6021,8 +6024,10 @@ static bool hasMatchingConstructor(const Scope* classScope, const ValueType* arg
60216024
});
60226025
}
60236026

6024-
const Function* Scope::findFunction(const Token *tok, bool requireConst, Reference ref) const
6027+
const Function* Scope::findFunction(const Token* tok, bool requireConst, Reference ref, const std::string& funcName) const
60256028
{
6029+
const std::string& name = funcName.empty() ? tok->str() : funcName;
6030+
60266031
const bool isCall = Token::Match(tok->next(), "(|{");
60276032

60286033
const std::vector<const Token *> arguments = getArguments(tok);
@@ -6032,8 +6037,8 @@ const Function* Scope::findFunction(const Token *tok, bool requireConst, Referen
60326037
// find all the possible functions that could match
60336038
const std::size_t args = arguments.size();
60346039

6035-
auto addMatchingFunctions = [&](const Scope *scope) {
6036-
auto range = scope->functionMap.equal_range(tok->str());
6040+
auto addMatchingFunctions = [&](const Scope* scope) {
6041+
auto range = scope->functionMap.equal_range(name);
60376042
for (auto it = range.first; it != range.second; ++it) {
60386043
const Function *func = it->second;
60396044
if (ref == Reference::LValue && func->hasRvalRefQualifier())
@@ -6068,7 +6073,7 @@ const Function* Scope::findFunction(const Token *tok, bool requireConst, Referen
60686073
const std::size_t numberOfMatchesNonBase = matches.size();
60696074

60706075
// check in base classes
6071-
findFunctionInBase(tok, args, matches);
6076+
findFunctionInBase(name, tok, args, matches);
60726077

60736078
// Non-call => Do not match parameters
60746079
if (!isCall) {
@@ -6298,8 +6303,8 @@ const Function* Scope::findFunction(const Token *tok, bool requireConst, Referen
62986303
matches.erase(itPure);
62996304

63006305
// Only one candidate left
6301-
if (matches.size() == 1 && std::none_of(functionList.begin(), functionList.end(), [tok](const Function& f) {
6302-
return startsWith(f.name(), tok->str() + " <");
6306+
if (matches.size() == 1 && std::none_of(functionList.begin(), functionList.end(), [&name](const Function& f) {
6307+
return startsWith(f.name(), name + " <");
63036308
}))
63046309
return matches[0];
63056310

@@ -7792,6 +7797,13 @@ static const Function* getFunction(const Token* tok) {
77927797
lambda = lvar->nameToken()->tokAt(2)->function();
77937798
if (lambda && lambda->retDef)
77947799
return lambda;
7800+
// calling an object of a class that overloads operator()
7801+
if (tok != lvar->nameToken() && !lvar->isPointer() && !lvar->isArray() && lvar->typeScope()) {
7802+
const Function* callOp =
7803+
lvar->typeScope()->findFunction(tok, lvar->isConst(), Reference::LValue, "operator()");
7804+
if (callOp && callOp->retDef)
7805+
return callOp;
7806+
}
77957807
}
77967808
return nullptr;
77977809
}

‎lib/symboldatabase.h‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,9 +1144,14 @@ class CPPCHECKLIB Scope {
11441144
* @brief find a function
11451145
* @param tok token of function call
11461146
* @param requireConst if const refers to a const variable only const methods should be matched
1147+
* @param ref reference qualification of the object the function is called on
1148+
* @param funcName name to look up instead of tok->str(), e.g. "operator()" when tok is a variable that is called
11471149
* @return pointer to function if found or NULL if not found
11481150
*/
1149-
const Function *findFunction(const Token *tok, bool requireConst=false, Reference ref=Reference::None) const;
1151+
const Function* findFunction(const Token* tok,
1152+
bool requireConst = false,
1153+
Reference ref = Reference::None,
1154+
const std::string& funcName = "") const;
11501155

11511156
const Scope *findRecordInNestedList(const std::string & name, bool isC = false) const;
11521157
Scope *findRecordInNestedList(const std::string & name, bool isC = false);
@@ -1210,7 +1215,10 @@ class CPPCHECKLIB Scope {
12101215
*/
12111216
bool isVariableDeclaration(const Token* tok, const Token*& vartok, const Token*& typetok) const;
12121217

1213-
void findFunctionInBase(const Token* tok, size_t args, std::vector<const Function *> & matches) const;
1218+
void findFunctionInBase(const std::string& name,
1219+
const Token* tok,
1220+
size_t args,
1221+
std::vector<const Function*>& matches) const;
12141222

12151223
/** @brief initialize varlist */
12161224
void getVariableList(const Token *start, const Token *end);

‎test/testnullpointer.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ class TestNullPointer : public TestFixture {
147147
TEST_CASE(nullpointer107); // #13682 (FP/FN cases around guards that depend on the pointer indirectly)
148148
TEST_CASE(nullpointer108);
149149
TEST_CASE(nullpointer109);
150+
TEST_CASE(nullpointer110); // #14937
150151
TEST_CASE(nullpointer_addressOf); // address of
151152
TEST_CASE(nullpointerSwitch); // #2626
152153
TEST_CASE(nullpointer_cast); // #4692
@@ -3128,6 +3129,24 @@ class TestNullPointer : public TestFixture {
31283129
ASSERT_EQUALS("", errout_str());
31293130
}
31303131

3132+
void nullpointer110()
3133+
{ // #14937 - noreturn member function called on operator() result
3134+
check("struct A {\n"
3135+
" [[noreturn]] void g(int);\n"
3136+
"};\n"
3137+
"template<class T>\n"
3138+
"struct Thunk {\n"
3139+
" T& operator()() const;\n"
3140+
"};\n"
3141+
"void f(Thunk<A> thunk, int* p) {\n"
3142+
" if (!p)\n"
3143+
" thunk().g(0);\n"
3144+
" *p = 1;\n"
3145+
"}",
3146+
dinit(CheckOptions, $.inconclusive = true));
3147+
ASSERT_EQUALS("", errout_str());
3148+
}
3149+
31313150
void nullpointer_addressOf() { // address of
31323151
check("void f() {\n"
31333152
" struct X *x = 0;\n"

‎test/testsymboldatabase.cpp‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,8 @@ class TestSymbolDatabase : public TestFixture {
542542
TEST_CASE(findFunction60);
543543
TEST_CASE(findFunction61);
544544
TEST_CASE(findFunction62); // #14272 - pointer passed to function is const
545+
TEST_CASE(findFunction63); // #14937 - member function of type returned by operator()
546+
TEST_CASE(findFunction64); // overloaded operator()
545547
TEST_CASE(findFunctionRef1);
546548
TEST_CASE(findFunctionRef2); // #13328
547549
TEST_CASE(findFunctionContainer);
@@ -8875,6 +8877,90 @@ class TestSymbolDatabase : public TestFixture {
88758877
ASSERT_EQUALS(2, functionCall->function()->token->linenr());
88768878
}
88778879

8880+
void findFunction63()
8881+
{ // #14937
8882+
GET_SYMBOL_DB("struct A {\n"
8883+
" void g(int);\n"
8884+
"};\n"
8885+
"template<class T>\n"
8886+
"struct Thunk {\n"
8887+
" T& operator()() const;\n"
8888+
"};\n"
8889+
"void f(Thunk<A> thunk) {\n"
8890+
" thunk().g(0);\n"
8891+
"}\n");
8892+
const Token* g = Token::findsimplematch(tokenizer.tokens(), "g ( 0 )");
8893+
ASSERT(g);
8894+
ASSERT(g->function());
8895+
ASSERT(g->function()->tokenDef);
8896+
ASSERT_EQUALS(2, g->function()->tokenDef->linenr());
8897+
const Token* call = Token::findsimplematch(tokenizer.tokens(), "( ) . g");
8898+
ASSERT(call && call->valueType());
8899+
ASSERT(call->valueType()->typeScope && call->valueType()->typeScope->className == "A");
8900+
ASSERT_EQUALS(static_cast<int>(Reference::LValue), static_cast<int>(call->valueType()->reference));
8901+
}
8902+
8903+
void findFunction64()
8904+
{ // overloaded operator()
8905+
{
8906+
GET_SYMBOL_DB("struct A { void g(int); };\n" // overloads distinguished by argument count
8907+
"struct B { void h(int); };\n"
8908+
"template<class T>\n"
8909+
"struct C {\n"
8910+
" A& operator()();\n"
8911+
" B& operator()(int);\n"
8912+
"};\n"
8913+
"void f(C<int> c) {\n"
8914+
" c().g(1);\n"
8915+
" c(1).h(1);\n"
8916+
"}\n");
8917+
const Token* g = Token::findsimplematch(tokenizer.tokens(), "g ( 1 )");
8918+
ASSERT(g && g->function());
8919+
ASSERT_EQUALS(1, g->function()->tokenDef->linenr());
8920+
const Token* h = Token::findsimplematch(tokenizer.tokens(), "h ( 1 )");
8921+
ASSERT(h && h->function());
8922+
ASSERT_EQUALS(2, h->function()->tokenDef->linenr());
8923+
}
8924+
{
8925+
GET_SYMBOL_DB("struct A { void g(int); };\n" // overloads distinguished by constness of the object
8926+
"struct B { void h(int); };\n"
8927+
"template<class T>\n"
8928+
"struct C {\n"
8929+
" A& operator()();\n"
8930+
" B& operator()() const;\n"
8931+
"};\n"
8932+
"void f(C<int> c, const C<int>& k) {\n"
8933+
" c().g(1);\n"
8934+
" k().h(1);\n"
8935+
"}\n");
8936+
const Token* g = Token::findsimplematch(tokenizer.tokens(), "g ( 1 )");
8937+
ASSERT(g && g->function());
8938+
ASSERT_EQUALS(1, g->function()->tokenDef->linenr());
8939+
const Token* h = Token::findsimplematch(tokenizer.tokens(), "h ( 1 )");
8940+
ASSERT(h && h->function());
8941+
ASSERT_EQUALS(2, h->function()->tokenDef->linenr());
8942+
}
8943+
{
8944+
GET_SYMBOL_DB("struct A { void g(int); };\n" // overloads distinguished by argument type
8945+
"struct B { void h(int); };\n"
8946+
"template<class T>\n"
8947+
"struct C {\n"
8948+
" A& operator()(int);\n"
8949+
" B& operator()(double);\n"
8950+
"};\n"
8951+
"void f(C<int> c, int i, double d) {\n"
8952+
" c(i).g(1);\n"
8953+
" c(d).h(1);\n"
8954+
"}\n");
8955+
const Token* g = Token::findsimplematch(tokenizer.tokens(), "g ( 1 )");
8956+
ASSERT(g && g->function());
8957+
ASSERT_EQUALS(1, g->function()->tokenDef->linenr());
8958+
const Token* h = Token::findsimplematch(tokenizer.tokens(), "h ( 1 )");
8959+
ASSERT(h && h->function());
8960+
ASSERT_EQUALS(2, h->function()->tokenDef->linenr());
8961+
}
8962+
}
8963+
88788964
void findFunctionRef1() {
88798965
GET_SYMBOL_DB("struct X {\n"
88808966
" const std::vector<int> getInts() const & { return mInts; }\n"

0 commit comments

Comments
 (0)