Skip to content

Commit

Permalink
Fix #12322 detect usage of partially-initialized object in constructor (
Browse files Browse the repository at this point in the history 
danmar#5851)
  • Loading branch information
@chrchr-github
chrchr-github committed Jan 9, 2024
1 parent 8903f0e commit c211665
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 17 deletions.
49 changes: 33 additions & 16 deletions lib/checkclass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2633,6 +2633,7 @@ namespace { // avoid one-definition-rule violation

const Variable *var;
const Token *tok;
std::vector<const Variable*> initArgs;
};
}

Expand Down Expand Up @@ -2663,25 +2664,38 @@ void CheckClass::initializerListOrder()
tok = tok->next();

// find all variable initializations in list
while (tok && tok != func->functionScope->bodyStart) {
for (; tok && tok != func->functionScope->bodyStart; tok = tok->next()) {
if (Token::Match(tok, "%name% (|{")) {
const Variable *var = scope->getVariable(tok->str());
if (var)
vars.emplace_back(var, tok);
else
continue;

if (Token::Match(tok->tokAt(2), "%name% =")) {
var = scope->getVariable(tok->strAt(2));

if (var)
vars.emplace_back(var, tok->tokAt(2));
const Token* const end = tok->next()->link();
for (; tok != end; tok = tok->next()) {
if (const Variable* argVar = scope->getVariable(tok->str())) {
if (var->isPointer() && (argVar->isArray() || Token::simpleMatch(tok->astParent(), "&")))
continue;
if (var->isReference())
continue;
if (Token::simpleMatch(tok->astParent(), "="))
continue;
vars.back().initArgs.emplace_back(argVar);
}
}
tok = tok->next()->link()->next();
} else
tok = tok->next();
}
}

// need at least 2 members to have out of order initialization
for (int j = 1; j < vars.size(); j++) {
for (int j = 0; j < vars.size(); j++) {
// check for use of uninitialized arguments
for (const auto& arg : vars[j].initArgs)
if (vars[j].var->index() < arg->index())
initializerListError(vars[j].tok, vars[j].var->nameToken(), scope->className, vars[j].var->name(), /*isArgument*/ true);

// need at least 2 members to have out of order initialization
if (j == 0)
continue;
// check for out of order initialization
if (vars[j].var->index() < vars[j - 1].var->index())
initializerListError(vars[j].tok,vars[j].var->nameToken(), scope->className, vars[j].var->name());
Expand All @@ -2692,15 +2706,18 @@ void CheckClass::initializerListOrder()
}
}

void CheckClass::initializerListError(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &varname)
void CheckClass::initializerListError(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &varname, bool isArgument)
{
std::list<const Token *> toks = { tok1, tok2 };
const std::string msg = isArgument ?
"Member variable '$symbol' uses an uninitialized argument due to the order of declarations." :
"Member variable '$symbol' is in the wrong place in the initializer list.";
reportError(toks, Severity::style, "initializerList",
"$symbol:" + classname + "::" + varname +"\n"
"Member variable '$symbol' is in the wrong place in the initializer list.\n"
"Member variable '$symbol' is in the wrong place in the initializer list. "
"$symbol:" + classname + "::" + varname + '\n' +
msg + '\n' +
msg + ' ' +
"Members are initialized in the order they are declared, not in the "
"order they are in the initializer list. Keeping the initializer list "
"order they are in the initializer list. Keeping the initializer list "
"in the same order that the members were declared prevents order dependent "
"initialization errors.", CWE398, Certainty::inconclusive);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/checkclass.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class CPPCHECKLIB CheckClass : public Check {
void operatorEqToSelfError(const Token *tok);
void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic);
void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic);
void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname);
void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname, bool isArgument = false);
void suggestInitializationList(const Token *tok, const std::string& varname);
void selfInitializationError(const Token* tok, const std::string& varname);
void pureVirtualFunctionCallInConstructorError(const Function * scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
Expand Down
52 changes: 52 additions & 0 deletions test/testclass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ class TestClass : public TestFixture {
TEST_CASE(qualifiedNameMember); // #10872

TEST_CASE(initializerListOrder);
TEST_CASE(initializerListArgument);
TEST_CASE(initializerListUsage);
TEST_CASE(selfInitialization);

Expand Down Expand Up @@ -7571,6 +7572,57 @@ class TestClass : public TestFixture {
"};");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (style, inconclusive) Member variable 'Fred::b' is in the wrong place in the initializer list.\n"
"[test.cpp:4] -> [test.cpp:2]: (style, inconclusive) Member variable 'Fred::a' is in the wrong place in the initializer list.\n", errout.str());

checkInitializerListOrder("struct S {\n"
" S() : b(a = 1) {}\n"
" int a, b;\n"
"};");
ASSERT_EQUALS("", errout.str());
}

void initializerListArgument() {
checkInitializerListOrder("struct A { A(); };\n" // #12322
"struct B { explicit B(const A* a); };\n"
"struct C {\n"
" C() : b(&a) {}\n"
" B b;\n"
" const A a;\n"
"};");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (style, inconclusive) Member variable 'C::b' uses an uninitialized argument due to the order of declarations.\n",
errout.str());

checkInitializerListOrder("struct S {\n"
" S(const std::string& f, std::string i, int b, int c) : a(0), b(b), c(c) {}\n"
" int a, b, c;\n"
"};");
ASSERT_EQUALS("", errout.str());

checkInitializerListOrder("struct S {\n"
" S() : p(a) {}\n"
" int* p;\n"
" int a[1];\n"
"};");
ASSERT_EQUALS("", errout.str());

checkInitializerListOrder("struct S {\n"
" S() : p(&i) {}\n"
" int* p;\n"
" int i;\n"
"};");
ASSERT_EQUALS("", errout.str());

checkInitializerListOrder("struct S {\n"
" S() : a(b = 1) {}\n"
" int a, b;\n"
"};");
ASSERT_EQUALS("", errout.str());

checkInitializerListOrder("struct S {\n"
" S() : r(i) {}\n"
" int& r;\n"
" int i{};\n"
"};");
ASSERT_EQUALS("", errout.str());
}

#define checkInitializationListUsage(code) checkInitializationListUsage_(code, __FILE__, __LINE__)
Expand Down

0 comments on commit c211665

Please sign in to comment.