Skip to content

Commit

Permalink
Fix #11862 FN constParameterPointer with increment
Browse files Browse the repository at this point in the history
  • Loading branch information
@chrchr-github
chrchr-github committed Aug 3, 2023
1 parent 78130cb commit 4c75b1b
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 33 deletions.
12 changes: 5 additions & 7 deletions lib/astutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2188,15 +2188,15 @@ T* getTokenArgumentFunctionImpl(T* tok, int& argn)
argn = -1;
{
T* parent = tok->astParent();
if (parent && parent->isUnaryOp("&"))
if (parent && (parent->isUnaryOp("&") || parent->isIncDecOp()))
parent = parent->astParent();
while (parent && parent->isCast())
parent = parent->astParent();
if (Token::Match(parent, "[+-]") && parent->valueType() && parent->valueType()->pointer)
parent = parent->astParent();

// passing variable to subfunction?
if (Token::Match(parent, "[[(,{]"))
if (Token::Match(parent, "[*[(,{]"))
;
else if (Token::simpleMatch(parent, ":")) {
while (Token::Match(parent, "[?:]"))
Expand Down Expand Up @@ -2455,7 +2455,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
(Token::simpleMatch(tok2->astParent(), ":") && Token::simpleMatch(tok2->astParent()->astParent(), "?")))
tok2 = tok2->astParent();

if (tok2->astParent() && tok2->astParent()->tokType() == Token::eIncDecOp)
if (indirect == 0 && tok2->astParent() && tok2->astParent()->tokType() == Token::eIncDecOp)
return true;

auto skipRedundantPtrOp = [](const Token* tok, const Token* parent) {
Expand Down Expand Up @@ -2576,9 +2576,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
}

const Token *parent = tok2->astParent();
while (Token::Match(parent, ".|::"))
parent = parent->astParent();
if (parent && parent->tokType() == Token::eIncDecOp)
if (parent && parent->tokType() == Token::eIncDecOp && (indirect == 0 || tok2 != tok))
return true;

// structured binding, nonconst reference variable in lhs
Expand Down Expand Up @@ -2615,7 +2613,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
if (indirect > 0) {
// check for `*(ptr + 1) = new_value` case
parent = tok2->astParent();
while (parent && parent->isArithmeticalOp() && parent->isBinaryOp()) {
while (parent && ((parent->isArithmeticalOp() && parent->isBinaryOp()) || parent->isIncDecOp())) {
parent = parent->astParent();
}
if (Token::simpleMatch(parent, "*")) {
Expand Down
13 changes: 9 additions & 4 deletions lib/checkother.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1587,9 +1587,10 @@ void CheckOther::checkConstPointer()
if (std::find(nonConstPointers.cbegin(), nonConstPointers.cend(), var) != nonConstPointers.cend())
continue;
pointers.emplace_back(var);
const Token* const parent = tok->astParent();
const Token* parent = tok->astParent();
enum Deref { NONE, DEREF, MEMBER } deref = NONE;
if (parent && parent->isUnaryOp("*"))
bool hasIncDec = false;
if (parent && (parent->isUnaryOp("*") || (hasIncDec = parent->isIncDecOp() && parent->astParent() && parent->astParent()->isUnaryOp("*"))))
deref = DEREF;
else if (Token::simpleMatch(parent, "[") && parent->astOperand1() == tok && tok != nameTok)
deref = DEREF;
Expand All @@ -1600,7 +1601,7 @@ void CheckOther::checkConstPointer()
else if (astIsRangeBasedForDecl(tok))
continue;
if (deref != NONE) {
const Token* const gparent = parent->astParent();
const Token* gparent = parent->astParent();
if (deref == MEMBER) {
if (!gparent)
continue;
Expand All @@ -1613,6 +1614,10 @@ void CheckOther::checkConstPointer()
}
if (Token::Match(gparent, "%cop%") && !gparent->isUnaryOp("&") && !gparent->isUnaryOp("*"))
continue;
if (hasIncDec) {
parent = gparent;
gparent = gparent ? gparent->astParent() : nullptr;
}
int argn = -1;
if (Token::simpleMatch(gparent, "return")) {
const Function* function = gparent->scope()->function;
Expand All @@ -1633,7 +1638,7 @@ void CheckOther::checkConstPointer()
continue;
else if (const Token* ftok = getTokenArgumentFunction(parent, argn)) {
bool inconclusive{};
if (!isVariableChangedByFunctionCall(ftok, vt->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive)
if (!isVariableChangedByFunctionCall(ftok->next(), vt->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive)
continue;
}
} else {
Expand Down
4 changes: 2 additions & 2 deletions lib/checkunusedvar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1691,8 +1691,8 @@ bool CheckUnusedVar::isFunctionWithoutSideEffects(const Function& func, const To
}
// check if global variable is changed
if (bodyVariable->isGlobal() || (pointersToGlobals.find(bodyVariable) != pointersToGlobals.end())) {
const int depth = 20;
if (isVariableChanged(bodyToken, depth, mSettings, mTokenizer->isCPP())) {
const int indirect = bodyVariable->isArray() ? bodyVariable->dimensions().size() : bodyVariable->isPointer();
if (isVariableChanged(bodyToken, indirect, mSettings, mTokenizer->isCPP())) {
return false;
}
// check if pointer to global variable assigned to another variable (another_var = &global_var)
Expand Down
2 changes: 1 addition & 1 deletion test/testother.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8120,7 +8120,7 @@ class TestOther : public TestFixture {
"void packed_object_info(struct object_info *oi) {\n"
" if (*oi->typep < 0);\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'oi' can be declared as pointer to const\n", errout.str());
}

void checkSuspiciousSemicolon1() {
Expand Down
32 changes: 13 additions & 19 deletions test/teststl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2819,27 +2819,20 @@ class TestStl : public TestFixture {
}

void eraseOnVector() {
check("void f(const std::vector<int>& m_ImplementationMap) {\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.begin();\n"
" m_ImplementationMap.erase(something(unknown));\n" // All iterators become invalidated when erasing from std::vector
" m_ImplementationMap.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());

check("void f(const std::vector<int>& m_ImplementationMap) {\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.begin();\n"
" m_ImplementationMap.erase(*aIt);\n" // All iterators become invalidated when erasing from std::vector
" m_ImplementationMap.erase(aIt);\n"
check("void f(std::vector<int>& v) {\n"
" std::vector<int>::iterator aIt = v.begin();\n"
" v.erase(something(unknown));\n" // All iterators become invalidated when erasing from std::vector
" v.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'v' that may be invalid.\n", errout.str());

check("void f(const std::vector<int>& m_ImplementationMap) {\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.begin();\n"
" std::vector<int>::iterator bIt = m_ImplementationMap.begin();\n"
" m_ImplementationMap.erase(*bIt);\n" // All iterators become invalidated when erasing from std::vector
" aIt = m_ImplementationMap.erase(aIt);\n"
check("void f(std::vector<int>& v) {\n"
" std::vector<int>::iterator aIt = v.begin();\n"
" std::vector<int>::iterator bIt = v.begin();\n"
" v.erase(bIt);\n" // All iterators become invalidated when erasing from std::vector
" aIt = v.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'v' that may be invalid.\n", errout.str());
}

void pushback1() {
Expand Down Expand Up @@ -3040,7 +3033,8 @@ class TestStl : public TestFixture {
" }\n"
"}");
ASSERT_EQUALS(
"[test.cpp:4] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:9]: (error, inconclusive) Using iterator to local container 'vec' that may be invalid.\n",
"[test.cpp:4] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:9]: (error, inconclusive) Using iterator to local container 'vec' that may be invalid.\n"
"[test.cpp:4] -> [test.cpp:9]: (warning) Either the condition 'it!=vec.end()' is redundant or there is possible dereference of an invalid iterator: it.\n",
errout.str());
}

Expand Down

0 comments on commit 4c75b1b

Please sign in to comment.