Skip to content

Commit f2c8153

Browse files
pfultz2pfultz2
authored
Set the lower and upper bounds for variable that are only incremented or decremented (cppcheck-opensource#5523)
Also, changed `isExpressionChanged` and `isThisChanged` to return the token that is being modified and renamed the functions to `findExpressionChanged` and `findThisChanged`.
1 parent 7c3ae68 commit f2c8153

10 files changed

Lines changed: 146 additions & 61 deletions

‎lib/astutils.cpp‎

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,7 +1181,7 @@ static const Token * followVariableExpression(const Token * tok, bool cpp, const
11811181
return tok;
11821182
} else if (!precedes(startToken, endToken)) {
11831183
return tok;
1184-
} else if (isExpressionChanged(varTok, startToken, endToken, nullptr, cpp)) {
1184+
} else if (findExpressionChanged(varTok, startToken, endToken, nullptr, cpp)) {
11851185
return tok;
11861186
}
11871187
return varTok;
@@ -1549,7 +1549,7 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
15491549
const Token *start = refTok1, *end = refTok2;
15501550
if (!precedes(start, end))
15511551
std::swap(start, end);
1552-
if (isExpressionChanged(start, start, end, nullptr, cpp))
1552+
if (findExpressionChanged(start, start, end, nullptr, cpp))
15531553
return false;
15541554
}
15551555
return isSameExpression(cpp, macro, refTok1, refTok2, library, pure, followVar, errors);
@@ -2825,7 +2825,7 @@ bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp,
28252825
if (next)
28262826
start = next;
28272827
}
2828-
return isExpressionChanged(var->nameToken(), start->next(), var->scope()->bodyEnd, settings, cpp, depth);
2828+
return findExpressionChanged(var->nameToken(), start->next(), var->scope()->bodyEnd, settings, cpp, depth);
28292829
}
28302830

28312831
bool isVariablesChanged(const Token* start,
@@ -2871,34 +2871,36 @@ bool isThisChanged(const Token* tok, int indirect, const Settings* settings, boo
28712871
return false;
28722872
}
28732873

2874-
bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp)
2874+
const Token* findThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp)
28752875
{
28762876
if (!precedes(start, end))
2877-
return false;
2877+
return nullptr;
28782878
for (const Token* tok = start; tok != end; tok = tok->next()) {
28792879
if (!exprDependsOnThis(tok))
28802880
continue;
28812881
if (isThisChanged(tok, indirect, settings, cpp))
2882-
return true;
2882+
return tok;
28832883
}
2884-
return false;
2884+
return nullptr;
28852885
}
28862886

28872887
template<class Find>
2888-
bool isExpressionChangedImpl(const Token* expr,
2889-
const Token* start,
2890-
const Token* end,
2891-
const Settings* settings,
2892-
bool cpp,
2893-
int depth,
2894-
Find find)
2888+
const Token* findExpressionChangedImpl(const Token* expr,
2889+
const Token* start,
2890+
const Token* end,
2891+
const Settings* settings,
2892+
bool cpp,
2893+
int depth,
2894+
Find find)
28952895
{
28962896
if (depth < 0)
2897-
return true;
2897+
return start;
28982898
if (!precedes(start, end))
2899-
return false;
2900-
const Token* result = findAstNode(expr, [&](const Token* tok) {
2901-
if (exprDependsOnThis(tok) && isThisChanged(start, end, false, settings, cpp)) {
2899+
return nullptr;
2900+
const Token* result = nullptr;
2901+
findAstNode(expr, [&](const Token* tok) {
2902+
if (exprDependsOnThis(tok)) {
2903+
result = findThisChanged(start, end, false, settings, cpp);
29022904
return true;
29032905
}
29042906
bool global = false;
@@ -2912,7 +2914,7 @@ bool isExpressionChangedImpl(const Token* expr,
29122914
}
29132915

29142916
if (tok->exprId() > 0) {
2915-
const Token* result = find(start, end, [&](const Token* tok2) {
2917+
const Token* modifedTok = find(start, end, [&](const Token* tok2) {
29162918
int indirect = 0;
29172919
if (const ValueType* vt = tok->valueType()) {
29182920
indirect = vt->pointer;
@@ -2924,8 +2926,10 @@ bool isExpressionChangedImpl(const Token* expr,
29242926
return true;
29252927
return false;
29262928
});
2927-
if (result)
2929+
if (modifedTok) {
2930+
result = modifedTok;
29282931
return true;
2932+
}
29292933
}
29302934
return false;
29312935
});
@@ -2954,20 +2958,25 @@ struct ExpressionChangedSkipDeadCode {
29542958
}
29552959
};
29562960

2957-
bool isExpressionChanged(const Token* expr, const Token* start, const Token* end, const Settings* settings, bool cpp, int depth)
2961+
const Token* findExpressionChanged(const Token* expr,
2962+
const Token* start,
2963+
const Token* end,
2964+
const Settings* settings,
2965+
bool cpp,
2966+
int depth)
29582967
{
2959-
return isExpressionChangedImpl(expr, start, end, settings, cpp, depth, ExpressionChangedSimpleFind{});
2968+
return findExpressionChangedImpl(expr, start, end, settings, cpp, depth, ExpressionChangedSimpleFind{});
29602969
}
29612970

2962-
bool isExpressionChangedSkipDeadCode(const Token* expr,
2963-
const Token* start,
2964-
const Token* end,
2965-
const Settings* settings,
2966-
bool cpp,
2967-
const std::function<std::vector<MathLib::bigint>(const Token* tok)>& evaluate,
2968-
int depth)
2971+
const Token* findExpressionChangedSkipDeadCode(const Token* expr,
2972+
const Token* start,
2973+
const Token* end,
2974+
const Settings* settings,
2975+
bool cpp,
2976+
const std::function<std::vector<MathLib::bigint>(const Token* tok)>& evaluate,
2977+
int depth)
29692978
{
2970-
return isExpressionChangedImpl(
2979+
return findExpressionChangedImpl(
29712980
expr, start, end, settings, cpp, depth, ExpressionChangedSkipDeadCode{&settings->library, evaluate});
29722981
}
29732982

‎lib/astutils.h‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -335,25 +335,25 @@ bool isVariablesChanged(const Token* start,
335335
bool cpp);
336336

337337
bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp);
338-
bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp);
338+
const Token* findThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp);
339339

340340
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);
341341
Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
342342

343-
CPPCHECKLIB bool isExpressionChanged(const Token* expr,
344-
const Token* start,
345-
const Token* end,
346-
const Settings* settings,
347-
bool cpp,
348-
int depth = 20);
349-
350-
bool isExpressionChangedSkipDeadCode(const Token* expr,
351-
const Token* start,
352-
const Token* end,
353-
const Settings* settings,
354-
bool cpp,
355-
const std::function<std::vector<MathLib::bigint>(const Token* tok)>& evaluate,
356-
int depth = 20);
343+
CPPCHECKLIB const Token* findExpressionChanged(const Token* expr,
344+
const Token* start,
345+
const Token* end,
346+
const Settings* settings,
347+
bool cpp,
348+
int depth = 20);
349+
350+
const Token* findExpressionChangedSkipDeadCode(const Token* expr,
351+
const Token* start,
352+
const Token* end,
353+
const Settings* settings,
354+
bool cpp,
355+
const std::function<std::vector<MathLib::bigint>(const Token* tok)>& evaluate,
356+
int depth = 20);
357357

358358
bool isExpressionChangedAt(const Token* expr,
359359
const Token* tok,

‎lib/checkcondition.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ void CheckCondition::duplicateCondition()
505505
continue;
506506

507507
ErrorPath errorPath;
508-
if (!isExpressionChanged(cond1, scope.classDef->next(), cond2, mSettings, mTokenizer->isCPP()) &&
508+
if (!findExpressionChanged(cond1, scope.classDef->next(), cond2, mSettings, mTokenizer->isCPP()) &&
509509
isSameExpression(mTokenizer->isCPP(), true, cond1, cond2, mSettings->library, true, true, &errorPath))
510510
duplicateConditionError(cond1, cond2, errorPath);
511511
}
@@ -554,10 +554,12 @@ void CheckCondition::multiCondition()
554554

555555
if (tok2->astOperand2()) {
556556
ErrorPath errorPath;
557-
if (isOverlappingCond(cond1, tok2->astOperand2(), true) && !isExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP()))
557+
if (isOverlappingCond(cond1, tok2->astOperand2(), true) &&
558+
!findExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP()))
558559
overlappingElseIfConditionError(tok2->astOperand2(), cond1->linenr());
559-
else if (isOppositeCond(true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath) &&
560-
!isExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP()))
560+
else if (isOppositeCond(
561+
true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath) &&
562+
!findExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP()))
561563
oppositeElseIfConditionError(cond1, tok2->astOperand2(), errorPath);
562564
}
563565
}
@@ -709,7 +711,7 @@ void CheckCondition::multiCondition2()
709711
const Token * condStartToken = tok->str() == "if" ? tok->next() : tok;
710712
const Token * condEndToken = tok->str() == "if" ? condStartToken->link() : Token::findsimplematch(condStartToken, ";");
711713
// Does condition modify tracked variables?
712-
if (isExpressionChanged(cond1, condStartToken, condEndToken, mSettings, mTokenizer->isCPP()))
714+
if (findExpressionChanged(cond1, condStartToken, condEndToken, mSettings, mTokenizer->isCPP()))
713715
break;
714716

715717
// Condition..

‎lib/checkother.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,7 @@ static bool isSimpleExpr(const Token* tok, const Variable* var, const Settings*
889889
((ftok->function() && ftok->function()->isConst()) || settings->library.isFunctionConst(ftok->str(), /*pure*/ true)))
890890
needsCheck = true;
891891
}
892-
return (needsCheck && !isExpressionChanged(tok, tok->astParent(), var->scope()->bodyEnd, settings, true));
892+
return (needsCheck && !findExpressionChanged(tok, tok->astParent(), var->scope()->bodyEnd, settings, true));
893893
}
894894

895895
//---------------------------------------------------------------------------
@@ -2596,7 +2596,8 @@ void CheckOther::checkDuplicateExpression()
25962596
&errorPath)) {
25972597
if (isWithoutSideEffects(cpp, tok->astOperand1())) {
25982598
const Token* loopTok = isInLoopCondition(tok);
2599-
if (!loopTok || !isExpressionChanged(tok, tok, loopTok->link()->next()->link(), mSettings, cpp)) {
2599+
if (!loopTok ||
2600+
!findExpressionChanged(tok, tok, loopTok->link()->next()->link(), mSettings, cpp)) {
26002601
const bool isEnum = tok->scope()->type == Scope::eEnum;
26012602
const bool assignment = !isEnum && tok->str() == "=";
26022603
if (assignment && warningEnabled)

‎lib/forwardanalyzer.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,9 +387,10 @@ struct ForwardTraversal {
387387
if (stepTok) {
388388
std::pair<const Token*, const Token*> exprToks = stepTok->findExpressionStartEndTokens();
389389
if (exprToks.first != nullptr && exprToks.second != nullptr)
390-
stepChangesCond |= isExpressionChanged(condTok, exprToks.first, exprToks.second->next(), &settings, true);
390+
stepChangesCond |=
391+
findExpressionChanged(condTok, exprToks.first, exprToks.second->next(), &settings, true) != nullptr;
391392
}
392-
const bool bodyChangesCond = isExpressionChanged(condTok, endBlock->link(), endBlock, &settings, true);
393+
const bool bodyChangesCond = findExpressionChanged(condTok, endBlock->link(), endBlock, &settings, true);
393394
// Check for mutation in the condition
394395
const bool condChanged =
395396
nullptr != findAstNode(condTok, [&](const Token* tok) {

‎lib/programmemory.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke
289289
return;
290290
if (!truevalue.isIntValue())
291291
return;
292-
if (endTok && isExpressionChanged(vartok, tok->next(), endTok, settings, true))
292+
if (endTok && findExpressionChanged(vartok, tok->next(), endTok, settings, true))
293293
return;
294294
const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then);
295295
const ValueFlow::Value& v = then ? truevalue : falsevalue;
@@ -317,7 +317,7 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke
317317
pm.setIntValue(tok, 0, then);
318318
}
319319
} else if (tok->exprId() > 0) {
320-
if (endTok && isExpressionChanged(tok, tok->next(), endTok, settings, true))
320+
if (endTok && findExpressionChanged(tok, tok->next(), endTok, settings, true))
321321
return;
322322
pm.setIntValue(tok, 0, then);
323323
const Token* containerTok = settings->library.getContainerFromYield(tok, Library::Container::Yield::EMPTY);
@@ -501,7 +501,7 @@ void ProgramMemoryState::removeModifiedVars(const Token* tok)
501501
state.erase_if([&](const ExprIdToken& e) {
502502
const Token* start = origins[e.getExpressionId()];
503503
const Token* expr = e.tok;
504-
if (!expr || isExpressionChangedSkipDeadCode(expr, start, tok, settings, true, eval)) {
504+
if (!expr || findExpressionChangedSkipDeadCode(expr, start, tok, settings, true, eval)) {
505505
origins.erase(e.getExpressionId());
506506
return true;
507507
}

‎lib/tokenize.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8929,8 +8929,8 @@ void Tokenizer::simplifyDeclspec()
89298929
{
89308930
for (Token *tok = list.front(); tok; tok = tok->next()) {
89318931
while (isAttribute(tok, false)) {
8932-
Token *functok = getAttributeFuncTok(tok, false);
89338932
if (Token::Match(tok->tokAt(2), "noreturn|nothrow|dllexport")) {
8933+
Token *functok = getAttributeFuncTok(tok, false);
89348934
if (functok) {
89358935
if (tok->strAt(2) == "noreturn")
89368936
functok->isAttributeNoreturn(true);

‎lib/valueflow.cpp‎

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5667,6 +5667,46 @@ static void valueFlowForwardConst(Token* start,
56675667
valueFlowForwardConst(start, end, var, values, settings, 0);
56685668
}
56695669

5670+
static ValueFlow::Value::Bound findVarBound(const Variable* var,
5671+
const Token* start,
5672+
const Token* end,
5673+
const Settings* settings)
5674+
{
5675+
ValueFlow::Value::Bound result = ValueFlow::Value::Bound::Point;
5676+
const Token* next = start;
5677+
while ((next = findExpressionChangedSkipDeadCode(
5678+
var->nameToken(), next->next(), end, settings, true, &evaluateKnownValues))) {
5679+
ValueFlow::Value::Bound b = ValueFlow::Value::Bound::Point;
5680+
if (next->varId() != var->declarationId())
5681+
return ValueFlow::Value::Bound::Point;
5682+
if (Token::simpleMatch(next->astParent(), "++"))
5683+
b = ValueFlow::Value::Bound::Lower;
5684+
else if (Token::simpleMatch(next->astParent(), "--"))
5685+
b = ValueFlow::Value::Bound::Upper;
5686+
else
5687+
return ValueFlow::Value::Bound::Point;
5688+
if (result == ValueFlow::Value::Bound::Point)
5689+
result = b;
5690+
else if (result != b)
5691+
return ValueFlow::Value::Bound::Point;
5692+
}
5693+
return result;
5694+
}
5695+
5696+
static bool isInitialVarAssign(const Token* tok)
5697+
{
5698+
if (!tok)
5699+
return false;
5700+
if (!tok->variable())
5701+
return false;
5702+
if (tok->variable()->nameToken() == tok)
5703+
return true;
5704+
const Token* prev = tok->tokAt(2);
5705+
if (!Token::Match(prev, "%var% ; %var%"))
5706+
return false;
5707+
return tok->varId() == prev->varId() && tok->variable()->nameToken() == prev;
5708+
}
5709+
56705710
static void valueFlowForwardAssign(Token* const tok,
56715711
const Token* expr,
56725712
std::vector<const Variable*> vars,
@@ -5756,6 +5796,24 @@ static void valueFlowForwardAssign(Token* const tok,
57565796
constValues.splice(constValues.end(), values, it, values.end());
57575797
valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), constValues, settings);
57585798
}
5799+
if (isInitialVarAssign(expr)) {
5800+
// Check if variable is only incremented or decremented
5801+
ValueFlow::Value::Bound b = findVarBound(expr->variable(), nextExpression, endOfVarScope, settings);
5802+
if (b != ValueFlow::Value::Bound::Point) {
5803+
auto knownValueIt = std::find_if(values.begin(), values.end(), [](const ValueFlow::Value& value) {
5804+
if (!value.isKnown())
5805+
return false;
5806+
return value.isIntValue();
5807+
});
5808+
if (knownValueIt != values.end()) {
5809+
ValueFlow::Value value = *knownValueIt;
5810+
value.bound = b;
5811+
value.invertRange();
5812+
value.setImpossible();
5813+
valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), {value}, settings);
5814+
}
5815+
}
5816+
}
57595817
valueFlowForward(nextExpression, endOfVarScope, expr, values, tokenlist, settings);
57605818
}
57615819

@@ -6254,7 +6312,7 @@ struct ConditionHandler {
62546312
// Variable changed in 3rd for-expression
62556313
if (Token::simpleMatch(top->previous(), "for (")) {
62566314
if (top->astOperand2() && top->astOperand2()->astOperand2() &&
6257-
isExpressionChanged(
6315+
findExpressionChanged(
62586316
cond.vartok, top->astOperand2()->astOperand2(), top->link(), settings, tokenlist.isCPP())) {
62596317
if (settings->debugwarnings)
62606318
bailout(tokenlist,
@@ -6270,7 +6328,7 @@ struct ConditionHandler {
62706328
const Token* const block = top->link()->next();
62716329
const Token* const end = block->link();
62726330

6273-
if (isExpressionChanged(cond.vartok, start, end, settings, tokenlist.isCPP())) {
6331+
if (findExpressionChanged(cond.vartok, start, end, settings, tokenlist.isCPP())) {
62746332
// If its reassigned in loop then analyze from the end
62756333
if (!Token::Match(tok, "%assign%|++|--") &&
62766334
findExpression(cond.vartok->exprId(), start, end, [&](const Token* tok2) {

‎test/testastutils.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ class TestAstUtils : public TestFixture {
397397
const Token* const start = Token::findsimplematch(tokenizer.tokens(), startPattern, strlen(startPattern));
398398
const Token* const end = Token::findsimplematch(start, endPattern, strlen(endPattern));
399399
const Token* const expr = Token::findsimplematch(tokenizer.tokens(), var, strlen(var));
400-
return (isExpressionChanged)(expr, start, end, &settings, true);
400+
return (findExpressionChanged)(expr, start, end, &settings, true);
401401
}
402402

403403
void isExpressionChangedTest()

0 commit comments

Comments
 (0)