diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 2d6620629..49fe0fc02 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1181,7 +1181,7 @@ static const Token * followVariableExpression(const Token * tok, bool cpp, const return tok; } else if (!precedes(startToken, endToken)) { return tok; - } else if (isExpressionChanged(varTok, startToken, endToken, nullptr, cpp)) { + } else if (findExpressionChanged(varTok, startToken, endToken, nullptr, cpp)) { return tok; } return varTok; @@ -1549,7 +1549,7 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 const Token *start = refTok1, *end = refTok2; if (!precedes(start, end)) std::swap(start, end); - if (isExpressionChanged(start, start, end, nullptr, cpp)) + if (findExpressionChanged(start, start, end, nullptr, cpp)) return false; } 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) 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, @@ -2871,34 +2871,36 @@ bool isThisChanged(const Token* tok, int indirect, const Settings* settings, boo 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)) - return false; + return nullptr; for (const Token* tok = start; tok != end; tok = tok->next()) { if (!exprDependsOnThis(tok)) continue; if (isThisChanged(tok, indirect, settings, cpp)) - return true; + return tok; } - return false; + return nullptr; } template -bool isExpressionChangedImpl(const Token* expr, - const Token* start, - const Token* end, - const Settings* settings, - bool cpp, - int depth, - Find find) +const Token* findExpressionChangedImpl(const Token* expr, + const Token* start, + const Token* end, + const Settings* settings, + bool cpp, + int depth, + Find find) { if (depth < 0) - return true; + return start; if (!precedes(start, end)) - return false; - const Token* result = findAstNode(expr, [&](const Token* tok) { - if (exprDependsOnThis(tok) && isThisChanged(start, end, false, settings, cpp)) { + return nullptr; + const Token* result = nullptr; + findAstNode(expr, [&](const Token* tok) { + if (exprDependsOnThis(tok)) { + result = findThisChanged(start, end, false, settings, cpp); return true; } bool global = false; @@ -2912,7 +2914,7 @@ bool isExpressionChangedImpl(const Token* expr, } 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; if (const ValueType* vt = tok->valueType()) { indirect = vt->pointer; @@ -2924,8 +2926,10 @@ bool isExpressionChangedImpl(const Token* expr, return true; return false; }); - if (result) + if (modifedTok) { + result = modifedTok; return true; + } } return false; }); @@ -2954,20 +2958,25 @@ 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* start, - const Token* end, - const Settings* settings, - bool cpp, - const std::function(const Token* tok)>& evaluate, - int depth) +const Token* findExpressionChangedSkipDeadCode(const Token* expr, + const Token* start, + const Token* end, + const Settings* settings, + bool cpp, + const std::function(const Token* tok)>& evaluate, + int depth) { - return isExpressionChangedImpl( + return findExpressionChangedImpl( expr, start, end, settings, cpp, depth, ExpressionChangedSkipDeadCode{&settings->library, evaluate}); } diff --git a/lib/astutils.h b/lib/astutils.h index 3677a4ddb..dd56b4cb5 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -335,25 +335,25 @@ bool isVariablesChanged(const Token* start, 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); 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, - const Token* start, - const Token* end, - const Settings* settings, - bool cpp, - int depth = 20); +CPPCHECKLIB const Token* findExpressionChanged(const Token* expr, + const Token* start, + const Token* end, + const Settings* settings, + bool cpp, + int depth = 20); -bool isExpressionChangedSkipDeadCode(const Token* expr, - const Token* start, - const Token* end, - const Settings* settings, - bool cpp, - const std::function(const Token* tok)>& evaluate, - int depth = 20); +const Token* findExpressionChangedSkipDeadCode(const Token* expr, + const Token* start, + const Token* end, + const Settings* settings, + bool cpp, + const std::function(const Token* tok)>& evaluate, + int depth = 20); bool isExpressionChangedAt(const Token* expr, const Token* tok, diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 4866b499c..a9813297a 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -505,7 +505,7 @@ void CheckCondition::duplicateCondition() continue; 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)) duplicateConditionError(cond1, cond2, errorPath); } @@ -554,10 +554,12 @@ void CheckCondition::multiCondition() if (tok2->astOperand2()) { 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()); - else if (isOppositeCond(true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath) && - !isExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP())) + else if (isOppositeCond( + true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath) && + !findExpressionChanged(cond1, cond1, tok2->astOperand2(), mSettings, mTokenizer->isCPP())) oppositeElseIfConditionError(cond1, tok2->astOperand2(), errorPath); } } @@ -709,7 +711,7 @@ void CheckCondition::multiCondition2() const Token * condStartToken = tok->str() == "if" ? tok->next() : tok; const Token * condEndToken = tok->str() == "if" ? condStartToken->link() : Token::findsimplematch(condStartToken, ";"); // Does condition modify tracked variables? - if (isExpressionChanged(cond1, condStartToken, condEndToken, mSettings, mTokenizer->isCPP())) + if (findExpressionChanged(cond1, condStartToken, condEndToken, mSettings, mTokenizer->isCPP())) break; // Condition.. diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 2a56127a5..8dec514a9 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -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))) 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)) { if (isWithoutSideEffects(cpp, tok->astOperand1())) { 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 assignment = !isEnum && tok->str() == "="; if (assignment && warningEnabled) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 6a2860886..cd62fbc2c 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -387,9 +387,10 @@ struct ForwardTraversal { if (stepTok) { std::pair exprToks = stepTok->findExpressionStartEndTokens(); 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 const bool condChanged = nullptr != findAstNode(condTok, [&](const Token* tok) { diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 3c829b095..72519c164 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -289,7 +289,7 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke return; if (!truevalue.isIntValue()) return; - if (endTok && isExpressionChanged(vartok, tok->next(), endTok, settings, true)) + if (endTok && findExpressionChanged(vartok, tok->next(), endTok, settings, true)) return; const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then); 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); } } else if (tok->exprId() > 0) { - if (endTok && isExpressionChanged(tok, tok->next(), endTok, settings, true)) + if (endTok && findExpressionChanged(tok, tok->next(), endTok, settings, true)) return; pm.setIntValue(tok, 0, then); 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) { const Token* start = origins[e.getExpressionId()]; 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()); return true; } diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index be6b403b5..9e9749a4f 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -8929,8 +8929,8 @@ void Tokenizer::simplifyDeclspec() { for (Token *tok = list.front(); tok; tok = tok->next()) { while (isAttribute(tok, false)) { - Token *functok = getAttributeFuncTok(tok, false); if (Token::Match(tok->tokAt(2), "noreturn|nothrow|dllexport")) { + Token *functok = getAttributeFuncTok(tok, false); if (functok) { if (tok->strAt(2) == "noreturn") functok->isAttributeNoreturn(true); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index f0a7f4801..78fde897d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5667,6 +5667,46 @@ static void valueFlowForwardConst(Token* start, 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, const Token* expr, std::vector vars, @@ -5756,6 +5796,24 @@ static void valueFlowForwardAssign(Token* const tok, constValues.splice(constValues.end(), values, it, values.end()); 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); } @@ -6254,7 +6312,7 @@ struct ConditionHandler { // Variable changed in 3rd for-expression if (Token::simpleMatch(top->previous(), "for (")) { if (top->astOperand2() && top->astOperand2()->astOperand2() && - isExpressionChanged( + findExpressionChanged( cond.vartok, top->astOperand2()->astOperand2(), top->link(), settings, tokenlist.isCPP())) { if (settings->debugwarnings) bailout(tokenlist, @@ -6270,7 +6328,7 @@ struct ConditionHandler { const Token* const block = top->link()->next(); 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 (!Token::Match(tok, "%assign%|++|--") && findExpression(cond.vartok->exprId(), start, end, [&](const Token* tok2) { diff --git a/test/testastutils.cpp b/test/testastutils.cpp index 50a6aa36a..eacaaace8 100644 --- a/test/testastutils.cpp +++ b/test/testastutils.cpp @@ -397,7 +397,7 @@ private: const Token* const start = Token::findsimplematch(tokenizer.tokens(), startPattern, strlen(startPattern)); const Token* const end = Token::findsimplematch(start, endPattern, strlen(endPattern)); 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() diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 75f985e43..542691618 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -156,6 +156,7 @@ private: TEST_CASE(valueFlowSymbolicStrlen); TEST_CASE(valueFlowSmartPointer); TEST_CASE(valueFlowImpossibleMinMax); + TEST_CASE(valueFlowImpossibleIncDec); TEST_CASE(valueFlowImpossibleUnknownConstant); TEST_CASE(valueFlowContainerEqual); @@ -8160,6 +8161,19 @@ private: 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() { const char* code;