From 42a56b069dc230b18acc31da43aa3f99c1f936dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 11 Dec 2023 11:49:16 +0100 Subject: [PATCH] Revert "Fix 12031: False positive: uninitialized variable (#5637)" This reverts commit 243fa66bd39338d8be5565c3b41a63d54b4965ad. --- lib/astutils.cpp | 27 ++-- lib/astutils.h | 16 --- lib/forwardanalyzer.cpp | 8 ++ lib/programmemory.cpp | 309 ++++++++++------------------------------ lib/programmemory.h | 44 +----- lib/utils.h | 9 -- lib/valueflow.cpp | 11 +- test/testcondition.cpp | 15 -- test/testvalueflow.cpp | 38 ----- 9 files changed, 107 insertions(+), 370 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 3b4d7231103..fecf3171168 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -107,17 +107,31 @@ static int getArgumentPos(const Token* ftok, const Token* tokToFind){ return findArgumentPos(startTok, tokToFind); } +template )> +static void astFlattenRecursive(T* tok, std::vector& result, const char* op, nonneg int depth = 0) +{ + ++depth; + if (!tok || depth >= 100) + return; + if (tok->str() == op) { + astFlattenRecursive(tok->astOperand1(), result, op, depth); + astFlattenRecursive(tok->astOperand2(), result, op, depth); + } else { + result.push_back(tok); + } +} + std::vector astFlatten(const Token* tok, const char* op) { std::vector result; - astFlattenCopy(tok, op, std::back_inserter(result)); + astFlattenRecursive(tok, result, op); return result; } std::vector astFlatten(Token* tok, const char* op) { std::vector result; - astFlattenCopy(tok, op, std::back_inserter(result)); + astFlattenRecursive(tok, result, op); return result; } @@ -149,15 +163,6 @@ bool astHasVar(const Token * tok, nonneg int varid) return astHasVar(tok->astOperand1(), varid) || astHasVar(tok->astOperand2(), varid); } -bool astHasExpr(const Token* tok, nonneg int exprid) -{ - if (!tok) - return false; - if (tok->exprId() == exprid) - return true; - return astHasExpr(tok->astOperand1(), exprid) || astHasExpr(tok->astOperand2(), exprid); -} - static bool astIsCharWithSign(const Token *tok, ValueType::Sign sign) { if (!tok) diff --git a/lib/astutils.h b/lib/astutils.h index 66eab821d6f..b4a7be82c80 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -116,21 +116,6 @@ const Token* findExpression(const nonneg int exprid, const std::function& pred); const Token* findExpression(const Token* start, const nonneg int exprid); -template )> -void astFlattenCopy(T* tok, const char* op, OuputIterator out, nonneg int depth = 100) -{ - --depth; - if (!tok || depth < 0) - return; - if (tok->str() == op) { - astFlattenCopy(tok->astOperand1(), op, out, depth); - astFlattenCopy(tok->astOperand2(), op, out, depth); - } else { - *out = tok; - ++out; - } -} - std::vector astFlatten(const Token* tok, const char* op); std::vector astFlatten(Token* tok, const char* op); @@ -138,7 +123,6 @@ nonneg int astCount(const Token* tok, const char* op, int depth = 100); bool astHasToken(const Token* root, const Token * tok); -bool astHasExpr(const Token* tok, nonneg int exprid); bool astHasVar(const Token * tok, nonneg int varid); bool astIsPrimitive(const Token* tok); diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 6b95905c151..c46a32da9da 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -40,6 +40,14 @@ #include namespace { + struct OnExit { + std::function f; + + ~OnExit() { + f(); + } + }; + struct ForwardTraversal { enum class Progress { Continue, Break, Skip }; enum class Terminate { None, Bail, Inconclusive }; diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 3a140424fd9..aaef10b6ad2 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -40,10 +40,6 @@ #include #include -#include - -ExprIdToken::ExprIdToken(const Token* tok) : tok(tok), exprid(tok ? tok->exprId() : 0) {} - nonneg int ExprIdToken::getExpressionId() const { return tok ? tok->exprId() : exprid; } @@ -197,12 +193,24 @@ void ProgramMemory::insert(const ProgramMemory &pm) mValues.insert(p); } -static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings* settings); +static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings* settings = nullptr); -static bool evaluateCondition(MathLib::bigint r, const Token* condition, ProgramMemory& pm, const Settings* settings) +static bool evaluateCondition(const std::string& op, + MathLib::bigint r, + const Token* condition, + ProgramMemory& pm, + const Settings* settings) { if (!condition) return false; + if (condition->str() == op) { + if (evaluateCondition(op, r, condition->astOperand1(), pm, settings) || + evaluateCondition(op, r, condition->astOperand2(), pm, settings)) { + return true; + } + if (!pm.hasValue(condition->exprId())) + return false; + } MathLib::bigint result = 0; bool error = false; execute(condition, pm, &result, &error, settings); @@ -211,12 +219,12 @@ static bool evaluateCondition(MathLib::bigint r, const Token* condition, Program bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings* settings) { - return evaluateCondition(0, condition, pm, settings); + return evaluateCondition("&&", 0, condition, pm, settings); } bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings* settings) { - return evaluateCondition(1, condition, pm, settings); + return evaluateCondition("||", 1, condition, pm, settings); } static bool frontIs(const std::vector& v, bool i) @@ -230,8 +238,6 @@ static bool frontIs(const std::vector& v, bool i) static bool isTrue(const ValueFlow::Value& v) { - if (v.isUninitValue()) - return false; if (v.isImpossible()) return v.intvalue == 0; return v.intvalue != 0; @@ -239,20 +245,11 @@ static bool isTrue(const ValueFlow::Value& v) static bool isFalse(const ValueFlow::Value& v) { - if (v.isUninitValue()) - return false; if (v.isImpossible()) return false; return v.intvalue == 0; } -static bool isTrueOrFalse(const ValueFlow::Value& v, bool b) -{ - if (b) - return isTrue(v); - return isFalse(v); -} - // If the scope is a non-range for loop static bool isBasicForLoop(const Token* tok) { @@ -279,7 +276,7 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke return {t->values().front().intvalue}; MathLib::bigint result = 0; bool error = false; - execute(t, pm, &result, &error, settings); + execute(t, pm, &result, &error); if (!error) return {result}; return std::vector{}; @@ -325,7 +322,6 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke if (endTok && findExpressionChanged(tok, tok->next(), endTok, settings, true)) return; pm.setIntValue(tok, 0, then); - assert(settings); const Token* containerTok = settings->library.getContainerFromYield(tok, Library::Container::Yield::EMPTY); if (containerTok) pm.setContainerSizeValue(containerTok, 0, then); @@ -346,7 +342,7 @@ static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Scope* scop return; MathLib::bigint result = 0; bool error = false; - execute(condTok, pm, &result, &error, settings); + execute(condTok, pm, &result, &error); if (error) programMemoryParseCondition(pm, condTok, endTok, settings, scope->type != Scope::eElse); } @@ -376,7 +372,7 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok if (!setvar) { if (!pm.hasValue(vartok->exprId())) { const Token* valuetok = tok2->astOperand2(); - pm.setValue(vartok, execute(valuetok, pm, settings)); + pm.setValue(vartok, execute(valuetok, pm)); } } } else if (tok2->exprId() > 0 && Token::Match(tok2, ".|(|[|*|%var%") && !pm.hasValue(tok2->exprId()) && @@ -388,7 +384,7 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok if (indentlevel <= 0) { const Token* cond = getCondTokFromEnd(tok2->link()); // Keep progressing with anonymous/do scopes and always true branches - if (!Token::Match(tok2->previous(), "do|; {") && !conditionIsTrue(cond, state, settings) && + if (!Token::Match(tok2->previous(), "do|; {") && !conditionIsTrue(cond, state) && (cond || !isBasicForLoop(tok2))) break; } else @@ -400,12 +396,12 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok const Token *cond = getCondTokFromEnd(tok2); const bool inElse = Token::simpleMatch(tok2->link()->previous(), "else {"); if (cond) { - if (conditionIsFalse(cond, state, settings)) { + if (conditionIsFalse(cond, state)) { if (inElse) { ++indentlevel; continue; } - } else if (conditionIsTrue(cond, state, settings)) { + } else if (conditionIsTrue(cond, state)) { if (inElse) tok2 = tok2->link()->tokAt(-2); ++indentlevel; @@ -498,9 +494,9 @@ void ProgramMemoryState::removeModifiedVars(const Token* tok) { ProgramMemory pm = state; auto eval = [&](const Token* cond) -> std::vector { - if (conditionIsTrue(cond, pm, settings)) + if (conditionIsTrue(cond, pm)) return {1}; - if (conditionIsFalse(cond, pm, settings)) + if (conditionIsFalse(cond, pm)) return {0}; return {}; }; @@ -1204,183 +1200,20 @@ static BuiltinLibraryFunction getBuiltinLibraryFunction(const std::string& name) return nullptr; return it->second; } -static bool TokenExprIdCompare(const Token* tok1, const Token* tok2) { - return tok1->exprId() < tok2->exprId(); -} -static bool TokenExprIdEqual(const Token* tok1, const Token* tok2) { - return tok1->exprId() == tok2->exprId(); -} - -static std::vector setDifference(const std::vector& v1, const std::vector& v2) -{ - std::vector result; - std::set_difference(v1.begin(), v1.end(), v2.begin(), v2.end(), std::back_inserter(result), &TokenExprIdCompare); - return result; -} - -static bool evalSameCondition(const ProgramMemory& state, - const Token* storedValue, - const Token* cond, - const Settings* settings) -{ - assert(!conditionIsTrue(cond, state, settings)); - ProgramMemory pm = state; - programMemoryParseCondition(pm, storedValue, nullptr, settings, true); - if (pm == state) - return false; - return conditionIsTrue(cond, pm, settings); -} - -static void pruneConditions(std::vector& conds, - bool b, - const std::unordered_map& state) -{ - conds.erase(std::remove_if(conds.begin(), - conds.end(), - [&](const Token* cond) { - if (cond->exprId() == 0) - return false; - auto it = state.find(cond->exprId()); - if (it == state.end()) - return false; - const ValueFlow::Value& v = it->second; - return isTrueOrFalse(v, !b); - }), - conds.end()); -} namespace { struct Executor { ProgramMemory* pm = nullptr; const Settings* settings = nullptr; int fdepth = 4; - int depth = 10; explicit Executor(ProgramMemory* pm = nullptr, const Settings* settings = nullptr) : pm(pm), settings(settings) {} - static ValueFlow::Value unknown() { - return ValueFlow::Value::unknown(); - } - - std::unordered_map executeAll(const std::vector& toks, - const bool* b = nullptr) const - { - std::unordered_map result; - auto state = *this; - for (const Token* tok : toks) { - ValueFlow::Value r = state.execute(tok); - if (r.isUninitValue()) - continue; - result.insert(std::make_pair(tok->exprId(), r)); - // Short-circuit evaluation - if (b && isTrueOrFalse(r, *b)) - break; - } - return result; - } - - static std::vector flattenConditions(const Token* tok) - { - return astFlatten(tok, tok->str().c_str()); - } - static bool sortConditions(std::vector& conditions) - { - if (std::any_of(conditions.begin(), conditions.end(), [](const Token* child) { - return Token::Match(child, "&&|%oror%"); - })) - return false; - std::sort(conditions.begin(), conditions.end(), &TokenExprIdCompare); - return !conditions.empty() && conditions.front()->exprId() != 0; - } - - ValueFlow::Value executeMultiCondition(bool b, const Token* expr) - { - if (pm->hasValue(expr->exprId())) { - const ValueFlow::Value& v = pm->at(expr->exprId()); - if (v.isIntValue()) - return v; - } - - // Evaluate recursively if there are no exprids - if ((expr->astOperand1() && expr->astOperand1()->exprId() == 0) || - (expr->astOperand2() && expr->astOperand2()->exprId() == 0)) { - ValueFlow::Value lhs = execute(expr->astOperand1()); - if (isTrueOrFalse(lhs, b)) - return lhs; - ValueFlow::Value rhs = execute(expr->astOperand2()); - if (isTrueOrFalse(rhs, b)) - return rhs; - if (isTrueOrFalse(lhs, !b) && isTrueOrFalse(rhs, !b)) - return lhs; - return unknown(); - } - - nonneg int n = astCount(expr, expr->str().c_str()); - if (n > 50) - return unknown(); - std::vector conditions1 = flattenConditions(expr); - if (conditions1.empty()) - return unknown(); - std::unordered_map condValues = executeAll(conditions1, &b); - bool allNegated = true; - ValueFlow::Value negatedValue = unknown(); - for (const auto& p : condValues) { - const ValueFlow::Value& v = p.second; - if (isTrueOrFalse(v, b)) - return v; - allNegated &= isTrueOrFalse(v, !b); - if (allNegated && negatedValue.isUninitValue()) - negatedValue = v; - } - if (condValues.size() == conditions1.size() && allNegated) - return negatedValue; - if (n > 4) - return unknown(); - if (!sortConditions(conditions1)) - return unknown(); - - for (const auto& p : *pm) { - const Token* tok = p.first.tok; - const ValueFlow::Value& value = p.second; - - if (tok->str() == expr->str() && !astHasExpr(tok, expr->exprId())) { - // TODO: Handle when it is greater - if (n != astCount(tok, expr->str().c_str())) - continue; - std::vector conditions2 = flattenConditions(tok); - if (!sortConditions(conditions2)) - return unknown(); - if (conditions1.size() == conditions2.size() && - std::equal(conditions1.begin(), conditions1.end(), conditions2.begin(), &TokenExprIdEqual)) - return value; - std::vector diffConditions1 = setDifference(conditions1, conditions2); - std::vector diffConditions2 = setDifference(conditions2, conditions1); - pruneConditions(diffConditions1, b, condValues); - pruneConditions(diffConditions2, b, executeAll(diffConditions2)); - if (diffConditions1.size() != diffConditions2.size()) - continue; - if (diffConditions1.size() == conditions1.size()) - continue; - for (const Token* cond1 : diffConditions1) { - auto it = std::find_if(diffConditions2.begin(), diffConditions2.end(), [&](const Token* cond2) { - return evalSameCondition(*pm, cond2, cond1, settings); - }); - if (it == diffConditions2.end()) - break; - diffConditions2.erase(it); - } - if (diffConditions2.empty()) - return value; - } - } - return unknown(); - } - ValueFlow::Value executeImpl(const Token* expr) { const ValueFlow::Value* value = nullptr; if (!expr) - return unknown(); + return ValueFlow::Value::unknown(); if (expr->hasKnownIntValue() && !expr->isAssignmentOp() && expr->str() != ",") return expr->values().front(); if ((value = expr->getKnownValue(ValueFlow::Value::ValueType::FLOAT)) || @@ -1392,10 +1225,10 @@ namespace { } if (expr->isNumber()) { if (MathLib::isFloat(expr->str())) - return unknown(); + return ValueFlow::Value::unknown(); MathLib::bigint i = MathLib::toBigNumber(expr->str()); if (i < 0 && astIsUnsigned(expr)) - return unknown(); + return ValueFlow::Value::unknown(); return ValueFlow::Value{i}; } if (expr->isBoolean()) @@ -1406,14 +1239,14 @@ namespace { if (yield == Library::Container::Yield::SIZE) { ValueFlow::Value v = execute(containerTok); if (!v.isContainerSizeValue()) - return unknown(); + return ValueFlow::Value::unknown(); v.valueType = ValueFlow::Value::ValueType::INT; return v; } if (yield == Library::Container::Yield::EMPTY) { ValueFlow::Value v = execute(containerTok); if (!v.isContainerSizeValue()) - return unknown(); + return ValueFlow::Value::unknown(); if (v.isImpossible() && v.intvalue == 0) return ValueFlow::Value{0}; if (!v.isImpossible()) @@ -1423,10 +1256,10 @@ namespace { expr->astOperand1()->exprId() > 0) { ValueFlow::Value rhs = execute(expr->astOperand2()); if (rhs.isUninitValue()) - return unknown(); + return ValueFlow::Value::unknown(); if (expr->str() != "=") { if (!pm->hasValue(expr->astOperand1()->exprId())) - return unknown(); + return ValueFlow::Value::unknown(); ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId()); rhs = evaluate(removeAssign(expr->str()), lhs, rhs); if (lhs.isIntValue()) @@ -1435,15 +1268,29 @@ namespace { ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.floatValue), std::placeholders::_1)); else - return unknown(); + return ValueFlow::Value::unknown(); return lhs; } pm->setValue(expr->astOperand1(), rhs); return rhs; } else if (expr->str() == "&&" && expr->astOperand1() && expr->astOperand2()) { - return executeMultiCondition(false, expr); + ValueFlow::Value lhs = execute(expr->astOperand1()); + if (!lhs.isIntValue()) + return ValueFlow::Value::unknown(); + if (isFalse(lhs)) + return lhs; + if (isTrue(lhs)) + return execute(expr->astOperand2()); + return ValueFlow::Value::unknown(); } else if (expr->str() == "||" && expr->astOperand1() && expr->astOperand2()) { - return executeMultiCondition(true, expr); + ValueFlow::Value lhs = execute(expr->astOperand1()); + if (!lhs.isIntValue() || lhs.isImpossible()) + return ValueFlow::Value::unknown(); + if (isTrue(lhs)) + return lhs; + if (isFalse(lhs)) + return execute(expr->astOperand2()); + return ValueFlow::Value::unknown(); } else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) { execute(expr->astOperand1()); return execute(expr->astOperand2()); @@ -1452,10 +1299,10 @@ namespace { return ValueFlow::Value::unknown(); ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId()); if (!lhs.isIntValue()) - return unknown(); + return ValueFlow::Value::unknown(); // overflow if (!lhs.isImpossible() && lhs.intvalue == 0 && expr->str() == "--" && astIsUnsigned(expr->astOperand1())) - return unknown(); + return ValueFlow::Value::unknown(); if (expr->str() == "++") lhs.intvalue++; @@ -1469,17 +1316,17 @@ namespace { expr->astOperand1()->values().cend(), std::mem_fn(&ValueFlow::Value::isTokValue)); if (tokvalue_it == expr->astOperand1()->values().cend() || !tokvalue_it->isKnown()) { - return unknown(); + return ValueFlow::Value::unknown(); } tokvalue = tokvalue_it->tokvalue; } if (!tokvalue || !tokvalue->isLiteral()) { - return unknown(); + return ValueFlow::Value::unknown(); } const std::string strValue = tokvalue->strValue(); ValueFlow::Value rhs = execute(expr->astOperand2()); if (!rhs.isIntValue()) - return unknown(); + return ValueFlow::Value::unknown(); const MathLib::bigint index = rhs.intvalue; if (index >= 0 && index < strValue.size()) return ValueFlow::Value{strValue[index]}; @@ -1488,7 +1335,7 @@ namespace { } else if (Token::Match(expr, "%cop%") && expr->astOperand1() && expr->astOperand2()) { ValueFlow::Value lhs = execute(expr->astOperand1()); ValueFlow::Value rhs = execute(expr->astOperand2()); - ValueFlow::Value r = unknown(); + ValueFlow::Value r = ValueFlow::Value::unknown(); if (!lhs.isUninitValue() && !rhs.isUninitValue()) r = evaluate(expr->str(), lhs, rhs); if (expr->isComparisonOp() && (r.isUninitValue() || r.isImpossible())) { @@ -1504,7 +1351,7 @@ namespace { if (!result.empty() && result.front().isKnown()) return result.front(); } - return unknown(); + return ValueFlow::Value::unknown(); } return r; } @@ -1512,14 +1359,14 @@ namespace { else if (Token::Match(expr, "!|+|-") && expr->astOperand1() && !expr->astOperand2()) { ValueFlow::Value lhs = execute(expr->astOperand1()); if (!lhs.isIntValue()) - return unknown(); + return ValueFlow::Value::unknown(); if (expr->str() == "!") { if (isTrue(lhs)) { lhs.intvalue = 0; } else if (isFalse(lhs)) { lhs.intvalue = 1; } else { - return unknown(); + return ValueFlow::Value::unknown(); } lhs.setPossible(); lhs.bound = ValueFlow::Value::Bound::Point; @@ -1530,14 +1377,14 @@ namespace { } else if (expr->str() == "?" && expr->astOperand1() && expr->astOperand2()) { ValueFlow::Value cond = execute(expr->astOperand1()); if (!cond.isIntValue()) - return unknown(); + return ValueFlow::Value::unknown(); const Token* child = expr->astOperand2(); if (isFalse(cond)) return execute(child->astOperand2()); if (isTrue(cond)) return execute(child->astOperand1()); - return unknown(); + return ValueFlow::Value::unknown(); } else if (expr->str() == "(" && expr->isCast()) { if (Token::simpleMatch(expr->previous(), ">") && expr->previous()->link()) return execute(expr->astOperand2()); @@ -1555,12 +1402,11 @@ namespace { if (Token::Match(expr->previous(), ">|%name% {|(")) { const Token* ftok = expr->previous(); const Function* f = ftok->function(); - ValueFlow::Value result = unknown(); + ValueFlow::Value result = ValueFlow::Value::unknown(); if (settings && expr->str() == "(") { std::vector tokArgs = getArguments(expr); std::vector args(tokArgs.size()); - std::transform( - tokArgs.cbegin(), tokArgs.cend(), args.begin(), [&](const Token* tok) { + std::transform(tokArgs.cbegin(), tokArgs.cend(), args.begin(), [&](const Token* tok) { return execute(tok); }); if (f) { @@ -1569,7 +1415,7 @@ namespace { for (std::size_t i = 0; i < args.size(); ++i) { const Variable* const arg = f->getArgumentVar(i); if (!arg) - return unknown(); + return ValueFlow::Value::unknown(); functionState.setValue(arg->nameToken(), args[i]); } Executor ex = *this; @@ -1603,10 +1449,10 @@ namespace { ValueFlow::Value& v = pm->at(child->exprId()); if (v.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) { if (ValueFlow::isContainerSizeChanged(child, v.indirect, settings)) - v = unknown(); + v = ValueFlow::Value::unknown(); } else if (v.valueType != ValueFlow::Value::ValueType::UNINIT) { if (isVariableChanged(child, v.indirect, settings, true)) - v = unknown(); + v = ValueFlow::Value::unknown(); } } return ChildrenToVisit::op1_and_op2; @@ -1614,7 +1460,7 @@ namespace { return result; } - return unknown(); + return ValueFlow::Value::unknown(); } static const ValueFlow::Value* getImpossibleValue(const Token* tok) { @@ -1639,12 +1485,6 @@ namespace { ValueFlow::Value execute(const Token* expr) { - depth--; - OnExit onExit{[&] { - depth++; - }}; - if (depth < 0) - return unknown(); ValueFlow::Value v = executeImpl(expr); if (!v.isUninitValue()) return v; @@ -1659,30 +1499,31 @@ namespace { std::vector execute(const Scope* scope) { + static const std::vector unknown = {ValueFlow::Value::unknown()}; if (!scope) - return {unknown()}; + return unknown; if (!scope->bodyStart) - return {unknown()}; + return unknown; for (const Token* tok = scope->bodyStart->next(); precedes(tok, scope->bodyEnd); tok = tok->next()) { const Token* top = tok->astTop(); if (!top) - return {unknown()}; + return unknown; if (Token::simpleMatch(top, "return") && top->astOperand1()) return {execute(top->astOperand1())}; if (Token::Match(top, "%op%")) { if (execute(top).isUninitValue()) - return {unknown()}; + return unknown; const Token* next = nextAfterAstRightmostLeaf(top); if (!next) - return {unknown()}; + return unknown; tok = next; } else if (Token::simpleMatch(top->previous(), "if (")) { const Token* condTok = top->astOperand2(); ValueFlow::Value v = execute(condTok); if (!v.isIntValue()) - return {unknown()}; + return unknown; const Token* thenStart = top->link()->next(); const Token* next = thenStart->link(); const Token* elseStart = nullptr; @@ -1697,19 +1538,19 @@ namespace { if (elseStart) result = execute(elseStart->scope()); } else { - return {unknown()}; + return unknown; } if (!result.empty()) return result; tok = next; } else { - return {unknown()}; + return unknown; } } return {}; } }; -} // namespace +} static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings* settings) { diff --git a/lib/programmemory.h b/lib/programmemory.h index f81ef7b35be..1820b1770c7 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -43,7 +43,7 @@ struct ExprIdToken { ExprIdToken() = default; // cppcheck-suppress noExplicitConstructor // NOLINTNEXTLINE(google-explicit-constructor) - ExprIdToken(const Token* tok); + ExprIdToken(const Token* tok) : tok(tok) {} // TODO: Make this constructor only available from ProgramMemory // cppcheck-suppress noExplicitConstructor // NOLINTNEXTLINE(google-explicit-constructor) @@ -55,42 +55,12 @@ struct ExprIdToken { return getExpressionId() == rhs.getExpressionId(); } - bool operator<(const ExprIdToken& rhs) const { - return getExpressionId() < rhs.getExpressionId(); - } - template friend bool operator!=(const T& lhs, const U& rhs) { return !(lhs == rhs); } - template - friend bool operator<=(const T& lhs, const U& rhs) - { - return !(lhs > rhs); - } - - template - friend bool operator>(const T& lhs, const U& rhs) - { - return rhs < lhs; - } - - template - friend bool operator>=(const T& lhs, const U& rhs) - { - return !(lhs < rhs); - } - - const Token& operator*() const NOEXCEPT { - return *tok; - } - - const Token* operator->() const NOEXCEPT { - return tok; - } - struct Hash { std::size_t operator()(ExprIdToken etok) const; }; @@ -149,14 +119,6 @@ struct ProgramMemory { return mValues.end(); } - friend bool operator==(const ProgramMemory& x, const ProgramMemory& y) { - return x.mValues == y.mValues; - } - - friend bool operator!=(const ProgramMemory& x, const ProgramMemory& y) { - return x.mValues != y.mValues; - } - private: Map mValues; }; @@ -195,14 +157,14 @@ void execute(const Token* expr, * \param condition top ast token in condition * \param pm program memory */ -bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings* settings); +bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings* settings = nullptr); /** * Is condition always true when variable has given value? * \param condition top ast token in condition * \param pm program memory */ -bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings* settings); +bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings* settings = nullptr); /** * Get program memory by looking backwards from given token. diff --git a/lib/utils.h b/lib/utils.h index 481d9e890b3..9d11a68e983 100644 --- a/lib/utils.h +++ b/lib/utils.h @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -50,14 +49,6 @@ struct SelectMapValues { } }; -struct OnExit { - std::function f; - - ~OnExit() { - f(); - } -}; - template bool contains(const Range& r, const T& x) { diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 6cf189e24c4..10f0a663813 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7077,8 +7077,8 @@ static void valueFlowForLoopSimplify(Token* const bodyStart, if (Token::Match(tok2, "%oror%|&&")) { const ProgramMemory programMemory(getProgramMemory(tok2->astTop(), expr, ValueFlow::Value(value), settings)); - if ((tok2->str() == "&&" && !conditionIsTrue(tok2->astOperand1(), programMemory, settings)) || - (tok2->str() == "||" && !conditionIsFalse(tok2->astOperand1(), programMemory, settings))) { + if ((tok2->str() == "&&" && !conditionIsTrue(tok2->astOperand1(), programMemory)) || + (tok2->str() == "||" && !conditionIsFalse(tok2->astOperand1(), programMemory))) { // Skip second expression.. Token *parent = tok2; while (parent && parent->str() == tok2->str()) @@ -7091,6 +7091,7 @@ static void valueFlowForLoopSimplify(Token* const bodyStart, tok2 = tok2->linkAt(1); } } + } const Token* vartok = expr; const Token* rml = nextAfterAstRightmostLeaf(vartok); @@ -7101,12 +7102,10 @@ static void valueFlowForLoopSimplify(Token* const bodyStart, if ((tok2->str() == "&&" && conditionIsFalse(tok2->astOperand1(), - getProgramMemory(tok2->astTop(), expr, ValueFlow::Value(value), settings), - settings)) || + getProgramMemory(tok2->astTop(), expr, ValueFlow::Value(value), settings))) || (tok2->str() == "||" && conditionIsTrue(tok2->astOperand1(), - getProgramMemory(tok2->astTop(), expr, ValueFlow::Value(value), settings), - settings))) + getProgramMemory(tok2->astTop(), expr, ValueFlow::Value(value), settings)))) break; if (Token::simpleMatch(tok2, ") {")) { diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 17a28fbe892..d1337330907 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4532,21 +4532,6 @@ class TestCondition : public TestFixture { " static_assert(static_cast(E::E1) == 1);\n" "}\n"); ASSERT_EQUALS("", errout.str()); - - check("struct a {\n" - " bool g();\n" - " int h();\n" - "};\n" - "void f(a c, int d, int e) {\n" - " if (c.g() && c.h()) {}\n" - " else {\n" - " bool u = false;\n" - " if (d && e)\n" - " u = true;\n" - " if (u) {}\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); } void alwaysTrueSymbolic() diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 315cc73c3d0..b3b9152c541 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -5625,26 +5625,6 @@ class TestValueFlow : public TestFixture { values = tokenValues(code, "x <", ValueFlow::Value::ValueType::UNINIT); ASSERT_EQUALS(0, values.size()); - code = "bool do_something(int *p);\n" - "int getY();\n" - "bool bar();\n" - "void foo() {\n" - " bool flag{true};\n" - " int x;\n" - " int y = getY();\n" - " if (flag == true) {\n" - " flag = bar();\n" - " }\n" - " if ((flag == true) && y > 0) {\n" - " flag = do_something(&x);\n" - " }\n" - " for (int i = 0; (flag == true) && i < y; i++) {\n" - " if (x < 0) {}\n" - " }\n" - "}\n"; - values = tokenValues(code, "x <", ValueFlow::Value::ValueType::UNINIT); - ASSERT_EQUALS(0, values.size()); - code = "void g(bool *result, size_t *buflen) {\n" // #12091 " if (*result && *buflen >= 5) {}\n" // <- *buflen might not be initialized "}\n" @@ -7257,24 +7237,6 @@ class TestValueFlow : public TestFixture { " int& q = (&r)[0];\n" "}\n"; valueOfTok(code, "&"); - - code = "bool a(int *);\n" - "void fn2(int b) {\n" - " if (b) {\n" - " bool c, d, e;\n" - " if (c && d)\n" - " return;\n" - " if (e && a(&b)) {\n" - " }\n" - " }\n" - "}\n"; - valueOfTok(code, "e"); - - code = "void f(int a, int b, int c) {\n" - " if (c && (a || a && b))\n" - " if (a && b) {}\n" - "}\n"; - valueOfTok(code, "a"); } void valueFlowHang() {