From 726c240b9e6575977c0d4464bb6122692b55801c Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 2 Aug 2024 10:18:07 +0200 Subject: [PATCH 1/8] valueflow.cpp: avoid potentially unnecessary evaluation in `parseCompareEachInt()` --- lib/valueflow.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 9799bb3e852..0a7956dbf29 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -287,13 +287,12 @@ static void parseCompareEachInt( return; if (tok->isComparisonOp()) { std::vector value1 = evaluate(tok->astOperand1()); - std::vector value2 = evaluate(tok->astOperand2()); - if (!value1.empty() && !value2.empty()) { - if (tok->astOperand1()->hasKnownIntValue()) - value2.clear(); - if (tok->astOperand2()->hasKnownIntValue()) - value1.clear(); + std::vector value2; + if (value1.empty() || !tok->astOperand1()->hasKnownIntValue()) { + value2 = evaluate(tok->astOperand2()); } + if (!value2.empty() && tok->astOperand2()->hasKnownIntValue()) + value1.clear(); for (const ValueFlow::Value& v1 : value1) { if (isSaturated(v1.intvalue) || astIsFloat(tok->astOperand2(), /*unknown*/ false)) continue; From 8f0f61921f3229021d31781ed7b8b29a70687dd2 Mon Sep 17 00:00:00 2001 From: firewave Date: Sat, 14 Sep 2024 13:56:55 +0200 Subject: [PATCH 2/8] added bailout message TODOs --- lib/astutils.cpp | 26 ++++++++++++++++++++------ lib/checkstl.cpp | 2 +- lib/forwardanalyzer.cpp | 2 +- lib/programmemory.cpp | 7 ++++--- lib/valueflow.cpp | 9 +++++---- lib/vf_analyzers.cpp | 4 ++-- 6 files changed, 33 insertions(+), 17 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 30f10794855..7202fe0b1c7 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -69,7 +69,9 @@ const Token* findExpression(const nonneg int exprid, static int findArgumentPosRecursive(const Token* tok, const Token* tokToFind, bool &found, nonneg int depth=0) { ++depth; - if (!tok || depth >= 100) + if (depth >= 100) + return -1; // TODO: add bailout message + if (!tok) return -1; if (tok->str() == ",") { int res = findArgumentPosRecursive(tok->astOperand1(), tokToFind, found, depth); @@ -112,8 +114,10 @@ templatestr().c_str(), op) == 0) { astFlattenCopy(tok->astOperand1(), op, out, depth); astFlattenCopy(tok->astOperand2(), op, out, depth); @@ -140,8 +144,10 @@ std::vector astFlatten(Token* tok, const char* op) nonneg int astCount(const Token* tok, const char* op, int depth) { --depth; - if (!tok || depth < 0) + if (!tok || depth < 0) { + // TODO: add bailout message return 0; + } if (strcmp(tok->str().c_str(), op) == 0) return astCount(tok->astOperand1(), op, depth) + astCount(tok->astOperand2(), op, depth); return 1; @@ -1118,6 +1124,7 @@ bool exprDependsOnThis(const Token* expr, bool onVar, nonneg int depth) return true; if (depth >= 1000) // Abort recursion to avoid stack overflow + // TODO: add bailout message return true; ++depth; @@ -1260,6 +1267,7 @@ SmallVector followAllReferences(const Token* tok, if (!tok) return {}; if (depth < 0) { + // TODO: add bailout message SmallVector refs_result; refs_result.emplace_back(tok, std::move(errors)); return refs_result; @@ -2891,8 +2899,10 @@ static bool isExpressionChangedAt(const F& getExprTok, const Settings& settings, int depth) { - if (depth < 0) + if (depth < 0) { + // TODO: add bailout message return true; + } if (!isMutableExpression(tok)) return false; if (tok->exprId() != exprid || (!tok->varId() && !tok->isName())) { @@ -2942,8 +2952,10 @@ Token* findVariableChanged(Token *start, const Token *end, int indirect, const n { if (!precedes(start, end)) return nullptr; - if (depth < 0) + if (depth < 0) { + // TODO: add bailout message return start; + } auto getExprTok = utils::memoize([&] { return findExpression(start, exprid); }); @@ -3041,8 +3053,10 @@ static const Token* findExpressionChangedImpl(const Token* expr, int depth, Find find) { - if (depth < 0) + if (depth < 0) { + // TODO: add bailout message return start; + } if (!precedes(start, end)) return nullptr; const Token* result = nullptr; diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index a88591a904a..fefcde93c23 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1087,7 +1087,7 @@ static const ValueFlow::Value* getInnerLifetime(const Token* tok, int depth = 4) { if (depth < 0) - return nullptr; + return nullptr;// TODO: add bailout message if (!tok) return nullptr; for (const ValueFlow::Value& val : tok->values()) { diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index ebdfc70a50d..875ef458d59 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -577,7 +577,7 @@ namespace { Progress updateRange(Token* start, const Token* end, int depth = 20) { if (depth < 0) - return Break(Analyzer::Terminate::Bail); + return Break(Analyzer::Terminate::Bail); // TODO: add bailout message std::size_t i = 0; for (Token* tok = start; precedes(tok, end); tok = tok->next()) { Token* next = nullptr; diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 573a4dfe96d..1538a93da1e 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -1365,7 +1365,7 @@ namespace { nonneg int n = astCount(expr, expr->str().c_str()); if (n > 50) - return unknown(); + return unknown(); // TODO: add bailout message std::vector conditions1 = flattenConditions(expr); if (conditions1.empty()) return unknown(); @@ -1382,7 +1382,7 @@ namespace { } if (condValues.size() == conditions1.size() && allNegated) return negatedValue; - if (n > 4) + if (n > 4) // TODO: add bailout message return unknown(); if (!sortConditions(conditions1)) return unknown(); @@ -1623,6 +1623,7 @@ namespace { return execute(tok); }); if (f) { + // TODO: add bailout message if (fdepth >= 0 && !f->isImplicitlyVirtual()) { ProgramMemory functionState; for (std::size_t i = 0; i < args.size(); ++i) { @@ -1711,7 +1712,7 @@ namespace { depth++; }}; if (depth < 0) - return unknown(); + return unknown(); // TODO: add bailout message ValueFlow::Value v = unknown(); if (updateValue(v, executeImpl(expr))) return v; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 0a7956dbf29..9af15ad6e2b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1215,7 +1215,7 @@ static std::vector minUnsignedValue(const Token* tok, int depth if (!tok) return result; if (depth < 0) - return result; + return result; // TODO: add bailout message if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::INT)) { result = {v->intvalue}; } else if (!Token::Match(tok, "-|%|&|^") && tok->isConstOp() && tok->astOperand1() && tok->astOperand2()) { @@ -1714,7 +1714,7 @@ static std::vector getLifetimeTokens(const Token* tok, if (pred(tok)) return {{tok, std::move(errorPath)}}; if (depth < 0) - return {{tok, std::move(errorPath)}}; + return {{tok, std::move(errorPath)}}; // TODO: add bailout message if (var && var->declarationId() == tok->varId()) { if (var->isReference() || var->isRValueReference()) { const Token * const varDeclEndToken = var->declEndToken(); @@ -2508,7 +2508,7 @@ struct LifetimeStore { static bool hasBorrowingVariables(const std::list& vars, const std::vector& args, int depth = 10) { if (depth < 0) - return true; + return true; // TODO: add bailout message return std::any_of(vars.cbegin(), vars.cend(), [&](const Variable& var) { if (const ValueType* vt = var.valueType()) { if (vt->pointer > 0 && @@ -3778,7 +3778,7 @@ static void valueFlowSymbolic(const TokenList& tokenlist, const SymbolDatabase& static const Token* isStrlenOf(const Token* tok, const Token* expr, int depth = 10) { if (depth < 0) - return nullptr; + return nullptr; // TODO: add bailout message if (!tok) return nullptr; if (!expr) @@ -6266,6 +6266,7 @@ static bool isContainerSizeChangedByFunction(const Token* tok, // Argument not used if (!arg->nameToken()) return false; + // TODO: add bailout message if (depth > 0) return isContainerSizeChanged(arg->nameToken(), scope->bodyStart, diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index 54aeeab3034..2ef0280f4ef 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -118,7 +118,7 @@ struct ValueFlowAnalyzer : Analyzer { if (!tok) return {}; if (depth < 0) - return {}; + return {}; // TODO: add bailout message depth--; if (analyze(tok, Direction::Forward).isRead()) { ConditionState result; @@ -825,7 +825,7 @@ static bool bifurcateVariableChanged(const Variable* var, static bool bifurcate(const Token* tok, const std::set& varids, const Settings& settings, int depth) { if (depth < 0) - return false; + return false; // TODO: add bailout message if (!tok) return true; if (tok->hasKnownIntValue()) From e1c0eb1154f7ef61651a92b370a5acf49ddfdeaa Mon Sep 17 00:00:00 2001 From: firewave Date: Mon, 30 Sep 2024 01:42:45 +0200 Subject: [PATCH 3/8] valueflow.cpp: made some `SymbolicInferModel` members private --- lib/valueflow.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 9af15ad6e2b..1a76db81465 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -3898,10 +3898,10 @@ static void valueFlowSymbolicOperators(const SymbolDatabase& symboldatabase, con } struct SymbolicInferModel : InferModel { - const Token* expr; explicit SymbolicInferModel(const Token* tok) : expr(tok) { assert(expr->exprId() != 0); } +private: bool match(const ValueFlow::Value& value) const override { return value.isSymbolicValue() && value.tokvalue && value.tokvalue->exprId() == expr->exprId(); @@ -3914,6 +3914,7 @@ struct SymbolicInferModel : InferModel { result.setKnown(); return result; } + const Token* expr; }; static void valueFlowSymbolicInfer(const SymbolDatabase& symboldatabase, const Settings& settings) From ec6f68bbd8ca9a6560879edf91bbea2108fed7e6 Mon Sep 17 00:00:00 2001 From: firewave Date: Wed, 25 Dec 2024 16:53:13 +0100 Subject: [PATCH 4/8] valueflow.cpp: refactored `valueFlowForwardLifetime()` to avoid unnecessary operations --- lib/valueflow.cpp | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1a76db81465..9749bf6774e 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2158,11 +2158,9 @@ static void valueFlowForwardLifetime(Token * tok, const TokenList &tokenlist, Er if (!expr) return; - if (expr->exprId() == 0) + if (expr->exprId() <= 0) return; - const Token* endOfVarScope = ValueFlow::getEndOfExprScope(expr); - // Only forward lifetime values std::list values = parent->astOperand2()->values(); values.remove_if(&isNotLifetimeValue); @@ -2175,21 +2173,21 @@ static void valueFlowForwardLifetime(Token * tok, const TokenList &tokenlist, Er // Skip RHS Token* nextExpression = nextAfterAstRightmostLeaf(parent); + const Token* endOfVarScope = ValueFlow::getEndOfExprScope(expr); - if (expr->exprId() > 0) { - valueFlowForward(nextExpression, endOfVarScope->next(), expr, values, tokenlist, errorLogger, settings); + valueFlowForward(nextExpression, endOfVarScope->next(), expr, values, tokenlist, errorLogger, settings); + // TODO: handle `[` + if (Token::simpleMatch(parent->astOperand1(), ".")) { for (ValueFlow::Value& val : values) { if (val.lifetimeKind == ValueFlow::Value::LifetimeKind::Address) val.lifetimeKind = ValueFlow::Value::LifetimeKind::SubObject; } - // TODO: handle `[` - if (Token::simpleMatch(parent->astOperand1(), ".")) { - const Token* parentLifetime = - getParentLifetime(parent->astOperand1()->astOperand2(), settings.library); - if (parentLifetime && parentLifetime->exprId() > 0) { - valueFlowForward(nextExpression, endOfVarScope, parentLifetime, std::move(values), tokenlist, errorLogger, settings); - } + + const Token* parentLifetime = + getParentLifetime(parent->astOperand1()->astOperand2(), settings.library); + if (parentLifetime && parentLifetime->exprId() > 0) { + valueFlowForward(nextExpression, endOfVarScope, parentLifetime, std::move(values), tokenlist, errorLogger, settings); } } // Constructor From 260ae3e1bf175ace67700d926baaddae138e795d Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 7 Jan 2025 20:05:55 +0100 Subject: [PATCH 5/8] ValueFlow: avoid possibly unused assignments in `combineValueProperties()` --- lib/valueflow.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 9749bf6774e..24f766ffd5d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -395,28 +395,28 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val result.tokvalue = value1.tokvalue; else if (value2.tokvalue) result.tokvalue = value2.tokvalue; - if (value1.isSymbolicValue()) { - result.valueType = value1.valueType; - result.tokvalue = value1.tokvalue; - } if (value2.isSymbolicValue()) { result.valueType = value2.valueType; result.tokvalue = value2.tokvalue; } - if (value1.isIteratorValue()) + else if (value1.isSymbolicValue()) { result.valueType = value1.valueType; + result.tokvalue = value1.tokvalue; + } if (value2.isIteratorValue()) result.valueType = value2.valueType; + else if (value1.isIteratorValue()) + result.valueType = value1.valueType; result.condition = value1.condition ? value1.condition : value2.condition; result.varId = (value1.varId != 0) ? value1.varId : value2.varId; result.varvalue = (result.varId == value1.varId) ? value1.varvalue : value2.varvalue; result.errorPath = (value1.errorPath.empty() ? value2 : value1).errorPath; result.safe = value1.safe || value2.safe; if (value1.bound == ValueFlow::Value::Bound::Point || value2.bound == ValueFlow::Value::Bound::Point) { - if (value1.bound == ValueFlow::Value::Bound::Upper || value2.bound == ValueFlow::Value::Bound::Upper) - result.bound = ValueFlow::Value::Bound::Upper; if (value1.bound == ValueFlow::Value::Bound::Lower || value2.bound == ValueFlow::Value::Bound::Lower) result.bound = ValueFlow::Value::Bound::Lower; + else if (value1.bound == ValueFlow::Value::Bound::Upper || value2.bound == ValueFlow::Value::Bound::Upper) + result.bound = ValueFlow::Value::Bound::Upper; } if (value1.path != value2.path) result.path = -1; From 981656a7592bb14c89a260efc423ccfe5a1ff1c1 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 9 Jan 2025 00:36:35 +0100 Subject: [PATCH 6/8] forwardanalyzer.cpp: added TODO --- lib/forwardanalyzer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 875ef458d59..ca2154283c8 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -46,6 +46,7 @@ namespace { struct ForwardTraversal { enum class Progress : std::uint8_t { Continue, Break, Skip }; + // TODO: analyzer is copied ForwardTraversal(const ValuePtr& analyzer, const TokenList& tokenList, ErrorLogger& errorLogger, const Settings& settings) : analyzer(analyzer), tokenList(tokenList), errorLogger(errorLogger), settings(settings) {} From e5113d2902a42db4d031270f173ddd53801ed437 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 9 Jan 2025 00:43:08 +0100 Subject: [PATCH 7/8] valueflow.cpp: switched check order in `getKnownValueFromToken()` --- lib/valueflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 24f766ffd5d..e54cdda6944 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5907,7 +5907,7 @@ static const ValueFlow::Value* getKnownValueFromToken(const Token* tok) if (!tok) return nullptr; auto it = std::find_if(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v) { - return (v.isIntValue() || v.isContainerSizeValue() || v.isFloatValue()) && v.isKnown(); + return v.isKnown() && (v.isIntValue() || v.isContainerSizeValue() || v.isFloatValue()); }); if (it == tok->values().end()) return nullptr; From 158c55863797174d72d7857673d669cf2ad9a3c2 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 10 Jun 2025 13:35:48 +0200 Subject: [PATCH 8/8] programmemory.cpp: avoid unnecessary copy in `ProgramMemoryState::get()` [skip ci] --- lib/programmemory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 1538a93da1e..7d5e05ce8c2 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -580,7 +580,7 @@ ProgramMemory ProgramMemoryState::get(const Token* tok, const Token* ctx, const } else { local.removeModifiedVars(ctx); } - return local.state; + return std::move(local.state); } ProgramMemory getProgramMemory(const Token* tok, const Token* expr, const ValueFlow::Value& value, const Settings& settings)