walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Set the lower and upper bounds for variable that are only incremented or decremented (#5523) 

Also, changed `isExpressionChanged` and `isThisChanged` to return the
token that is being modified and renamed the functions to
`findExpressionChanged` and `findThisChanged`.
This commit is contained in:
Paul Fultz II 2023-10-19 11:42:52 -05:00 committed by GitHub
parent 7c3ae68e4d
commit f2c8153231
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 145 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
lib/astutils.cpp
View File

@ -1181,7 +1181,7 @@ static const Token * followVariableExpression(const Token * tok, bool cpp, const
return tok; return tok;
} else if (!precedes(startToken, endToken)) { } else if (!precedes(startToken, endToken)) {
return tok; return tok;
} else if (isExpressionChanged(varTok, startToken, endToken, nullptr, cpp)) { } else if (findExpressionChanged(varTok, startToken, endToken, nullptr, cpp)) {
return tok; return tok;
} }
return varTok; return varTok;
@ -1549,7 +1549,7 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
const Token *start = refTok1, *end = refTok2; const Token *start = refTok1, *end = refTok2;
if (!precedes(start, end)) if (!precedes(start, end))
std::swap(start, end); std::swap(start, end);
if (isExpressionChanged(start, start, end, nullptr, cpp)) if (findExpressionChanged(start, start, end, nullptr, cpp))
return false; return false;
} }
return isSameExpression(cpp, macro, refTok1, refTok2, library, pure, followVar, errors); return isSameExpression(cpp, macro, refTok1, refTok2, library, pure, followVar, errors);
@ -2825,7 +2825,7 @@ bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp,
if (next) if (next)
start = next; start = next;
} }
return isExpressionChanged(var->nameToken(), start->next(), var->scope()->bodyEnd, settings, cpp, depth); return findExpressionChanged(var->nameToken(), start->next(), var->scope()->bodyEnd, settings, cpp, depth);
} }
bool isVariablesChanged(const Token* start, bool isVariablesChanged(const Token* start,
@ -2871,21 +2871,21 @@ bool isThisChanged(const Token* tok, int indirect, const Settings* settings, boo
return false; return false;
} }
bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp) const Token* findThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp)
{ {
if (!precedes(start, end)) if (!precedes(start, end))
return false; return nullptr;
for (const Token* tok = start; tok != end; tok = tok->next()) { for (const Token* tok = start; tok != end; tok = tok->next()) {
if (!exprDependsOnThis(tok)) if (!exprDependsOnThis(tok))
continue; continue;
if (isThisChanged(tok, indirect, settings, cpp)) if (isThisChanged(tok, indirect, settings, cpp))
return true; return tok;
} }
return false; return nullptr;
} }
template<class Find> template<class Find>
bool isExpressionChangedImpl(const Token* expr, const Token* findExpressionChangedImpl(const Token* expr,
const Token* start, const Token* start,
const Token* end, const Token* end,
const Settings* settings, const Settings* settings,
@ -2894,11 +2894,13 @@ bool isExpressionChangedImpl(const Token* expr,
Find find) Find find)
{ {
if (depth < 0) if (depth < 0)
return true; return start;
if (!precedes(start, end)) if (!precedes(start, end))
return false; return nullptr;
const Token* result = findAstNode(expr, [&](const Token* tok) { const Token* result = nullptr;
if (exprDependsOnThis(tok) && isThisChanged(start, end, false, settings, cpp)) { findAstNode(expr, [&](const Token* tok) {
if (exprDependsOnThis(tok)) {
result = findThisChanged(start, end, false, settings, cpp);
return true; return true;
} }
bool global = false; bool global = false;
@ -2912,7 +2914,7 @@ bool isExpressionChangedImpl(const Token* expr,
} }
if (tok->exprId() > 0) { if (tok->exprId() > 0) {
const Token* result = find(start, end, [&](const Token* tok2) { const Token* modifedTok = find(start, end, [&](const Token* tok2) {
int indirect = 0; int indirect = 0;
if (const ValueType* vt = tok->valueType()) { if (const ValueType* vt = tok->valueType()) {
indirect = vt->pointer; indirect = vt->pointer;
@ -2924,9 +2926,11 @@ bool isExpressionChangedImpl(const Token* expr,
return true; return true;
return false; return false;
}); });
if (result) if (modifedTok) {
result = modifedTok;
return true; return true;
} }
}
return false; return false;
}); });
return result; return result;
@ -2954,12 +2958,17 @@ struct ExpressionChangedSkipDeadCode {
} }
}; };
bool isExpressionChanged(const Token* expr, const Token* start, const Token* end, const Settings* settings, bool cpp, int depth) const Token* findExpressionChanged(const Token* expr,
const Token* start,
const Token* end,
const Settings* settings,
bool cpp,
int depth)
{ {
return isExpressionChangedImpl(expr, start, end, settings, cpp, depth, ExpressionChangedSimpleFind{}); return findExpressionChangedImpl(expr, start, end, settings, cpp, depth, ExpressionChangedSimpleFind{});
} }
bool isExpressionChangedSkipDeadCode(const Token* expr, const Token* findExpressionChangedSkipDeadCode(const Token* expr,
const Token* start, const Token* start,
const Token* end, const Token* end,
const Settings* settings, const Settings* settings,
@ -2967,7 +2976,7 @@ bool isExpressionChangedSkipDeadCode(const Token* expr,
const std::function<std::vector<MathLib::bigint>(const Token* tok)>& evaluate, const std::function<std::vector<MathLib::bigint>(const Token* tok)>& evaluate,
int depth) int depth)
{ {
return isExpressionChangedImpl( return findExpressionChangedImpl(
expr, start, end, settings, cpp, depth, ExpressionChangedSkipDeadCode{&settings->library, evaluate}); expr, start, end, settings, cpp, depth, ExpressionChangedSkipDeadCode{&settings->library, evaluate});
} }

6
lib/astutils.h
View File

@ -335,19 +335,19 @@ bool isVariablesChanged(const Token* start,
bool cpp); bool cpp);
bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp); bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp);
bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp); const Token* findThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp);
const Token* findVariableChanged(const Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20); const Token* findVariableChanged(const Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20); Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
CPPCHECKLIB bool isExpressionChanged(const Token* expr, CPPCHECKLIB const Token* findExpressionChanged(const Token* expr,
const Token* start, const Token* start,
const Token* end, const Token* end,
const Settings* settings, const Settings* settings,
bool cpp, bool cpp,
int depth = 20); int depth = 20);
bool isExpressionChangedSkipDeadCode(const Token* expr, const Token* findExpressionChangedSkipDeadCode(const Token* expr,
const Token* start, const Token* start,
const Token* end, const Token* end,
const Settings* settings, const Settings* settings,

12
lib/checkcondition.cpp
View File

@ -505,7 +505,7 @@ void CheckCondition::duplicateCondition()
continue; continue;
ErrorPath errorPath; ErrorPath errorPath;
if (!isExpressionChanged(cond1, scope.classDef->next(), cond2, mSettings, mTokenizer->isCPP()) && if (!findExpressionChanged(cond1, scope.classDef->next(), cond2, mSettings, mTokenizer->isCPP()) &&
isSameExpression(mTokenizer->isCPP(), true, cond1, cond2, mSettings->library, true, true, &errorPath)) isSameExpression(mTokenizer->isCPP(), true, cond1, cond2, mSettings->library, true, true, &errorPath))
duplicateConditionError(cond1, cond2, errorPath); duplicateConditionError(cond1, cond2, errorPath);
} }
@ -554,10 +554,12 @@ void CheckCondition::multiCondition()
if (tok2->astOperand2()) { if (tok2->astOperand2()) {
ErrorPath errorPath; ErrorPath errorPath;
if (isOverlappingCond(cond1, tok2->astOperand2(), true) && !isExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP())) if (isOverlappingCond(cond1, tok2->astOperand2(), true) &&
!findExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP()))
overlappingElseIfConditionError(tok2->astOperand2(), cond1->linenr()); overlappingElseIfConditionError(tok2->astOperand2(), cond1->linenr());
else if (isOppositeCond(true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath) && else if (isOppositeCond(
!isExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP())) true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath) &&
!findExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP()))
oppositeElseIfConditionError(cond1, tok2->astOperand2(), errorPath); oppositeElseIfConditionError(cond1, tok2->astOperand2(), errorPath);
} }
} }
@ -709,7 +711,7 @@ void CheckCondition::multiCondition2()
const Token * condStartToken = tok->str() == "if" ? tok->next() : tok; const Token * condStartToken = tok->str() == "if" ? tok->next() : tok;
const Token * condEndToken = tok->str() == "if" ? condStartToken->link() : Token::findsimplematch(condStartToken, ";"); const Token * condEndToken = tok->str() == "if" ? condStartToken->link() : Token::findsimplematch(condStartToken, ";");
// Does condition modify tracked variables? // Does condition modify tracked variables?
if (isExpressionChanged(cond1, condStartToken, condEndToken, mSettings, mTokenizer->isCPP())) if (findExpressionChanged(cond1, condStartToken, condEndToken, mSettings, mTokenizer->isCPP()))
break; break;
// Condition.. // Condition..

5
lib/checkother.cpp
View File

@ -889,7 +889,7 @@ static bool isSimpleExpr(const Token* tok, const Variable* var, const Settings*
((ftok->function() && ftok->function()->isConst()) || settings->library.isFunctionConst(ftok->str(), /*pure*/ true))) ((ftok->function() && ftok->function()->isConst()) || settings->library.isFunctionConst(ftok->str(), /*pure*/ true)))
needsCheck = true; needsCheck = true;
} }
return (needsCheck && !isExpressionChanged(tok, tok->astParent(), var->scope()->bodyEnd, settings, true)); return (needsCheck && !findExpressionChanged(tok, tok->astParent(), var->scope()->bodyEnd, settings, true));
} }
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
@ -2596,7 +2596,8 @@ void CheckOther::checkDuplicateExpression()
&errorPath)) { &errorPath)) {
if (isWithoutSideEffects(cpp, tok->astOperand1())) { if (isWithoutSideEffects(cpp, tok->astOperand1())) {
const Token* loopTok = isInLoopCondition(tok); const Token* loopTok = isInLoopCondition(tok);
if (!loopTok || !isExpressionChanged(tok, tok, loopTok->link()->next()->link(), mSettings, cpp)) { if (!loopTok ||
!findExpressionChanged(tok, tok, loopTok->link()->next()->link(), mSettings, cpp)) {
const bool isEnum = tok->scope()->type == Scope::eEnum; const bool isEnum = tok->scope()->type == Scope::eEnum;
const bool assignment = !isEnum && tok->str() == "="; const bool assignment = !isEnum && tok->str() == "=";
if (assignment && warningEnabled) if (assignment && warningEnabled)

5
lib/forwardanalyzer.cpp
View File

@ -387,9 +387,10 @@ struct ForwardTraversal {
if (stepTok) { if (stepTok) {
std::pair<const Token*, const Token*> exprToks = stepTok->findExpressionStartEndTokens(); std::pair<const Token*, const Token*> exprToks = stepTok->findExpressionStartEndTokens();
if (exprToks.first != nullptr && exprToks.second != nullptr) if (exprToks.first != nullptr && exprToks.second != nullptr)
stepChangesCond |= isExpressionChanged(condTok, exprToks.first, exprToks.second->next(), &settings, true); stepChangesCond |=
findExpressionChanged(condTok, exprToks.first, exprToks.second->next(), &settings, true) != nullptr;
} }
const bool bodyChangesCond = isExpressionChanged(condTok, endBlock->link(), endBlock, &settings, true); const bool bodyChangesCond = findExpressionChanged(condTok, endBlock->link(), endBlock, &settings, true);
// Check for mutation in the condition // Check for mutation in the condition
const bool condChanged = const bool condChanged =
nullptr != findAstNode(condTok, [&](const Token* tok) { nullptr != findAstNode(condTok, [&](const Token* tok) {

6
lib/programmemory.cpp
View File

@ -289,7 +289,7 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke
return; return;
if (!truevalue.isIntValue()) if (!truevalue.isIntValue())
return; return;
if (endTok && isExpressionChanged(vartok, tok->next(), endTok, settings, true)) if (endTok && findExpressionChanged(vartok, tok->next(), endTok, settings, true))
return; return;
const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then); const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then);
const ValueFlow::Value& v = then ? truevalue : falsevalue; const ValueFlow::Value& v = then ? truevalue : falsevalue;
@ -317,7 +317,7 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke
pm.setIntValue(tok, 0, then); pm.setIntValue(tok, 0, then);
} }
} else if (tok->exprId() > 0) { } else if (tok->exprId() > 0) {
if (endTok && isExpressionChanged(tok, tok->next(), endTok, settings, true)) if (endTok && findExpressionChanged(tok, tok->next(), endTok, settings, true))
return; return;
pm.setIntValue(tok, 0, then); pm.setIntValue(tok, 0, then);
const Token* containerTok = settings->library.getContainerFromYield(tok, Library::Container::Yield::EMPTY); const Token* containerTok = settings->library.getContainerFromYield(tok, Library::Container::Yield::EMPTY);
@ -501,7 +501,7 @@ void ProgramMemoryState::removeModifiedVars(const Token* tok)
state.erase_if([&](const ExprIdToken& e) { state.erase_if([&](const ExprIdToken& e) {
const Token* start = origins[e.getExpressionId()]; const Token* start = origins[e.getExpressionId()];
const Token* expr = e.tok; const Token* expr = e.tok;
if (!expr || isExpressionChangedSkipDeadCode(expr, start, tok, settings, true, eval)) { if (!expr || findExpressionChangedSkipDeadCode(expr, start, tok, settings, true, eval)) {
origins.erase(e.getExpressionId()); origins.erase(e.getExpressionId());
return true; return true;
} }

2
lib/tokenize.cpp
View File

@ -8929,8 +8929,8 @@ void Tokenizer::simplifyDeclspec()
{ {
for (Token *tok = list.front(); tok; tok = tok->next()) { for (Token *tok = list.front(); tok; tok = tok->next()) {
while (isAttribute(tok, false)) { while (isAttribute(tok, false)) {
Token *functok = getAttributeFuncTok(tok, false);
if (Token::Match(tok->tokAt(2), "noreturn|nothrow|dllexport")) { if (Token::Match(tok->tokAt(2), "noreturn|nothrow|dllexport")) {
Token *functok = getAttributeFuncTok(tok, false);
if (functok) { if (functok) {
if (tok->strAt(2) == "noreturn") if (tok->strAt(2) == "noreturn")
functok->isAttributeNoreturn(true); functok->isAttributeNoreturn(true);

62
lib/valueflow.cpp
View File

@ -5667,6 +5667,46 @@ static void valueFlowForwardConst(Token* start,
valueFlowForwardConst(start, end, var, values, settings, 0); valueFlowForwardConst(start, end, var, values, settings, 0);
} }
static ValueFlow::Value::Bound findVarBound(const Variable* var,
const Token* start,
const Token* end,
const Settings* settings)
{
ValueFlow::Value::Bound result = ValueFlow::Value::Bound::Point;
const Token* next = start;
while ((next = findExpressionChangedSkipDeadCode(
var->nameToken(), next->next(), end, settings, true, &evaluateKnownValues))) {
ValueFlow::Value::Bound b = ValueFlow::Value::Bound::Point;
if (next->varId() != var->declarationId())
return ValueFlow::Value::Bound::Point;
if (Token::simpleMatch(next->astParent(), "++"))
b = ValueFlow::Value::Bound::Lower;
else if (Token::simpleMatch(next->astParent(), "--"))
b = ValueFlow::Value::Bound::Upper;
else
return ValueFlow::Value::Bound::Point;
if (result == ValueFlow::Value::Bound::Point)
result = b;
else if (result != b)
return ValueFlow::Value::Bound::Point;
}
return result;
}
static bool isInitialVarAssign(const Token* tok)
{
if (!tok)
return false;
if (!tok->variable())
return false;
if (tok->variable()->nameToken() == tok)
return true;
const Token* prev = tok->tokAt(2);
if (!Token::Match(prev, "%var% ; %var%"))
return false;
return tok->varId() == prev->varId() && tok->variable()->nameToken() == prev;
}
static void valueFlowForwardAssign(Token* const tok, static void valueFlowForwardAssign(Token* const tok,
const Token* expr, const Token* expr,
std::vector<const Variable*> vars, std::vector<const Variable*> vars,
@ -5756,6 +5796,24 @@ static void valueFlowForwardAssign(Token* const tok,
constValues.splice(constValues.end(), values, it, values.end()); constValues.splice(constValues.end(), values, it, values.end());
valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), constValues, settings); valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), constValues, settings);
} }
if (isInitialVarAssign(expr)) {
// Check if variable is only incremented or decremented
ValueFlow::Value::Bound b = findVarBound(expr->variable(), nextExpression, endOfVarScope, settings);
if (b != ValueFlow::Value::Bound::Point) {
auto knownValueIt = std::find_if(values.begin(), values.end(), [](const ValueFlow::Value& value) {
if (!value.isKnown())
return false;
return value.isIntValue();
});
if (knownValueIt != values.end()) {
ValueFlow::Value value = *knownValueIt;
value.bound = b;
value.invertRange();
value.setImpossible();
valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), {value}, settings);
}
}
}
valueFlowForward(nextExpression, endOfVarScope, expr, values, tokenlist, settings); valueFlowForward(nextExpression, endOfVarScope, expr, values, tokenlist, settings);
} }
@ -6254,7 +6312,7 @@ struct ConditionHandler {
// Variable changed in 3rd for-expression // Variable changed in 3rd for-expression
if (Token::simpleMatch(top->previous(), "for (")) { if (Token::simpleMatch(top->previous(), "for (")) {
if (top->astOperand2() && top->astOperand2()->astOperand2() && if (top->astOperand2() && top->astOperand2()->astOperand2() &&
isExpressionChanged( findExpressionChanged(
cond.vartok, top->astOperand2()->astOperand2(), top->link(), settings, tokenlist.isCPP())) { cond.vartok, top->astOperand2()->astOperand2(), top->link(), settings, tokenlist.isCPP())) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, bailout(tokenlist,
@ -6270,7 +6328,7 @@ struct ConditionHandler {
const Token* const block = top->link()->next(); const Token* const block = top->link()->next();
const Token* const end = block->link(); const Token* const end = block->link();
if (isExpressionChanged(cond.vartok, start, end, settings, tokenlist.isCPP())) { if (findExpressionChanged(cond.vartok, start, end, settings, tokenlist.isCPP())) {
// If its reassigned in loop then analyze from the end // If its reassigned in loop then analyze from the end
if (!Token::Match(tok, "%assign%|++|--") && if (!Token::Match(tok, "%assign%|++|--") &&
findExpression(cond.vartok->exprId(), start, end, [&](const Token* tok2) { findExpression(cond.vartok->exprId(), start, end, [&](const Token* tok2) {

2
test/testastutils.cpp
View File

@ -397,7 +397,7 @@ private:
const Token* const start = Token::findsimplematch(tokenizer.tokens(), startPattern, strlen(startPattern)); const Token* const start = Token::findsimplematch(tokenizer.tokens(), startPattern, strlen(startPattern));
const Token* const end = Token::findsimplematch(start, endPattern, strlen(endPattern)); const Token* const end = Token::findsimplematch(start, endPattern, strlen(endPattern));
const Token* const expr = Token::findsimplematch(tokenizer.tokens(), var, strlen(var)); const Token* const expr = Token::findsimplematch(tokenizer.tokens(), var, strlen(var));
return (isExpressionChanged)(expr, start, end, &settings, true); return (findExpressionChanged)(expr, start, end, &settings, true);
} }
void isExpressionChangedTest() void isExpressionChangedTest()

14
test/testvalueflow.cpp
View File

@ -156,6 +156,7 @@ private:
TEST_CASE(valueFlowSymbolicStrlen); TEST_CASE(valueFlowSymbolicStrlen);
TEST_CASE(valueFlowSmartPointer); TEST_CASE(valueFlowSmartPointer);
TEST_CASE(valueFlowImpossibleMinMax); TEST_CASE(valueFlowImpossibleMinMax);
TEST_CASE(valueFlowImpossibleIncDec);
TEST_CASE(valueFlowImpossibleUnknownConstant); TEST_CASE(valueFlowImpossibleUnknownConstant);
TEST_CASE(valueFlowContainerEqual); TEST_CASE(valueFlowContainerEqual);
@ -8160,6 +8161,19 @@ private:
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1)); ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1));
} }
void valueFlowImpossibleIncDec()
{
const char* code;
code = "int f() {\n"
" for(int i = 0; i < 5; i++) {\n"
" int x = i;\n"
" return x;\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXImpossible(code, 4U, -1));
}
void valueFlowImpossibleUnknownConstant() void valueFlowImpossibleUnknownConstant()
{ {
const char* code; const char* code;