From 931217b2fc6aabe4a7fa5f9417ec57252a1d2ecb Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 26 Sep 2026 18:00:11 -0500 Subject: [PATCH 1/9] Store multiple values in programmemory --- lib/programmemory.cpp | 478 +++++++++++++++++++++++++++++-------- lib/programmemory.h | 33 ++- lib/token.cpp | 2 +- lib/token.h | 6 + lib/valueflow.cpp | 29 +-- lib/vf_analyzers.cpp | 4 +- test/testnullpointer.cpp | 30 +++ test/testprogrammemory.cpp | 265 ++++++++++++++++++++ 8 files changed, 724 insertions(+), 123 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 142385ac641..7f8e4dd8cba 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -63,12 +63,99 @@ std::size_t ExprIdToken::Hash::operator()(ExprIdToken etok) const return std::hash()(etok.getExpressionId()); } +// Does the value carry its range in intvalue? +static bool isRangeValue(const ValueFlow::Value& value) +{ + return value.isIntValue() || value.isContainerSizeValue() || value.isBufferSizeValue() || value.isIteratorValue(); +} + +// A constraint that is a lower bound: the values up to the bound are impossible +static bool isLowerBound(const ValueFlow::Value& value) +{ + return value.isImpossible() && isRangeValue(value) && value.bound == ValueFlow::Value::Bound::Upper; +} + +// A constraint that is an upper bound: the values from the bound on are impossible +static bool isUpperBound(const ValueFlow::Value& value) +{ + return value.isImpossible() && isRangeValue(value) && value.bound == ValueFlow::Value::Bound::Lower; +} + +// An impossible value of the expression, without a bound +static bool isImpossiblePoint(const ValueFlow::Value& value) +{ + return value.isImpossible() && value.bound == ValueFlow::Value::Bound::Point; +} + +// The smallest value the expression can have according to a lower bound constraint +static MathLib::bigint lowerBound(const ValueFlow::Value& value) +{ + assert(isLowerBound(value)); + return value.intvalue + 1; +} + +// The largest value the expression can have according to an upper bound constraint +static MathLib::bigint upperBound(const ValueFlow::Value& value) +{ + assert(isUpperBound(value)); + return value.intvalue - 1; +} + +// Does the value satisfy the constraint? False when they are not comparable. +static bool satisfies(const ValueFlow::Value& value, const ValueFlow::Value& constraint) +{ + if (value.valueType != constraint.valueType) + return false; + if (isImpossiblePoint(constraint)) + return !value.equalValue(constraint); + if (isLowerBound(constraint)) + return value.intvalue >= lowerBound(constraint); + if (isUpperBound(constraint)) + return value.intvalue <= upperBound(constraint); + return false; +} + +// Add the constraint to the constraints recorded for an expression, merging them the way +// Token::addValue() merges values: duplicates and weaker bounds are dropped, and an impossible value +// next to a bound moves the bound past it. Returns false if nothing changed. +static bool mergeConstraint(ProgramMemory::Values& values, const ValueFlow::Value& value) +{ + ProgramMemory::Values merged = values; + merged.push_back(value); + Token::removeContradictions(merged); + if (merged.size() == values.size() && + std::equal(values.cbegin(), values.cend(), merged.cbegin(), [](const ValueFlow::Value& x, const ValueFlow::Value& y) { + return x == y && x.bound == y.bound; + })) + return false; + values = std::move(merged); + return true; +} + +// Record the value for an expression whose values are given. Returns false if nothing changed. +static bool mergeValue(ProgramMemory::Values& values, const ValueFlow::Value& value) +{ + // A value of the expression, a first value, or a value of another type replaces what is recorded + if (!value.isImpossible() || values.empty() || values.front().valueType != value.valueType) { + if (values.size() == 1 && values.front() == value && values.front().bound == value.bound) + return false; + values.assign(1, value); + return true; + } + if (!values.front().isImpossible()) { + // A value that satisfies the constraint is more precise than the constraint + if (satisfies(values.front(), value)) + return false; + values.assign(1, value); + return true; + } + return mergeConstraint(values, value); +} + void ProgramMemory::setValue(const Token* expr, const ValueFlow::Value& value) { if (!expr) return; - copyOnWrite(); - ValueFlow::Value subvalue = value; const Token* subexpr = solveExprValue( expr, @@ -81,24 +168,57 @@ void ProgramMemory::setValue(const Token* expr, const ValueFlow::Value& value) { return {}; }, subvalue); + + auto record = [&](const Token* tok, const ValueFlow::Value& v) { + const Values* existing = getValues(tok->exprId()); + Values values = existing ? *existing : Values{}; + if (!mergeValue(values, v)) + return; + copyOnWrite(); + (*mValues)[tok] = std::move(values); + }; if (expr != subexpr) - (*mValues)[expr] = value; + record(expr, value); if (subexpr) - (*mValues)[subexpr] = std::move(subvalue); + record(subexpr, std::move(subvalue)); +} + +void ProgramMemory::setValues(const Token* expr, const Values& values) +{ + for (const ValueFlow::Value& value : values) + setValue(expr, value); } const ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid, bool impossible) const +{ + const Values* values = getValues(exprid); + if (!values || values->size() != 1) + return nullptr; + const ValueFlow::Value& value = values->front(); + if (!impossible && value.isImpossible()) + return nullptr; + return &value; +} + +const ProgramMemory::Values* ProgramMemory::getValues(nonneg int exprid) const { const auto it = find(exprid); - const bool found = it != mValues->cend() && (impossible || !it->second.isImpossible()); - if (found) - return &it->second; - return nullptr; + if (it == mValues->cend()) + return nullptr; + return &it->second; +} + +// The value of the expression, if one is recorded (rather than constraints) +static const ValueFlow::Value* getPointValue(const ProgramMemory::Values* values) +{ + if (!values || values->size() != 1 || values->front().isImpossible()) + return nullptr; + return &values->front(); } bool ProgramMemory::getIntValue(nonneg int exprid, MathLib::bigint& result) const { - const ValueFlow::Value* value = getValue(exprid); + const ValueFlow::Value* value = getPointValue(getValues(exprid)); if (value && value->isIntValue()) { result = value->intvalue; return true; @@ -116,7 +236,7 @@ void ProgramMemory::setIntValue(const Token* expr, MathLib::bigint value, bool i bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result) const { - const ValueFlow::Value* value = getValue(exprid); + const ValueFlow::Value* value = getPointValue(getValues(exprid)); if (value && value->isTokValue()) { result = value->tokvalue; return true; @@ -127,27 +247,42 @@ bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result) const // cppcheck-suppress unusedFunction bool ProgramMemory::getContainerSizeValue(nonneg int exprid, MathLib::bigint& result) const { - const ValueFlow::Value* value = getValue(exprid); + const ValueFlow::Value* value = getPointValue(getValues(exprid)); if (value && value->isContainerSizeValue()) { result = value->intvalue; return true; } return false; } -bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint& result) const + +// Is the container empty according to its recorded size values? Unknown if they do not decide it. +static ValueFlow::Value containerEmptyValue(const ProgramMemory::Values& values) { - const ValueFlow::Value* value = getValue(exprid, true); - if (value && value->isContainerSizeValue()) { - if (value->isImpossible() && value->intvalue == 0) { - result = false; - return true; - } - if (!value->isImpossible()) { - result = (value->intvalue == 0); - return true; - } + for (const ValueFlow::Value& value : values) { + if (!value.isContainerSizeValue()) + continue; + if (!value.isImpossible()) + return ValueFlow::Value{value.intvalue == 0}; + if (isLowerBound(value) && lowerBound(value) > 0) + return ValueFlow::Value{0}; + if (isUpperBound(value) && upperBound(value) <= 0) + return ValueFlow::Value{1}; + if (isImpossiblePoint(value) && value.intvalue == 0) + return ValueFlow::Value{0}; } - return false; + return ValueFlow::Value::unknown(); +} + +bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint& result) const +{ + const Values* values = getValues(exprid); + if (!values) + return false; + const ValueFlow::Value empty = containerEmptyValue(*values); + if (empty.isUninitValue()) + return false; + result = empty.intvalue; + return true; } void ProgramMemory::setContainerSizeValue(const Token* expr, MathLib::bigint value, bool equal) @@ -162,7 +297,7 @@ void ProgramMemory::setContainerSizeValue(const Token* expr, MathLib::bigint val void ProgramMemory::setUnknown(const Token* expr) { copyOnWrite(); - (*mValues)[expr].valueType = ValueFlow::Value::ValueType::UNINIT; + (*mValues)[expr].assign(1, ValueFlow::Value::unknown()); } bool ProgramMemory::hasValue(nonneg int exprid) const @@ -171,7 +306,7 @@ bool ProgramMemory::hasValue(nonneg int exprid) const return it != mValues->cend(); } -const ValueFlow::Value& ProgramMemory::at(nonneg int exprid) const { +const ProgramMemory::Values& ProgramMemory::at(nonneg int exprid) const { const auto it = find(exprid); if (it == mValues->cend()) { throw std::out_of_range("ProgramMemory::at"); @@ -179,7 +314,7 @@ const ValueFlow::Value& ProgramMemory::at(nonneg int exprid) const { return it->second; } -ValueFlow::Value& ProgramMemory::at(nonneg int exprid) { +ProgramMemory::Values& ProgramMemory::at(nonneg int exprid) { copyOnWrite(); const auto it = find(exprid); @@ -225,6 +360,12 @@ bool ProgramMemory::empty() const return mValues->empty(); } +// Is the expression recorded as modified with an unknown value? +static bool isUnknown(const ProgramMemory::Values& values) +{ + return !values.empty() && values.front().isUninitValue(); +} + // NOLINTNEXTLINE(performance-unnecessary-value-param) - technically correct but we are moving the given values void ProgramMemory::replace(ProgramMemory pm, bool skipUnknown) { @@ -236,7 +377,7 @@ void ProgramMemory::replace(ProgramMemory pm, bool skipUnknown) for (auto&& p : (*pm.mValues)) { if (skipUnknown) { auto it = mValues->find(p.first); - if (it != mValues->end() && it->second.isUninitValue()) + if (it != mValues->end() && isUnknown(it->second)) continue; } (*mValues)[p.first] = std::move(p.second); @@ -267,6 +408,12 @@ static ValueFlow::Value execute(const Token* expr, const Settings& settings, const ProgramMemory::Map& vars = {}); +// All values of the expression: the constraints of a range, or the single result of execute() +static ProgramMemory::Values executeValues(const Token* expr, + ProgramMemory& pm, + const Settings& settings, + const ProgramMemory::Map& vars = {}); + static bool evaluateCondition(MathLib::bigint r, const Token* condition, ProgramMemory& pm, @@ -304,8 +451,14 @@ static bool isTrue(const ValueFlow::Value& v) { if (v.isUninitValue()) return false; - if (v.isImpossible()) + if (v.isImpossible()) { + // An impossible range excludes zero when zero is on its side of the bound + if (v.bound == ValueFlow::Value::Bound::Upper) + return v.intvalue >= 0; + if (v.bound == ValueFlow::Value::Bound::Lower) + return v.intvalue <= 0; return v.intvalue == 0; + } return v.intvalue != 0; } @@ -381,11 +534,18 @@ static void programMemoryParseCondition(ProgramMemory& pm, if (endTok && changed(vartok, tok->next(), endTok)) return; const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then); - const ValueFlow::Value& v = then ? truevalue : falsevalue; - pm.setValue(vartok, impossible ? asImpossible(v) : v); + ValueFlow::Value v = then ? truevalue : falsevalue; + // A value with a bound is a range: record it as the range of impossible values so that it + // constrains the expression instead of standing in for its value. + if (impossible || v.bound != ValueFlow::Value::Bound::Point) + v = asImpossible(v); + pm.setValue(vartok, v); const Token* containerTok = settings.library.getContainerFromYield(vartok, Library::Container::Yield::SIZE); - if (containerTok) - pm.setContainerSizeValue(containerTok, v.intvalue, !impossible); + if (containerTok) { + ValueFlow::Value size = v; + size.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + pm.setValue(containerTok, size); + } } else if (Token::simpleMatch(tok, "!")) { programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then, findChanged); } else if (then && Token::simpleMatch(tok, "&&")) { @@ -458,7 +618,11 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok const Token* valuetok = tok2->astOperand2(); ProgramMemory local = state; // Tracked values are substituted by execute() when the expression is evaluated. - pm.setValue(vartok, execute(valuetok, local, settings, vars)); + const ProgramMemory::Values values = executeValues(valuetok, local, settings, vars); + if (values.empty()) + pm.setUnknown(vartok); + else + pm.setValues(vartok, values); } } else if (Token::simpleMatch(tok2, ")") && tok2->link() && Token::Match(tok2->link()->previous(), "assert|ASSERT ( !!)")) { @@ -543,10 +707,8 @@ void ProgramMemoryState::replace(ProgramMemory pm, const Token* origin) static void addVars(ProgramMemory& pm, const ProgramMemory::Map& vars) { - for (const auto& p:vars) { - const ValueFlow::Value &value = p.second; - pm.setValue(p.first.tok, value); - } + for (const auto& p:vars) + pm.setValues(p.first.tok, p.second); } void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& vars) @@ -657,7 +819,7 @@ ProgramMemory getProgramMemory(const Token* tok, const Token* expr, const ValueF fillProgramMemoryFromConditions(programMemory, tok, settings); programMemory.setValue(expr, value); const ProgramMemory state = programMemory; - fillProgramMemoryFromAssignments(programMemory, tok, settings, state, {{expr, value}}); + fillProgramMemoryFromAssignments(programMemory, tok, settings, state, {{expr, {value}}}); return programMemory; } @@ -690,12 +852,32 @@ static bool isIntegralValue(const ValueFlow::Value& value) return value.isIntValue() || value.isIteratorValue() || value.isSymbolicValue(); } +static bool isBounded(const ValueFlow::Value& value) +{ + return value.bound != ValueFlow::Value::Bound::Point; +} + static ValueFlow::Value evaluate(const Token* op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs, bool removeAssign = false) { const std::string opStr = removeAssign ? op->str().substr(0, op->str().size() - 1) : op->str(); ValueFlow::Value result; if (lhs.isImpossible() && rhs.isImpossible()) return ValueFlow::Value::unknown(); + // Shifting an impossible range by an int moves its bound along + const ValueFlow::Value& range = isBounded(lhs) ? lhs : rhs; + const ValueFlow::Value& delta = isBounded(lhs) ? rhs : lhs; + if (range.isImpossible() && isBounded(range) && !isBounded(delta) && !delta.isImpossible() && range.isIntValue() && + delta.isIntValue() && contains({"+", "-"}, opStr)) { + bool error = false; + result = range; + result.intvalue = calculate(opStr, lhs.intvalue, rhs.intvalue, &error); + if (error) + return ValueFlow::Value::unknown(); + // c - x reverses the direction of the range + if (isBounded(rhs) && opStr == "-") + result.invertBound(); + return result; + } if (lhs.isImpossible() || rhs.isImpossible()) { // noninvertible if (contains({"%", "/", "&", "|"}, opStr)) @@ -1370,8 +1552,8 @@ namespace { assert(pm != nullptr); } - // Is the tracked value for this expression available? - const ValueFlow::Value* getTrackedValue(const Token* expr) const + // The tracked values for this expression, if there are any + const ProgramMemory::Values* getTrackedValues(const Token* expr) const { if (!vars || expr->exprId() == 0) return nullptr; @@ -1386,10 +1568,27 @@ namespace { if (!vars || vars->empty()) return false; return findAstNode(expr, [&](const Token* tok) { - return getTrackedValue(tok) != nullptr; + return getTrackedValues(tok) != nullptr; }) != nullptr; } + // The one value to read for an expression: its value, or the first of its constraints (every + // one of them holds for the expression) + static ValueFlow::Value representative(const ProgramMemory::Values& values) + { + return values.empty() ? unknown() : values.front(); + } + + // Is the expression read directly from the program memory (no known value, no tracked value)? + const ProgramMemory::Values* getStoredValues(const Token* expr) const + { + if (!expr || expr->exprId() == 0 || expr->hasKnownIntValue()) + return nullptr; + if (dependsOnTrackedValue(expr)) + return nullptr; + return pm->getValues(expr->exprId()); + } + static ValueFlow::Value unknown() { return ValueFlow::Value::unknown(); } @@ -1429,10 +1628,9 @@ namespace { ValueFlow::Value executeMultiCondition(bool b, const Token* expr) { - if (pm->hasValue(expr->exprId())) { - const ValueFlow::Value& v = utils::as_const(*pm).at(expr->exprId()); - if (v.isIntValue()) - return v; + if (const ValueFlow::Value* v = pm->getValue(expr->exprId(), /*impossible*/ true)) { + if (v->isIntValue()) + return *v; } // Evaluate recursively if there are no exprids @@ -1477,7 +1675,9 @@ namespace { const Token* tok = p.first.tok; if (!tok) continue; - const ValueFlow::Value& value = p.second; + if (p.second.size() != 1) + continue; + const ValueFlow::Value& value = p.second.front(); if (tok->str() == expr->str() && !astHasExpr(tok, expr->exprId())) { // TODO: Handle when it is greater @@ -1512,13 +1712,16 @@ namespace { return unknown(); } - // Get the size of the container. If the container itself is not tracked in the program - // memory then check if it is symbolically equal to a container whose size is tracked. - ValueFlow::Value executeContainerSize(const Token* containerTok) + // Get the size values of the container. If the container itself is not tracked in the + // program memory then check if it is symbolically equal to a container whose size is tracked. + ProgramMemory::Values executeContainerSizes(const Token* containerTok) { - ValueFlow::Value v = execute(containerTok); - if (v.isContainerSizeValue()) - return v; + ProgramMemory::Values sizes = executeValues(containerTok); + sizes.remove_if([](const ValueFlow::Value& v) { + return !v.isContainerSizeValue(); + }); + if (!sizes.empty()) + return sizes; for (const ValueFlow::Value& value : containerTok->values()) { if (!value.isSymbolicValue()) continue; @@ -1532,9 +1735,41 @@ namespace { continue; const ValueFlow::Value* sizeValue = pm->getValue(value.tokvalue->exprId()); if (sizeValue && sizeValue->isContainerSizeValue()) - return *sizeValue; + return {*sizeValue}; } - return unknown(); + return {}; + } + + // The container whose size the expression yields, or nullptr + static const Token* getContainerFromSizeYield(const Token* expr) + { + if (!Token::Match(expr->tokAt(-2), ". %name% (") || !astIsContainer(expr->tokAt(-2)->astOperand1())) + return nullptr; + const Token* containerTok = expr->tokAt(-2)->astOperand1(); + const Library::Container::Yield yield = containerTok->valueType()->container->getYield(expr->strAt(-1)); + return yield == Library::Container::Yield::SIZE ? containerTok : nullptr; + } + + // All values of the expression: the constraints of a range read from the program memory, + // or the single result of execute() + ProgramMemory::Values executeValues(const Token* expr) + { + if (!expr) + return {}; + if (const ProgramMemory::Values* stored = getStoredValues(expr)) { + if (stored->size() > 1) + return *stored; + } + if (const Token* containerTok = getContainerFromSizeYield(expr)) { + ProgramMemory::Values sizes = executeContainerSizes(containerTok); + for (ValueFlow::Value& v : sizes) + v.valueType = ValueFlow::Value::ValueType::INT; + return sizes; + } + ValueFlow::Value v = execute(expr); + if (v.isUninitValue()) + return {}; + return {std::move(v)}; } ValueFlow::Value executeImpl(const Token* expr) @@ -1565,20 +1800,16 @@ namespace { const Token* containerTok = expr->tokAt(-2)->astOperand1(); const Library::Container::Yield yield = containerTok->valueType()->container->getYield(expr->strAt(-1)); if (yield == Library::Container::Yield::SIZE) { - ValueFlow::Value v = executeContainerSize(containerTok); + ValueFlow::Value v = representative(executeContainerSizes(containerTok)); if (!v.isContainerSizeValue()) return unknown(); v.valueType = ValueFlow::Value::ValueType::INT; return v; } if (yield == Library::Container::Yield::EMPTY) { - ValueFlow::Value v = executeContainerSize(containerTok); - if (!v.isContainerSizeValue()) - return unknown(); - if (v.isImpossible() && v.intvalue == 0) - return ValueFlow::Value{0}; - if (!v.isImpossible()) - return ValueFlow::Value{v.intvalue == 0}; + ValueFlow::Value v = containerEmptyValue(executeContainerSizes(containerTok)); + if (!v.isUninitValue()) + return v; } } else if (expr->isAssignmentOp() && expr->astOperand1() && expr->astOperand2() && expr->astOperand1()->exprId() > 0) { @@ -1588,16 +1819,22 @@ namespace { if (expr->str() != "=") { if (!pm->hasValue(expr->astOperand1()->exprId())) return unknown(); - ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId()); - rhs = evaluate(expr, lhs, rhs, /*removeAssign*/ true); - if (lhs.isIntValue()) - ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.intvalue), std::placeholders::_1)); - else if (lhs.isFloatValue()) - ValueFlow::Value::visitValue(rhs, - std::bind(assign{}, std::ref(lhs.floatValue), std::placeholders::_1)); - else - return unknown(); - return lhs; + // Apply the operation to every value of the variable + ProgramMemory::Values& lhs = pm->at(expr->astOperand1()->exprId()); + for (ValueFlow::Value& v : lhs) { + const ValueFlow::Value r = evaluate(expr, v, rhs, /*removeAssign*/ true); + if (r.isUninitValue()) { + pm->setUnknown(expr->astOperand1()); + return unknown(); + } + if (v.isIntValue()) + ValueFlow::Value::visitValue(r, std::bind(assign{}, std::ref(v.intvalue), std::placeholders::_1)); + else if (v.isFloatValue()) + ValueFlow::Value::visitValue(r, std::bind(assign{}, std::ref(v.floatValue), std::placeholders::_1)); + else + return unknown(); + } + return representative(lhs); } pm->setValue(expr->astOperand1(), rhs); return rhs; @@ -1611,18 +1848,25 @@ namespace { } else if (expr->tokType() == Token::eIncDecOp && expr->astOperand1() && expr->astOperand1()->exprId() != 0) { if (!pm->hasValue(expr->astOperand1()->exprId())) return ValueFlow::Value::unknown(); - ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId()); - if (!lhs.isIntValue()) + const ProgramMemory::Values& values = utils::as_const(*pm).at(expr->astOperand1()->exprId()); + if (!std::all_of(values.cbegin(), values.cend(), std::mem_fn(&ValueFlow::Value::isIntValue))) return unknown(); // overflow - if (!lhs.isImpossible() && lhs.intvalue == 0 && expr->str() == "--" && astIsUnsigned(expr->astOperand1())) + if (expr->str() == "--" && astIsUnsigned(expr->astOperand1()) && + std::any_of(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) { + return !v.isImpossible() && v.intvalue == 0; + })) return unknown(); - if (expr->str() == "++") - lhs.intvalue++; - else - lhs.intvalue--; - return lhs; + // Shift every value of the variable; bounds and impossible values move along + ProgramMemory::Values& lhs = pm->at(expr->astOperand1()->exprId()); + for (ValueFlow::Value& v : lhs) { + if (expr->str() == "++") + v.intvalue++; + else + v.intvalue--; + } + return representative(lhs); } else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) { const Token* tokvalue = nullptr; if (!pm->getTokValue(expr->astOperand1()->exprId(), tokvalue)) { @@ -1639,7 +1883,7 @@ namespace { } const std::string strValue = tokvalue->strValue(); ValueFlow::Value rhs = execute(expr->astOperand2()); - if (!rhs.isIntValue()) + if (!rhs.isIntValue() || rhs.isImpossible()) return unknown(); const MathLib::bigint index = rhs.intvalue; if (index >= 0 && index < strValue.size()) @@ -1647,12 +1891,22 @@ namespace { if (index == strValue.size()) return ValueFlow::Value{}; } else if (Token::Match(expr, "%cop%") && expr->astOperand1() && expr->astOperand2()) { - ValueFlow::Value lhs = execute(expr->astOperand1()); - if (lhs.isUninitValue()) + const ProgramMemory::Values lhsValues = executeValues(expr->astOperand1()); + if (lhsValues.empty()) return unknown(); - ValueFlow::Value rhs = execute(expr->astOperand2()); - if (rhs.isUninitValue()) + const ProgramMemory::Values rhsValues = executeValues(expr->astOperand2()); + if (rhsValues.empty()) return unknown(); + // Compare ranges: an operand with constraints is compared as the interval they describe + if (expr->isComparisonOp() && + (std::any_of(lhsValues.cbegin(), lhsValues.cend(), std::mem_fn(&ValueFlow::Value::isImpossible)) || + std::any_of(rhsValues.cbegin(), rhsValues.cend(), std::mem_fn(&ValueFlow::Value::isImpossible)))) { + std::vector result = infer(makeIntegralInferModel(), expr->str(), lhsValues, rhsValues); + if (!result.empty()) + return std::move(result.front()); + } + ValueFlow::Value lhs = representative(lhsValues); + ValueFlow::Value rhs = representative(rhsValues); ValueFlow::Value r = evaluate(expr, lhs, rhs); if (expr->isComparisonOp() && (r.isUninitValue() || r.isImpossible())) { if (rhs.isIntValue() && !expr->astOperand1()->values().empty()) { @@ -1691,8 +1945,10 @@ namespace { lhs.setPossible(); lhs.bound = ValueFlow::Value::Bound::Point; } - if (expr->str() == "-") + if (expr->str() == "-") { lhs.intvalue = -lhs.intvalue; + lhs.invertBound(); + } return lhs; } else if (expr->str() == "?" && expr->astOperand1() && expr->astOperand2()) { ValueFlow::Value cond = execute(expr->astOperand1()); @@ -1715,19 +1971,22 @@ namespace { } // Return the tracked value and write it back when it differs, so later reads see the // same value (as fillProgramMemoryFromAssignments used to do). - if (const ValueFlow::Value* tracked = getTrackedValue(expr)) { - const ValueFlow::Value* stored = pm->getValue(expr->exprId(), /*impossible*/ true); + if (const ProgramMemory::Values* tracked = getTrackedValues(expr)) { + const ProgramMemory::Values* stored = pm->getValues(expr->exprId()); if (!stored || *stored != *tracked) - pm->setValue(expr, *tracked); - return *tracked; + pm->setValues(expr, *tracked); + return representative(*tracked); } - if (expr->exprId() > 0 && pm->hasValue(expr->exprId()) && !dependsOnTrackedValue(expr)) { - ValueFlow::Value result = utils::as_const(*pm).at(expr->exprId()); - if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr, settings)) { - result.intvalue = !result.intvalue; + if (const ProgramMemory::Values* stored = getStoredValues(expr)) { + // An impossible value that excludes zero makes the expression true as a bool + if (isUsedAsBool(expr, settings) && std::any_of(stored->cbegin(), stored->cend(), [](const ValueFlow::Value& v) { + return v.isImpossible() && v.isIntValue() && isTrue(v); + })) { + ValueFlow::Value result{1}; result.setKnown(); + return result; } - return result; + return representative(*stored); } if (Token::Match(expr->previous(), ">|%name% {|(")) { @@ -1760,8 +2019,11 @@ namespace { } } else { BuiltinLibraryFunction lf = getBuiltinLibraryFunction(ftok->str()); - if (lf) + // The builtin functions compute with values, not with constraints + if (lf && std::none_of(args.cbegin(), args.cend(), std::mem_fn(&ValueFlow::Value::isImpossible))) return lf(args); + if (lf) + return unknown(); const std::string& returnValue = settings.library.returnValue(ftok); if (!returnValue.empty()) { std::unordered_map arg_map; @@ -1778,13 +2040,14 @@ namespace { // Check if function modifies argument visitAstNodes(expr->astOperand2(), [&](const Token* child) { if (child->exprId() > 0 && pm->hasValue(child->exprId())) { - ValueFlow::Value& v = pm->at(child->exprId()); + // The values of an expression all have the same type + const ValueFlow::Value& v = utils::as_const(*pm).at(child->exprId()).front(); if (v.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) { if (ValueFlow::isContainerSizeChanged(child, v.indirect, settings)) - v = unknown(); + pm->setUnknown(child); } else if (v.valueType != ValueFlow::Value::ValueType::UNINIT) { if (isVariableChanged(child, v.indirect, settings)) - v = unknown(); + pm->setUnknown(child); } } return ChildrenToVisit::op1_and_op2; @@ -1837,7 +2100,7 @@ namespace { if (!expr) return v; if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) { - if (updateValue(v, utils::as_const(*pm).at(expr->exprId()))) + if (updateValue(v, representative(utils::as_const(*pm).at(expr->exprId())))) return v; } // Find symbolic values @@ -1848,10 +2111,9 @@ namespace { continue; if (value.tokvalue->exprId() > 0 && !pm->hasValue(value.tokvalue->exprId())) continue; - const ValueFlow::Value& v_ref = utils::as_const(*pm).at(value.tokvalue->exprId()); - if (!v_ref.isIntValue() && value.intvalue != 0) + ValueFlow::Value v2 = representative(utils::as_const(*pm).at(value.tokvalue->exprId())); + if (!v2.isIntValue() && value.intvalue != 0) continue; - ValueFlow::Value v2 = v_ref; v2.intvalue += value.intvalue; return v2; } @@ -1924,6 +2186,16 @@ static ValueFlow::Value execute(const Token* expr, return ex.execute(expr); } +static ProgramMemory::Values executeValues(const Token* expr, + ProgramMemory& pm, + const Settings& settings, + const ProgramMemory::Map& vars) +{ + Executor ex{&pm, settings}; + ex.vars = &vars; + return ex.executeValues(expr); +} + std::vector execute(const Scope* scope, ProgramMemory& pm, const Settings& settings) { Executor ex{&pm, settings}; diff --git a/lib/programmemory.h b/lib/programmemory.h index de81d0bc901..48fb522e650 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -103,29 +104,55 @@ struct ExprIdToken { }; struct CPPCHECKLIB ProgramMemory { - using Map = std::map; + /** + * The values recorded for one expression. Either a single value of the expression (a possible + * value with a bound is still its value; the bound is extra information about the range it lies + * in) or a set of constraints that hold at the same time: impossible values, where a bound makes + * the value an impossible range, so that "x > 3" is recorded as "values <= 3 are impossible". + * The constraints of one expression all have the same value type. A list, so that references to + * the values stay valid while values are added. + */ + using Values = std::list; + using Map = std::map; ProgramMemory() : mValues(new Map()) {} explicit ProgramMemory(Map values) : mValues(new Map(std::move(values))) {} + /** + * Record a fact about the expression. A value of the expression replaces everything recorded so + * far. A constraint (impossible value) is added to the constraints already recorded, keeping only + * the strongest bound in each direction; it replaces a recorded value only if that value violates it. + */ void setValue(const Token* expr, const ValueFlow::Value& value); + /** setValue() for each of the values */ + void setValues(const Token* expr, const Values& values); + /** + * The single value recorded for the expression, or nullptr if there is none or if several + * constraints are recorded. Impossible values are skipped unless impossible is true. + */ const ValueFlow::Value* getValue(nonneg int exprid, bool impossible = false) const; + /** All values recorded for the expression, or nullptr if there are none */ + const Values* getValues(nonneg int exprid) const; + /** The int value of the expression, if it has one */ bool getIntValue(nonneg int exprid, MathLib::bigint& result) const; void setIntValue(const Token* expr, MathLib::bigint value, bool impossible = false); + /** The container size of the expression, if it has one */ bool getContainerSizeValue(nonneg int exprid, MathLib::bigint& result) const; + /** Is the container empty? Decided from the size or from the recorded size constraints. */ bool getContainerEmptyValue(nonneg int exprid, MathLib::bigint& result) const; void setContainerSizeValue(const Token* expr, MathLib::bigint value, bool equal = true); void setUnknown(const Token* expr); + /** The token value of the expression, if it has one */ bool getTokValue(nonneg int exprid, const Token*& result) const; bool hasValue(nonneg int exprid) const; - const ValueFlow::Value& at(nonneg int exprid) const; - ValueFlow::Value& at(nonneg int exprid); + const Values& at(nonneg int exprid) const; + Values& at(nonneg int exprid); void erase_if(const std::function& pred); diff --git a/lib/token.cpp b/lib/token.cpp index eac69decee7..e20c64fe6b4 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -2198,7 +2198,7 @@ static void removeOverlaps(std::list& values) // Removing contradictions is an NP-hard problem. Instead we run multiple // passes to try to catch most contradictions -static void removeContradictions(std::list& values) +void Token::removeContradictions(std::list& values) { removeOverlaps(values); for (int i = 0; i < 4; i++) { diff --git a/lib/token.h b/lib/token.h index fd4804ec980..96b0a5c1ccd 100644 --- a/lib/token.h +++ b/lib/token.h @@ -1421,6 +1421,12 @@ class CPPCHECKLIB Token { /** Add token value. Return true if value is added. */ bool addValue(const ValueFlow::Value &value); + /** + * Remove the values that contradict each other and merge adjacent ranges, as addValue() does + * after adding a value. + */ + static void removeContradictions(std::list& values); + void removeValues(std::function pred) { if (mImpl->mValues) mImpl->mValues->remove_if(std::move(pred)); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index aa0b2c91c29..e88fcab7a5a 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5485,32 +5485,29 @@ static void valueFlowForLoop(const TokenList &tokenlist, const SymbolDatabase& s } } else { for (const auto& p : mem1) { - if (!p.second.isIntValue()) - continue; - if (p.second.isImpossible()) + MathLib::bigint value = 0; + if (!mem1.getIntValue(p.first.getExpressionId(), value)) continue; if (p.first.tok->varId() == 0) continue; - valueFlowForLoopSimplify(bodyStart, p.first.tok, false, p.second.intvalue, tokenlist, errorLogger, settings); + valueFlowForLoopSimplify(bodyStart, p.first.tok, false, value, tokenlist, errorLogger, settings); } for (const auto& p : mem2) { - if (!p.second.isIntValue()) - continue; - if (p.second.isImpossible()) + MathLib::bigint value = 0; + if (!mem2.getIntValue(p.first.getExpressionId(), value)) continue; if (p.first.tok->varId() == 0) continue; - valueFlowForLoopSimplify(bodyStart, p.first.tok, false, p.second.intvalue, tokenlist, errorLogger, settings); + valueFlowForLoopSimplify(bodyStart, p.first.tok, false, value, tokenlist, errorLogger, settings); } } for (const auto& p : memAfter) { - if (!p.second.isIntValue()) - continue; - if (p.second.isImpossible()) + MathLib::bigint value = 0; + if (!memAfter.getIntValue(p.first.getExpressionId(), value)) continue; if (p.first.tok->varId() == 0) continue; - valueFlowForLoopSimplifyAfter(tok, p.first.getExpressionId(), p.second.intvalue, tokenlist, errorLogger, settings); + valueFlowForLoopSimplifyAfter(tok, p.first.getExpressionId(), value, tokenlist, errorLogger, settings); } } } @@ -6287,9 +6284,11 @@ const Token* ValueFlow::solveExprValue(const Token* expr, return ValueFlow::solveExprValue(binaryTok, eval, value); } case '-': { - if (rhs) + if (rhs) { value.intvalue = intval - value.intvalue; - else + // c - x >= a <=> x <= c - a + value.invertBound(); + } else value.intvalue += intval; return ValueFlow::solveExprValue(binaryTok, eval, value); } @@ -6297,6 +6296,8 @@ const Token* ValueFlow::solveExprValue(const Token* expr, if (intval == 0) break; value.intvalue /= intval; + if (intval < 0) + value.invertBound(); return ValueFlow::solveExprValue(binaryTok, eval, value); } case '^': { diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index 1a4d17aa849..f2f1981dd99 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -1104,7 +1104,7 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer { if (!var) continue; assert(var->nameToken()); - ps[var->nameToken()] = p.second; + ps[var->nameToken()] = {p.second}; } return ps; } @@ -1379,7 +1379,7 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { ProgramState getProgramState() const override { ProgramState ps; - ps[expr] = value; + ps[expr] = {value}; return ps; } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index ca5c183c911..4f5741b99cd 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -148,6 +148,7 @@ class TestNullPointer : public TestFixture { TEST_CASE(nullpointer108); TEST_CASE(nullpointer109); TEST_CASE(nullpointer110); // #14937 + TEST_CASE(nullpointer111); // ranges from conditions TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -3147,6 +3148,35 @@ class TestNullPointer : public TestFixture { ASSERT_EQUALS("", errout_str()); } + void nullpointer111() { // a condition 'x > 3' does not give x the value 4 + check("void f(int x) {\n" + " int* p = 0;\n" + " if (x > 3) {\n" + " if (x < 10) {}\n" + " else { *p = 1; }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5:17]: (error) Null pointer dereference: p [nullPointer]\n", errout_str()); + + check("void f(int x) {\n" + " int* p = 0;\n" + " if (x > 3) {\n" + " if (x == 15) { *p = 1; }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4:25]: (error) Null pointer dereference: p [nullPointer]\n", errout_str()); + + // 3 < x < 10: x == 15 is impossible, x == 5 is not + check("void f(int x) {\n" + " int* p = 0;\n" + " if (x > 3 && x < 10) {\n" + " if (x == 15) { *p = 1; }\n" + " if (x == 5) { *p = 1; }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5:24]: (error) Null pointer dereference: p [nullPointer]\n", errout_str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n" diff --git a/test/testprogrammemory.cpp b/test/testprogrammemory.cpp index c37431de608..f5d5713da7a 100644 --- a/test/testprogrammemory.cpp +++ b/test/testprogrammemory.cpp @@ -19,23 +19,64 @@ #include "config.h" #include "fixture.h" #include "helpers.h" +#include "mathlib.h" +#include "settings.h" #include "token.h" #include "programmemory.h" #include "utils.h" #include "vfvalue.h" +#include +#include #include +#include +#include class TestProgramMemory : public TestFixture { public: TestProgramMemory() : TestFixture("TestProgramMemory") {} private: + const Settings settings = settingsBuilder().library("std.cfg").build(); + void run() override { TEST_CASE(copyOnWrite); TEST_CASE(hasValue); TEST_CASE(getValue); TEST_CASE(at); + TEST_CASE(setValueConstraints); + TEST_CASE(setValueReplacesConstraints); + TEST_CASE(containerEmpty); + TEST_CASE(executeRange); + TEST_CASE(executeContainerSizeRange); + } + + static ValueFlow::Value impossible(MathLib::bigint x, ValueFlow::Value::Bound bound = ValueFlow::Value::Bound::Point) { + ValueFlow::Value v{x, bound}; + v.setImpossible(); + return v; + } + + // The constraint "x > lower": the values up to lower are impossible + static ValueFlow::Value greaterThan(MathLib::bigint lower) { + return impossible(lower, ValueFlow::Value::Bound::Upper); + } + + // The constraint "x < upper": the values from upper on are impossible + static ValueFlow::Value lessThan(MathLib::bigint upper) { + return impossible(upper, ValueFlow::Value::Bound::Lower); + } + + static ValueFlow::Value containerSize(ValueFlow::Value v) { + v.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + return v; + } + + static const ValueFlow::Value* findValue(const ProgramMemory::Values& values, MathLib::bigint x, ValueFlow::Value::Bound bound) { + const auto it = std::find_if(values.cbegin(), values.cend(), [&](const ValueFlow::Value& v) { + return v.intvalue == x && v.bound == bound; + }); + return it == values.cend() ? nullptr : &*it; } void copyOnWrite() const { @@ -94,6 +135,7 @@ class TestProgramMemory : public TestFixture { void getValue() const { ProgramMemory pm; ASSERT(!pm.getValue(123)); + ASSERT(!pm.getValues(123)); } void at() const { @@ -101,6 +143,229 @@ class TestProgramMemory : public TestFixture { ASSERT_THROW_EQUALS(pm.at(123), std::out_of_range, "ProgramMemory::at"); ASSERT_THROW_EQUALS(utils::as_const(pm).at(123), std::out_of_range, "ProgramMemory::at"); } + + void setValueConstraints() const { + SimpleTokenList tokenlist("1+1;\n"); + Token* tok = tokenlist.front(); + const nonneg int id = 123; + tok->exprId(id); + + ProgramMemory pm; + // x > 3 and x < 10 hold at the same time + pm.setValue(tok, greaterThan(3)); + pm.setValue(tok, lessThan(10)); + const ProgramMemory::Values* values = pm.getValues(id); + ASSERT(values); + ASSERT_EQUALS(2U, values->size()); + ASSERT(findValue(*values, 3, ValueFlow::Value::Bound::Upper)); + ASSERT(findValue(*values, 10, ValueFlow::Value::Bound::Lower)); + + // several constraints are not a single value + ASSERT(!pm.getValue(id)); + ASSERT(!pm.getValue(id, true)); + MathLib::bigint i = 0; + ASSERT(!pm.getIntValue(id, i)); + + // a repeated constraint is not added again + pm.setValue(tok, greaterThan(3)); + ASSERT_EQUALS(2U, pm.at(id).size()); + + // a weaker bound is dropped + pm.setValue(tok, greaterThan(1)); + ASSERT_EQUALS(2U, pm.at(id).size()); + ASSERT(findValue(pm.at(id), 3, ValueFlow::Value::Bound::Upper)); + + // a stronger bound replaces the bound + pm.setValue(tok, greaterThan(5)); + ASSERT_EQUALS(2U, pm.at(id).size()); + ASSERT(findValue(pm.at(id), 5, ValueFlow::Value::Bound::Upper)); + ASSERT(!findValue(pm.at(id), 3, ValueFlow::Value::Bound::Upper)); + + // an impossible value inside the range is kept + pm.setValue(tok, impossible(7)); + ASSERT_EQUALS(3U, pm.at(id).size()); + ASSERT(findValue(pm.at(id), 7, ValueFlow::Value::Bound::Point)); + + // x > 5 and x != 6 is x > 6, and then x != 7 makes it x > 7 + pm.setValue(tok, impossible(6)); + ASSERT_EQUALS(2U, pm.at(id).size()); + ASSERT(findValue(pm.at(id), 7, ValueFlow::Value::Bound::Upper)); + ASSERT(findValue(pm.at(id), 10, ValueFlow::Value::Bound::Lower)); + } + + void setValueReplacesConstraints() const { + SimpleTokenList tokenlist("1+1;\n"); + Token* tok = tokenlist.front(); + const nonneg int id = 123; + tok->exprId(id); + + ProgramMemory pm; + pm.setValue(tok, greaterThan(3)); + pm.setValue(tok, lessThan(10)); + + // a value of the expression replaces its constraints + pm.setValue(tok, ValueFlow::Value{5}); + MathLib::bigint i = 0; + ASSERT(pm.getIntValue(id, i)); + ASSERT_EQUALS(5, i); + ASSERT_EQUALS(1U, pm.at(id).size()); + + // a constraint the value satisfies keeps the value + pm.setValue(tok, greaterThan(3)); + pm.setValue(tok, impossible(7)); + ASSERT(pm.getIntValue(id, i)); + ASSERT_EQUALS(5, i); + + // a constraint the value violates replaces the value + pm.setValue(tok, impossible(5)); + ASSERT(!pm.getIntValue(id, i)); + ASSERT_EQUALS(1U, pm.at(id).size()); + ASSERT(pm.at(id).front().isImpossible()); + + // a possible value with a bound is a value of the expression + pm.setValue(tok, ValueFlow::Value{4, ValueFlow::Value::Bound::Lower}); + ASSERT(pm.getIntValue(id, i)); + ASSERT_EQUALS(4, i); + + // a value of another type replaces the value + pm.setValue(tok, containerSize(ValueFlow::Value{3})); + ASSERT(!pm.getIntValue(id, i)); + ASSERT(pm.getContainerSizeValue(id, i)); + ASSERT_EQUALS(3, i); + + pm.setUnknown(tok); + ASSERT(pm.hasValue(id)); + ASSERT(pm.getValue(id)); + ASSERT(pm.getValue(id)->isUninitValue()); + } + + void containerEmpty() const { + SimpleTokenList tokenlist("1+1;\n"); + Token* tok = tokenlist.front(); + const nonneg int id = 123; + tok->exprId(id); + + ProgramMemory pm; + MathLib::bigint empty = -1; + ASSERT(!pm.getContainerEmptyValue(id, empty)); + + pm.setContainerSizeValue(tok, 0); + ASSERT(pm.getContainerEmptyValue(id, empty)); + ASSERT_EQUALS(1, empty); + + pm.setContainerSizeValue(tok, 3); + ASSERT(pm.getContainerEmptyValue(id, empty)); + ASSERT_EQUALS(0, empty); + + // size != 0 + pm.clear(); + pm.setContainerSizeValue(tok, 0, false); + ASSERT(pm.getContainerEmptyValue(id, empty)); + ASSERT_EQUALS(0, empty); + + // size > 2 + pm.clear(); + pm.setValue(tok, containerSize(greaterThan(2))); + ASSERT(pm.getContainerEmptyValue(id, empty)); + ASSERT_EQUALS(0, empty); + + // size < 1 + pm.clear(); + pm.setValue(tok, containerSize(lessThan(1))); + ASSERT(pm.getContainerEmptyValue(id, empty)); + ASSERT_EQUALS(1, empty); + + // size < 5 does not decide it + pm.clear(); + pm.setValue(tok, containerSize(lessThan(5))); + ASSERT(!pm.getContainerEmptyValue(id, empty)); + } + + // Remove the values ValueFlow attached to the tokens, so that only the program memory decides + static void clearValues(SimpleTokenizer& tokenizer) { + for (Token* tok = tokenizer.list.front(); tok; tok = tok->next()) + tok->clearValueFlow(); + } + + // The right hand sides of the assignments to the variable, in order + static std::vector assignedExpressions(const Token* tokens, const std::string& var) { + std::vector result; + for (const Token* tok = tokens; tok; tok = tok->next()) { + if (tok->str() == "=" && tok->astOperand1() && tok->astOperand1()->str() == var && tok->astOperand2()) + result.push_back(tok->astOperand2()); + } + return result; + } + + // Evaluate the expression with the program memory built from the conditions enclosing it. + // The result as a string, empty if it is unknown. + std::string evaluate(const Token* expr) const { + ProgramMemoryState pms(settings); + pms.addState(expr, {}); + ProgramMemory pm = pms.state; + MathLib::bigint result = 0; + bool error = false; + execute(expr, pm, &result, &error, settings); + if (error) + return ""; + return std::to_string(result); + } + + void executeRange() { + const char code[] = "void f(int x, int y) {\n" + " if (x > 3) {\n" + " if (x < 10) {\n" + " y = x == 15;\n" + " y = x == 5;\n" + " y = x < 20;\n" + " y = x >= 4;\n" + " y = x + 1 > 4;\n" + " y = 10 - x < 7;\n" + " y = -x < 0;\n" + " y = x;\n" + " }\n" + " }\n" + "}\n"; + SimpleTokenizer tokenizer(settings, *this); + ASSERT(tokenizer.tokenize(code)); + clearValues(tokenizer); + const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); + ASSERT_EQUALS(8U, exprs.size()); + // 3 < x < 10 + ASSERT_EQUALS("0", evaluate(exprs[0])); + ASSERT_EQUALS("", evaluate(exprs[1])); + ASSERT_EQUALS("1", evaluate(exprs[2])); + ASSERT_EQUALS("1", evaluate(exprs[3])); + // the range is shifted by arithmetic + ASSERT_EQUALS("1", evaluate(exprs[4])); + ASSERT_EQUALS("1", evaluate(exprs[5])); + ASSERT_EQUALS("1", evaluate(exprs[6])); + // a range is not a value + ASSERT_EQUALS("", evaluate(exprs[7])); + } + + void executeContainerSizeRange() { + const char code[] = "void f(std::string s, bool y) {\n" + " if (s.size() > 3) {\n" + " if (s.size() < 10) {\n" + " y = s.size() == 15;\n" + " y = s.size() < 20;\n" + " y = s.empty();\n" + " y = s.size() == 5;\n" + " }\n" + " }\n" + "}\n"; + SimpleTokenizer tokenizer(settings, *this); + ASSERT(tokenizer.tokenize(code)); + clearValues(tokenizer); + const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); + ASSERT_EQUALS(4U, exprs.size()); + // 3 < s.size() < 10 + ASSERT_EQUALS("0", evaluate(exprs[0])); + ASSERT_EQUALS("1", evaluate(exprs[1])); + ASSERT_EQUALS("0", evaluate(exprs[2])); + ASSERT_EQUALS("", evaluate(exprs[3])); + } }; REGISTER_TEST(TestProgramMemory) From 96917e7188ca6ce726fe126a46c4306ff2f7dff4 Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 26 Sep 2026 18:19:36 -0500 Subject: [PATCH 2/9] Add record --- lib/programmemory.cpp | 20 +++++++++++--------- lib/programmemory.h | 2 ++ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 7f8e4dd8cba..ea7c60573d0 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -169,18 +169,20 @@ void ProgramMemory::setValue(const Token* expr, const ValueFlow::Value& value) { }, subvalue); - auto record = [&](const Token* tok, const ValueFlow::Value& v) { - const Values* existing = getValues(tok->exprId()); - Values values = existing ? *existing : Values{}; - if (!mergeValue(values, v)) - return; - copyOnWrite(); - (*mValues)[tok] = std::move(values); - }; if (expr != subexpr) record(expr, value); if (subexpr) - record(subexpr, std::move(subvalue)); + record(subexpr, subvalue); +} + +void ProgramMemory::record(const Token* expr, const ValueFlow::Value& value) +{ + const Values* existing = getValues(expr->exprId()); + Values values = existing ? *existing : Values{}; + if (!mergeValue(values, value)) + return; + copyOnWrite(); + (*mValues)[expr] = std::move(values); } void ProgramMemory::setValues(const Token* expr, const Values& values) diff --git a/lib/programmemory.h b/lib/programmemory.h index 48fb522e650..276a29126e8 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -181,6 +181,8 @@ struct CPPCHECKLIB ProgramMemory { } private: + /** Record the value for exactly this expression, without solving the expression */ + void record(const Token* expr, const ValueFlow::Value& value); void copyOnWrite(); Map::const_iterator find(nonneg int exprid) const; Map::iterator find(nonneg int exprid); From 0e9180f28799581d7b4b806ca01e4fcbbcbe640b Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 27 Sep 2026 15:14:16 -0500 Subject: [PATCH 3/9] Add some more testing and fixes --- lib/programmemory.cpp | 99 +++++++++++++++++++++++----- lib/valueflow.cpp | 53 +++++++++++++-- test/testnullpointer.cpp | 66 +++++++++++++++++++ test/testprogrammemory.cpp | 131 ++++++++++++++++++++++++++++++++++++- test/testvalueflow.cpp | 129 ++++++++++++++++++++++++++++++++++++ 5 files changed, 453 insertions(+), 25 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index ea7c60573d0..c5c28adf8d9 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -35,8 +35,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -859,30 +861,82 @@ static bool isBounded(const ValueFlow::Value& value) return value.bound != ValueFlow::Value::Bound::Point; } +static bool isSaturated(MathLib::bigint value) +{ + return value == std::numeric_limits::max() || value == std::numeric_limits::min(); +} + +static bool multiplyOverflows(MathLib::bigint x, MathLib::bigint y) +{ + if (x == 0 || y == 0) + return false; + if (isSaturated(x) || isSaturated(y)) + return true; + return std::abs(x) > std::numeric_limits::max() / std::abs(y); +} + +// The range of "x * k", "x / k" or "x << k" when the values of x up to (or from) the bound are +// impossible: the end of the range is transformed, and a negative factor turns the range around. +static ValueFlow::Value scaleRange(const ValueFlow::Value& range, const std::string& op, MathLib::bigint k) +{ + const bool lower = isLowerBound(range); + const MathLib::bigint edge = lower ? lowerBound(range) : upperBound(range); + if (k == 0 || isSaturated(edge)) + return ValueFlow::Value::unknown(); + MathLib::bigint scaled = 0; + bool lowerAfter = lower; + if (op == "*") { + if (multiplyOverflows(edge, k)) + return ValueFlow::Value::unknown(); + scaled = edge * k; + lowerAfter = (k > 0) == lower; + } else if (op == "/") { + // Truncation towards zero keeps the order of the values + scaled = edge / k; + lowerAfter = (k > 0) == lower; + } else if (op == "<<") { + if (k < 0 || k >= 63 || edge < 0 || edge > (std::numeric_limits::max() >> k)) + return ValueFlow::Value::unknown(); + scaled = edge << k; + } else { + return ValueFlow::Value::unknown(); + } + ValueFlow::Value result = range; + result.intvalue = lowerAfter ? scaled - 1 : scaled + 1; + result.bound = lowerAfter ? ValueFlow::Value::Bound::Upper : ValueFlow::Value::Bound::Lower; + return result; +} + static ValueFlow::Value evaluate(const Token* op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs, bool removeAssign = false) { const std::string opStr = removeAssign ? op->str().substr(0, op->str().size() - 1) : op->str(); ValueFlow::Value result; if (lhs.isImpossible() && rhs.isImpossible()) return ValueFlow::Value::unknown(); - // Shifting an impossible range by an int moves its bound along + // An impossible range combined with an int: shifting moves the bound along, scaling transforms it const ValueFlow::Value& range = isBounded(lhs) ? lhs : rhs; const ValueFlow::Value& delta = isBounded(lhs) ? rhs : lhs; if (range.isImpossible() && isBounded(range) && !isBounded(delta) && !delta.isImpossible() && range.isIntValue() && - delta.isIntValue() && contains({"+", "-"}, opStr)) { - bool error = false; - result = range; - result.intvalue = calculate(opStr, lhs.intvalue, rhs.intvalue, &error); - if (error) - return ValueFlow::Value::unknown(); - // c - x reverses the direction of the range - if (isBounded(rhs) && opStr == "-") - result.invertBound(); - return result; + delta.isIntValue()) { + if (contains({"+", "-"}, opStr)) { + bool error = false; + result = range; + result.intvalue = calculate(opStr, lhs.intvalue, rhs.intvalue, &error); + if (error) + return ValueFlow::Value::unknown(); + // c - x reverses the direction of the range + if (isBounded(rhs) && opStr == "-") + result.invertBound(); + return result; + } + if (opStr == "*" || (isBounded(lhs) && contains({"/", "<<"}, opStr))) + return scaleRange(range, opStr, delta.intvalue); } if (lhs.isImpossible() || rhs.isImpossible()) { // noninvertible - if (contains({"%", "/", "&", "|"}, opStr)) + if (contains({"%", "/", "&", "|", ">>"}, opStr)) + return ValueFlow::Value::unknown(); + if (opStr == "*" && (lhs.equalTo(0) || rhs.equalTo(0))) return ValueFlow::Value::unknown(); result.setImpossible(); } @@ -1835,6 +1889,8 @@ namespace { ValueFlow::Value::visitValue(r, std::bind(assign{}, std::ref(v.floatValue), std::placeholders::_1)); else return unknown(); + // The operation may have turned the range around or dissolved it + v.bound = r.bound; } return representative(lhs); } @@ -1853,12 +1909,19 @@ namespace { const ProgramMemory::Values& values = utils::as_const(*pm).at(expr->astOperand1()->exprId()); if (!std::all_of(values.cbegin(), values.cend(), std::mem_fn(&ValueFlow::Value::isIntValue))) return unknown(); - // overflow - if (expr->str() == "--" && astIsUnsigned(expr->astOperand1()) && - std::any_of(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) { - return !v.isImpossible() && v.intvalue == 0; - })) - return unknown(); + // An unsigned value wraps around when zero is decremented + if (expr->str() == "--" && astIsUnsigned(expr->astOperand1())) { + const bool zero = std::any_of(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) { + return !v.isImpossible() && v.intvalue == 0; + }); + const bool excludesZero = std::any_of(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) { + return (isLowerBound(v) && lowerBound(v) >= 1) || (isImpossiblePoint(v) && v.intvalue == 0); + }); + if (zero || (values.front().isImpossible() && !excludesZero)) { + pm->setUnknown(expr->astOperand1()); + return unknown(); + } + } // Shift every value of the variable; bounds and impossible values move along ProgramMemory::Values& lhs = pm->at(expr->astOperand1()->exprId()); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index e88fcab7a5a..0eba32eafab 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -6261,6 +6261,43 @@ static const Token* parseBinaryIntOp(const Token* expr, return varTok; } +static MathLib::bigint floorDiv(MathLib::bigint x, MathLib::bigint y) +{ + MathLib::bigint q = x / y; + if (x % y != 0 && (x < 0) != (y < 0)) + --q; + return q; +} + +static MathLib::bigint ceilDiv(MathLib::bigint x, MathLib::bigint y) +{ + MathLib::bigint q = x / y; + if (x % y != 0 && (x < 0) == (y < 0)) + ++q; + return q; +} + +// Solve "x * divisor" for x when the value is a bound: divide the end of the range, rounding towards +// the inside of the range so that it stays exact, and turn the range around for a negative divisor. +static void divideBound(ValueFlow::Value& value, MathLib::bigint divisor) +{ + // Is the value the lower end of the range? A possible lower bound is, and so is an impossible + // upper bound, as the values up to it are impossible. + const bool lower = (value.bound == ValueFlow::Value::Bound::Lower) != value.isImpossible(); + // The end of the range: the first value that is possible + MathLib::bigint edge = value.intvalue; + if (value.isImpossible()) + edge += lower ? 1 : -1; + const bool lowerAfter = (divisor > 0) == lower; + edge = lowerAfter ? ceilDiv(edge, divisor) : floorDiv(edge, divisor); + if (divisor < 0) + value.invertBound(); + if (value.isImpossible()) + value.intvalue = lowerAfter ? edge - 1 : edge + 1; + else + value.intvalue = edge; +} + const Token* ValueFlow::solveExprValue(const Token* expr, const std::function(const Token*)>& eval, ValueFlow::Value& value) @@ -6293,14 +6330,22 @@ const Token* ValueFlow::solveExprValue(const Token* expr, return ValueFlow::solveExprValue(binaryTok, eval, value); } case '*': { - if (intval == 0) + if (intval == 0 || isSaturated(value.intvalue)) break; - value.intvalue /= intval; - if (intval < 0) - value.invertBound(); + if (value.bound == ValueFlow::Value::Bound::Point) { + // x * k is v only for a v that k divides + if (value.intvalue % intval != 0) + break; + value.intvalue /= intval; + } else { + divideBound(value, intval); + } return ValueFlow::solveExprValue(binaryTok, eval, value); } case '^': { + // xor does not keep a range together + if (value.bound != ValueFlow::Value::Bound::Point) + break; value.intvalue ^= intval; return ValueFlow::solveExprValue(binaryTok, eval, value); } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 4f5741b99cd..0da1dbb6982 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -3175,6 +3175,72 @@ class TestNullPointer : public TestFixture { " }\n" "}\n"); ASSERT_EQUALS("[test.cpp:5:24]: (error) Null pointer dereference: p [nullPointer]\n", errout_str()); + + // the range of a product is solved exactly: -2 * x > 3 is x <= -2 + check("void f(int x) {\n" + " int* p = 0;\n" + " if (-2 * x > 3) {\n" + " if (x <= -2) p = &x;\n" + " *p = 1;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + // x * 2 < 3 is x <= 1, so x == 1 is possible + check("void f(int x) {\n" + " int* p = 0;\n" + " if (x * 2 < 3) {\n" + " if (x == 1) p = &x;\n" + " *p = 1;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5:10]: (warning) Possible null pointer dereference: p [nullPointer]\n", errout_str()); + + // the range follows the value through arithmetic + check("void f(int x) {\n" + " int* p = 0;\n" + " if (x > 3) {\n" + " if ((x << 1) >= 8) p = &x;\n" + " *p = 1;\n" + " }\n" + " if (x > 6) {\n" + " if (x / 2 > 2) p = &x;\n" + " *p = 1;\n" + " }\n" + " if (x > 3) {\n" + " if (10 - x < 7) p = &x;\n" + " *p = 1;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + // a range that excludes zero is true; one that includes zero is not known + check("void f(int x) {\n" + " int* p = 0;\n" + " if (x > 3) {\n" + " if (x) p = &x;\n" + " *p = 1;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + check("void f(int x) {\n" + " int* p = 0;\n" + " if (x > -5) {\n" + " if (x) p = &x;\n" + " *p = 1;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5:10]: (warning) Possible null pointer dereference: p [nullPointer]\n", errout_str()); + + // x >= 0 and x != 0 is x > 0 + check("void f(int x) {\n" + " int* p = 0;\n" + " if (x < 0) return;\n" + " if (x == 0) return;\n" + " if (x > 0) p = &x;\n" + " *p = 1;\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); } void nullpointer_addressOf() { // address of diff --git a/test/testprogrammemory.cpp b/test/testprogrammemory.cpp index f5d5713da7a..32a394ee96e 100644 --- a/test/testprogrammemory.cpp +++ b/test/testprogrammemory.cpp @@ -48,6 +48,9 @@ class TestProgramMemory : public TestFixture { TEST_CASE(setValueReplacesConstraints); TEST_CASE(containerEmpty); TEST_CASE(executeRange); + TEST_CASE(executeScaledRange); + TEST_CASE(executeSolvedRange); + TEST_CASE(executeCompoundAssignment); TEST_CASE(executeContainerSizeRange); } @@ -281,10 +284,13 @@ class TestProgramMemory : public TestFixture { ASSERT(!pm.getContainerEmptyValue(id, empty)); } - // Remove the values ValueFlow attached to the tokens, so that only the program memory decides + // Remove the values ValueFlow attached to the tokens, so that only the program memory decides. + // Numbers keep their value, as they always have it. static void clearValues(SimpleTokenizer& tokenizer) { - for (Token* tok = tokenizer.list.front(); tok; tok = tok->next()) - tok->clearValueFlow(); + for (Token* tok = tokenizer.list.front(); tok; tok = tok->next()) { + if (!tok->isNumber()) + tok->clearValueFlow(); + } } // The right hand sides of the assignments to the variable, in order @@ -344,6 +350,125 @@ class TestProgramMemory : public TestFixture { ASSERT_EQUALS("", evaluate(exprs[7])); } + void executeScaledRange() { + const char code[] = "void f(int x, int y) {\n" + " if (x > 3) {\n" + " y = x * 2 > 6;\n" + " y = x * 2 == 7;\n" + " y = -2 * x < -6;\n" + " y = x * 0 == 0;\n" + " y = (x << 1) >= 8;\n" + " y = x % 2 == 0;\n" + " y = (x >> 1) == 1;\n" + " }\n" + " if (x > 6) {\n" + " y = x / 2 > 2;\n" + " y = x / -2 < -2;\n" + " }\n" + "}\n"; + SimpleTokenizer tokenizer(settings, *this); + ASSERT(tokenizer.tokenize(code)); + clearValues(tokenizer); + const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); + ASSERT_EQUALS(9U, exprs.size()); + // x > 3: x * 2 >= 8 + ASSERT_EQUALS("1", evaluate(exprs[0])); + ASSERT_EQUALS("0", evaluate(exprs[1])); + ASSERT_EQUALS("1", evaluate(exprs[2])); + // x * 0 is not "not zero" + ASSERT_EQUALS("", evaluate(exprs[3])); + ASSERT_EQUALS("1", evaluate(exprs[4])); + // remainder and right shift do not keep the range + ASSERT_EQUALS("", evaluate(exprs[5])); + ASSERT_EQUALS("", evaluate(exprs[6])); + // x > 6: x / 2 >= 3 + ASSERT_EQUALS("1", evaluate(exprs[7])); + ASSERT_EQUALS("1", evaluate(exprs[8])); + } + + void executeSolvedRange() { + const char code[] = "void f(int x, int y) {\n" + " if (x * 2 < 3) {\n" + " y = x <= 1;\n" + " y = x == 1;\n" + " y = x == 2;\n" + " }\n" + " if (x * 3 >= 7) {\n" + " y = x >= 3;\n" + " y = x == 2;\n" + " }\n" + " if (-2 * x > 3) {\n" + " y = x <= -2;\n" + " y = x == -1;\n" + " }\n" + " if ((x ^ 4) > 3) {\n" + " y = x == 0;\n" + " }\n" + "}\n"; + SimpleTokenizer tokenizer(settings, *this); + ASSERT(tokenizer.tokenize(code)); + clearValues(tokenizer); + const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); + ASSERT_EQUALS(8U, exprs.size()); + // x * 2 < 3: x <= 1 + ASSERT_EQUALS("1", evaluate(exprs[0])); + ASSERT_EQUALS("", evaluate(exprs[1])); + ASSERT_EQUALS("0", evaluate(exprs[2])); + // x * 3 >= 7: x >= 3 + ASSERT_EQUALS("1", evaluate(exprs[3])); + ASSERT_EQUALS("0", evaluate(exprs[4])); + // -2 * x > 3: x <= -2 + ASSERT_EQUALS("1", evaluate(exprs[5])); + ASSERT_EQUALS("0", evaluate(exprs[6])); + // (x ^ 4) > 3 does not give a range for x + ASSERT_EQUALS("", evaluate(exprs[7])); + } + + void executeCompoundAssignment() { + const char code[] = "void f(int x, unsigned u, int y) {\n" + " x *= -1;\n" + " y = x < -3;\n" + " u--;\n" + " y = u > 100;\n" + "}\n"; + SimpleTokenizer tokenizer(settings, *this); + ASSERT(tokenizer.tokenize(code)); + clearValues(tokenizer); + const Token* xtok = Token::findsimplematch(tokenizer.tokens(), "x *="); + const Token* utok = Token::findsimplematch(tokenizer.tokens(), "u --"); + ASSERT(xtok && utok); + const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); + ASSERT_EQUALS(2U, exprs.size()); + + ProgramMemory pm; + // x > 3, then x *= -1: x < -3 + pm.setValue(xtok, greaterThan(3)); + execute(xtok->next(), pm, nullptr, nullptr, settings); + MathLib::bigint result = 0; + bool error = false; + execute(exprs[0], pm, &result, &error, settings); + ASSERT(!error); + ASSERT_EQUALS(1, result); + + // u < 1, then u--: the value wraps around, nothing is known + pm.setValue(utok, lessThan(1)); + execute(utok->next(), pm, nullptr, nullptr, settings); + error = false; + execute(exprs[1], pm, &result, &error, settings); + ASSERT(error); + + // u > 3, then u--: u > 2 + pm.setValue(utok, greaterThan(3)); + execute(utok->next(), pm, nullptr, nullptr, settings); + error = false; + execute(exprs[1], pm, &result, &error, settings); + ASSERT(error); + const ProgramMemory::Values* values = pm.getValues(utok->exprId()); + ASSERT(values); + ASSERT_EQUALS(1U, values->size()); + ASSERT(findValue(*values, 2, ValueFlow::Value::Bound::Upper)); + } + void executeContainerSizeRange() { const char code[] = "void f(std::string s, bool y) {\n" " if (s.size() > 3) {\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 69359c9bdf3..a52d6f3fb80 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -128,6 +128,7 @@ class TestValueFlow : public TestFixture { TEST_CASE(valueFlowUninit); TEST_CASE(valueFlowConditionExpressions); + TEST_CASE(valueFlowConditionRanges); TEST_CASE(valueFlowContainerSize); TEST_CASE(valueFlowContainerSizeIterator); @@ -9134,6 +9135,134 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(true, testValueOfXImpossible(code, 4U, 0)); } + // The ranges that conditions give the program memory decide which branches a value reaches + void valueFlowConditionRanges() { + const char* code; + + // n > 3 does not mean that n is 4: the else branch is reachable + code = "void f(int n) {\n" + " int x = 1;\n" + " if (n > 3) {\n" + " if (n < 10) {}\n" + " else { int a = x; }\n" + " }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 5U, 1)); + + code = "void f(int n) {\n" + " int x = 1;\n" + " if (n > 3) {\n" + " if (n == 15) { int a = x; }\n" + " }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 1)); + + // 3 < n < 10: n == 15 is impossible, n == 5 is not + code = "void f(int n) {\n" + " int x = 1;\n" + " if (n > 3 && n < 10) {\n" + " if (n == 15) { int a = x; }\n" + " if (n == 5) { int b = x; }\n" + " }\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 4U, 1)); + ASSERT_EQUALS(true, testValueOfX(code, 5U, 1)); + + // a condition on the same variable is decided from the range + code = "void f(int n) {\n" + " int x = 1;\n" + " if (n > 3) {\n" + " if (n > 2) { int a = x; }\n" + " else { int b = x; }\n" + " }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 5U, 1)); + + // n >= 0 and n != 0 is n > 0 + code = "void f(int n) {\n" + " int x = 1;\n" + " if (n < 0) return;\n" + " if (n == 0) return;\n" + " if (n > 0) { int a = x; }\n" + " else { int b = x; }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 5U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 6U, 1)); + + // the range is shifted and scaled by arithmetic + code = "void f(int n) {\n" + " int x = 1;\n" + " if (n > 3) {\n" + " if (n + 1 > 4) { int a = x; }\n" + " else { int b = x; }\n" + " if ((n << 1) >= 8) { int c = x; }\n" + " else { int d = x; }\n" + " if (n * 2 == 7) { int e = x; }\n" + " if (-2 * n < -6) { int g = x; }\n" + " else { int h = x; }\n" + " }\n" + " if (n > 6) {\n" + " if (n / 2 > 2) { int i = x; }\n" + " else { int j = x; }\n" + " }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 5U, 1)); + ASSERT_EQUALS(true, testValueOfX(code, 6U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 7U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 8U, 1)); + ASSERT_EQUALS(true, testValueOfX(code, 9U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 10U, 1)); + ASSERT_EQUALS(true, testValueOfX(code, 13U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 14U, 1)); + + // the range of a product is solved exactly + code = "void f(int n) {\n" + " int x = 1;\n" + " if (n * 2 < 3) {\n" + " if (n == 1) { int a = x; }\n" + " if (n == 2) { int b = x; }\n" + " }\n" + " if (-2 * n > 3) {\n" + " if (n == -2) { int c = x; }\n" + " if (n == -1) { int d = x; }\n" + " }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 5U, 1)); + ASSERT_EQUALS(true, testValueOfX(code, 8U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 9U, 1)); + + // a range that excludes zero is true as a bool; one that includes zero is not known + code = "void f(int n) {\n" + " int x = 1;\n" + " if (n > 3) {\n" + " if (n) {} else { int a = x; }\n" + " }\n" + " if (n > -5) {\n" + " if (n) {} else { int b = x; }\n" + " }\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 4U, 1)); + ASSERT_EQUALS(true, testValueOfX(code, 7U, 1)); + + // container sizes + code = "void f(const std::string& s) {\n" + " int x = 1;\n" + " if (s.size() > 3 && s.size() < 10) {\n" + " if (s.size() == 15) { int a = x; }\n" + " if (s.empty()) { int b = x; }\n" + " if (s.size() < 20) { int c = x; }\n" + " else { int d = x; }\n" + " }\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 4U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 5U, 1)); + ASSERT_EQUALS(true, testValueOfX(code, 6U, 1)); + ASSERT_EQUALS(false, testValueOfX(code, 7U, 1)); + } + void valueFlowSymbolic() { const char* code; From e158231f840617325f4215a420576a60f1c90549 Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 27 Sep 2026 15:57:34 -0500 Subject: [PATCH 4/9] Simplify --- Makefile | 2 +- lib/programmemory.cpp | 388 ++++++++++++++++--------------------- lib/programmemory.h | 4 +- lib/valueflow.cpp | 31 +-- lib/vf_common.h | 7 + lib/vfvalue.h | 26 +++ oss-fuzz/Makefile | 2 +- test/testprogrammemory.cpp | 155 +++++++-------- 8 files changed, 285 insertions(+), 330 deletions(-) diff --git a/Makefile b/Makefile index 5810fcc9aa3..acbecf622e2 100644 --- a/Makefile +++ b/Makefile @@ -645,7 +645,7 @@ $(libcppdir)/platform.o: lib/platform.cpp externals/tinyxml2/tinyxml2.h lib/conf $(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/preprocessor.cpp -$(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/calculate.h lib/checkers.h lib/config.h lib/errortypes.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/programmemory.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vfvalue.h +$(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/calculate.h lib/checkers.h lib/config.h lib/errortypes.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/programmemory.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vf_common.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/programmemory.cpp $(libcppdir)/regex.o: lib/regex.cpp lib/config.h lib/regex.h diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index c5c28adf8d9..a2a7b36c1c2 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -31,6 +31,7 @@ #include "utils.h" #include "valueflow.h" #include "valueptr.h" +#include "vf_common.h" #include #include @@ -89,69 +90,59 @@ static bool isImpossiblePoint(const ValueFlow::Value& value) return value.isImpossible() && value.bound == ValueFlow::Value::Bound::Point; } -// The smallest value the expression can have according to a lower bound constraint -static MathLib::bigint lowerBound(const ValueFlow::Value& value) +// Is the value (a range when it is impossible with a bound) known to be nonzero? +static bool isTrue(const ValueFlow::Value& v) { - assert(isLowerBound(value)); - return value.intvalue + 1; + if (v.isUninitValue()) + return false; + if (v.isImpossible()) { + if (v.bound == ValueFlow::Value::Bound::Point) + return v.intvalue == 0; + // An impossible range excludes zero when it lies on one side of it + return v.isLowerEdge() ? v.rangeEdge() > 0 : v.rangeEdge() < 0; + } + return v.intvalue != 0; } -// The largest value the expression can have according to an upper bound constraint -static MathLib::bigint upperBound(const ValueFlow::Value& value) +static bool isFalse(const ValueFlow::Value& v) { - assert(isUpperBound(value)); - return value.intvalue - 1; + if (v.isUninitValue()) + return false; + if (v.isImpossible()) + return false; + return v.intvalue == 0; } -// Does the value satisfy the constraint? False when they are not comparable. +// Does the value satisfy the constraint of the same type? static bool satisfies(const ValueFlow::Value& value, const ValueFlow::Value& constraint) { - if (value.valueType != constraint.valueType) - return false; if (isImpossiblePoint(constraint)) return !value.equalValue(constraint); if (isLowerBound(constraint)) - return value.intvalue >= lowerBound(constraint); + return value.intvalue >= constraint.rangeEdge(); if (isUpperBound(constraint)) - return value.intvalue <= upperBound(constraint); + return value.intvalue <= constraint.rangeEdge(); return false; } -// Add the constraint to the constraints recorded for an expression, merging them the way -// Token::addValue() merges values: duplicates and weaker bounds are dropped, and an impossible value -// next to a bound moves the bound past it. Returns false if nothing changed. -static bool mergeConstraint(ProgramMemory::Values& values, const ValueFlow::Value& value) +static bool sameValue(const ValueFlow::Value& x, const ValueFlow::Value& y) { - ProgramMemory::Values merged = values; - merged.push_back(value); - Token::removeContradictions(merged); - if (merged.size() == values.size() && - std::equal(values.cbegin(), values.cend(), merged.cbegin(), [](const ValueFlow::Value& x, const ValueFlow::Value& y) { - return x == y && x.bound == y.bound; - })) - return false; - values = std::move(merged); - return true; + return x == y && x.bound == y.bound; } -// Record the value for an expression whose values are given. Returns false if nothing changed. -static bool mergeValue(ProgramMemory::Values& values, const ValueFlow::Value& value) +// Is the value already recorded: as the value of the expression, as one of its constraints, or as +// a constraint that the recorded value satisfies? +static bool isRecorded(const ProgramMemory::Values& values, const ValueFlow::Value& value) { - // A value of the expression, a first value, or a value of another type replaces what is recorded - if (!value.isImpossible() || values.empty() || values.front().valueType != value.valueType) { - if (values.size() == 1 && values.front() == value && values.front().bound == value.bound) - return false; - values.assign(1, value); - return true; - } - if (!values.front().isImpossible()) { - // A value that satisfies the constraint is more precise than the constraint - if (satisfies(values.front(), value)) - return false; - values.assign(1, value); - return true; - } - return mergeConstraint(values, value); + if (values.empty() || values.front().valueType != value.valueType) + return false; + if (!value.isImpossible()) + return values.size() == 1 && sameValue(values.front(), value); + if (!values.front().isImpossible()) + return satisfies(values.front(), value); + return std::any_of(values.cbegin(), values.cend(), [&](const ValueFlow::Value& v) { + return sameValue(v, value); + }); } void ProgramMemory::setValue(const Token* expr, const ValueFlow::Value& value) { @@ -180,11 +171,21 @@ void ProgramMemory::setValue(const Token* expr, const ValueFlow::Value& value) { void ProgramMemory::record(const Token* expr, const ValueFlow::Value& value) { const Values* existing = getValues(expr->exprId()); - Values values = existing ? *existing : Values{}; - if (!mergeValue(values, value)) + if (existing && isRecorded(*existing, value)) return; copyOnWrite(); - (*mValues)[expr] = std::move(values); + Values& values = (*mValues)[expr]; + // A value of the expression, a first value, a value of another type or a constraint that the + // recorded value violates replaces what is recorded. A constraint joins the recorded constraints, + // merged the way Token::addValue() merges values: weaker bounds are dropped and an impossible + // value next to a bound moves the bound past it. + if (!value.isImpossible() || values.empty() || values.front().valueType != value.valueType || + !values.front().isImpossible()) { + values.assign(1, value); + } else { + values.push_back(value); + Token::removeContradictions(values); + } } void ProgramMemory::setValues(const Token* expr, const Values& values) @@ -212,17 +213,9 @@ const ProgramMemory::Values* ProgramMemory::getValues(nonneg int exprid) const return &it->second; } -// The value of the expression, if one is recorded (rather than constraints) -static const ValueFlow::Value* getPointValue(const ProgramMemory::Values* values) -{ - if (!values || values->size() != 1 || values->front().isImpossible()) - return nullptr; - return &values->front(); -} - bool ProgramMemory::getIntValue(nonneg int exprid, MathLib::bigint& result) const { - const ValueFlow::Value* value = getPointValue(getValues(exprid)); + const ValueFlow::Value* value = getValue(exprid); if (value && value->isIntValue()) { result = value->intvalue; return true; @@ -240,7 +233,7 @@ void ProgramMemory::setIntValue(const Token* expr, MathLib::bigint value, bool i bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result) const { - const ValueFlow::Value* value = getPointValue(getValues(exprid)); + const ValueFlow::Value* value = getValue(exprid); if (value && value->isTokValue()) { result = value->tokvalue; return true; @@ -251,7 +244,7 @@ bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result) const // cppcheck-suppress unusedFunction bool ProgramMemory::getContainerSizeValue(nonneg int exprid, MathLib::bigint& result) const { - const ValueFlow::Value* value = getPointValue(getValues(exprid)); + const ValueFlow::Value* value = getValue(exprid); if (value && value->isContainerSizeValue()) { result = value->intvalue; return true; @@ -267,11 +260,9 @@ static ValueFlow::Value containerEmptyValue(const ProgramMemory::Values& values) continue; if (!value.isImpossible()) return ValueFlow::Value{value.intvalue == 0}; - if (isLowerBound(value) && lowerBound(value) > 0) - return ValueFlow::Value{0}; - if (isUpperBound(value) && upperBound(value) <= 0) + if (isUpperBound(value) && value.rangeEdge() <= 0) return ValueFlow::Value{1}; - if (isImpossiblePoint(value) && value.intvalue == 0) + if (isTrue(value)) return ValueFlow::Value{0}; } return ValueFlow::Value::unknown(); @@ -451,30 +442,6 @@ static bool frontIs(const std::vector& v, bool i) return !i; } -static bool isTrue(const ValueFlow::Value& v) -{ - if (v.isUninitValue()) - return false; - if (v.isImpossible()) { - // An impossible range excludes zero when zero is on its side of the bound - if (v.bound == ValueFlow::Value::Bound::Upper) - return v.intvalue >= 0; - if (v.bound == ValueFlow::Value::Bound::Lower) - return v.intvalue <= 0; - return v.intvalue == 0; - } - return v.intvalue != 0; -} - -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) @@ -538,17 +505,16 @@ static void programMemoryParseCondition(ProgramMemory& pm, if (endTok && changed(vartok, tok->next(), endTok)) return; const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then); - ValueFlow::Value v = then ? truevalue : falsevalue; + ValueFlow::Value& v = then ? truevalue : falsevalue; // A value with a bound is a range: record it as the range of impossible values so that it // constrains the expression instead of standing in for its value. if (impossible || v.bound != ValueFlow::Value::Bound::Point) - v = asImpossible(v); + v = asImpossible(std::move(v)); pm.setValue(vartok, v); const Token* containerTok = settings.library.getContainerFromYield(vartok, Library::Container::Yield::SIZE); if (containerTok) { - ValueFlow::Value size = v; - size.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; - pm.setValue(containerTok, size); + v.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + pm.setValue(containerTok, v); } } else if (Token::simpleMatch(tok, "!")) { programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then, findChanged); @@ -861,50 +827,57 @@ static bool isBounded(const ValueFlow::Value& value) return value.bound != ValueFlow::Value::Bound::Point; } -static bool isSaturated(MathLib::bigint value) -{ - return value == std::numeric_limits::max() || value == std::numeric_limits::min(); -} - static bool multiplyOverflows(MathLib::bigint x, MathLib::bigint y) { if (x == 0 || y == 0) return false; - if (isSaturated(x) || isSaturated(y)) + if (ValueFlow::isSaturated(x) || ValueFlow::isSaturated(y)) return true; return std::abs(x) > std::numeric_limits::max() / std::abs(y); } -// The range of "x * k", "x / k" or "x << k" when the values of x up to (or from) the bound are -// impossible: the end of the range is transformed, and a negative factor turns the range around. -static ValueFlow::Value scaleRange(const ValueFlow::Value& range, const std::string& op, MathLib::bigint k) +// The operations that keep the order of the values of a range +static bool isMonotone(const std::string& op) +{ + return contains({"+", "-", "*", "/", "<<", ">>"}, op); +} + +// The range of "x k" (or "k x") when the values of x up to (or from) the bound are +// impossible: the end of the range is transformed; an operation that reverses the order of the +// values turns the range around. +static ValueFlow::Value applyToRange(const std::string& op, const ValueFlow::Value& range, MathLib::bigint k, bool rangeIsLhs) { - const bool lower = isLowerBound(range); - const MathLib::bigint edge = lower ? lowerBound(range) : upperBound(range); - if (k == 0 || isSaturated(edge)) + const MathLib::bigint edge = range.rangeEdge(); + if (ValueFlow::isSaturated(edge) || ValueFlow::isSaturated(k)) return ValueFlow::Value::unknown(); - MathLib::bigint scaled = 0; - bool lowerAfter = lower; - if (op == "*") { - if (multiplyOverflows(edge, k)) - return ValueFlow::Value::unknown(); - scaled = edge * k; - lowerAfter = (k > 0) == lower; + bool increasing = true; + MathLib::bigint result = 0; + if (op == "+") { + result = edge + k; + } else if (op == "-") { + increasing = rangeIsLhs; + result = rangeIsLhs ? edge - k : k - edge; + } else if (op == "*") { + if (k == 0 || multiplyOverflows(edge, k)) + return ValueFlow::Value::unknown(); + increasing = k > 0; + result = edge * k; } else if (op == "/") { - // Truncation towards zero keeps the order of the values - scaled = edge / k; - lowerAfter = (k > 0) == lower; - } else if (op == "<<") { - if (k < 0 || k >= 63 || edge < 0 || edge > (std::numeric_limits::max() >> k)) + if (k == 0 || !rangeIsLhs) return ValueFlow::Value::unknown(); - scaled = edge << k; + // Truncation towards zero keeps the order of the values + increasing = k > 0; + result = edge / k; } else { - return ValueFlow::Value::unknown(); + // Shifts: calculate() rejects a negative or too large shift and a negative value + bool error = false; + result = calculate(op, edge, k, &error); + if (!rangeIsLhs || error || (op == "<<" && (result >> k) != edge)) + return ValueFlow::Value::unknown(); } - ValueFlow::Value result = range; - result.intvalue = lowerAfter ? scaled - 1 : scaled + 1; - result.bound = lowerAfter ? ValueFlow::Value::Bound::Upper : ValueFlow::Value::Bound::Lower; - return result; + ValueFlow::Value scaled = range; + scaled.setRangeEdge(result, range.isLowerEdge() == increasing); + return scaled; } static ValueFlow::Value evaluate(const Token* op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs, bool removeAssign = false) @@ -913,30 +886,19 @@ static ValueFlow::Value evaluate(const Token* op, const ValueFlow::Value& lhs, c ValueFlow::Value result; if (lhs.isImpossible() && rhs.isImpossible()) return ValueFlow::Value::unknown(); - // An impossible range combined with an int: shifting moves the bound along, scaling transforms it - const ValueFlow::Value& range = isBounded(lhs) ? lhs : rhs; - const ValueFlow::Value& delta = isBounded(lhs) ? rhs : lhs; - if (range.isImpossible() && isBounded(range) && !isBounded(delta) && !delta.isImpossible() && range.isIntValue() && - delta.isIntValue()) { - if (contains({"+", "-"}, opStr)) { - bool error = false; - result = range; - result.intvalue = calculate(opStr, lhs.intvalue, rhs.intvalue, &error); - if (error) - return ValueFlow::Value::unknown(); - // c - x reverses the direction of the range - if (isBounded(rhs) && opStr == "-") - result.invertBound(); - return result; - } - if (opStr == "*" || (isBounded(lhs) && contains({"/", "<<"}, opStr))) - return scaleRange(range, opStr, delta.intvalue); - } + // An impossible range and an int: the range of the result, for the operations that keep the order + const bool rangeIsLhs = lhs.isImpossible() && isBounded(lhs); + const ValueFlow::Value& range = rangeIsLhs ? lhs : rhs; + const ValueFlow::Value& k = rangeIsLhs ? rhs : lhs; + if (range.isImpossible() && isBounded(range) && range.isIntValue() && !k.isImpossible() && !isBounded(k) && + k.isIntValue() && isMonotone(opStr)) + return applyToRange(opStr, range, k.intvalue, rangeIsLhs); if (lhs.isImpossible() || rhs.isImpossible()) { - // noninvertible + // The image of an impossible value is impossible only for an injective operation if (contains({"%", "/", "&", "|", ">>"}, opStr)) return ValueFlow::Value::unknown(); - if (opStr == "*" && (lhs.equalTo(0) || rhs.equalTo(0))) + const ValueFlow::Value& factor = lhs.isImpossible() ? rhs : lhs; + if (opStr == "*" && factor.equalTo(0)) return ValueFlow::Value::unknown(); result.setImpossible(); } @@ -1635,14 +1597,16 @@ namespace { return values.empty() ? unknown() : values.front(); } - // Is the expression read directly from the program memory (no known value, no tracked value)? + // The values recorded for the expression, when it is read from the program memory: it has no + // known value and does not depend on a tracked value const ProgramMemory::Values* getStoredValues(const Token* expr) const { - if (!expr || expr->exprId() == 0 || expr->hasKnownIntValue()) + if (expr->exprId() == 0) return nullptr; - if (dependsOnTrackedValue(expr)) + const ProgramMemory::Values* stored = pm->getValues(expr->exprId()); + if (!stored || expr->hasKnownIntValue() || dependsOnTrackedValue(expr)) return nullptr; - return pm->getValues(expr->exprId()); + return stored; } static ValueFlow::Value unknown() { @@ -1790,42 +1754,41 @@ namespace { if (value.tokvalue->exprId() == 0) continue; const ValueFlow::Value* sizeValue = pm->getValue(value.tokvalue->exprId()); - if (sizeValue && sizeValue->isContainerSizeValue()) - return {*sizeValue}; + if (sizeValue && sizeValue->isContainerSizeValue()) { + sizes.push_back(*sizeValue); + break; + } } - return {}; + return sizes; } - // The container whose size the expression yields, or nullptr - static const Token* getContainerFromSizeYield(const Token* expr) + // The size values of the container, as ints + ProgramMemory::Values executeSizeYield(const Token* containerTok) { - if (!Token::Match(expr->tokAt(-2), ". %name% (") || !astIsContainer(expr->tokAt(-2)->astOperand1())) - return nullptr; - const Token* containerTok = expr->tokAt(-2)->astOperand1(); - const Library::Container::Yield yield = containerTok->valueType()->container->getYield(expr->strAt(-1)); - return yield == Library::Container::Yield::SIZE ? containerTok : nullptr; + ProgramMemory::Values sizes = executeContainerSizes(containerTok); + for (ValueFlow::Value& v : sizes) + v.valueType = ValueFlow::Value::ValueType::INT; + return sizes; } // All values of the expression: the constraints of a range read from the program memory, // or the single result of execute() ProgramMemory::Values executeValues(const Token* expr) { - if (!expr) - return {}; - if (const ProgramMemory::Values* stored = getStoredValues(expr)) { - if (stored->size() > 1) + if (expr->exprId() > 0) { + // Several constraints are read as they are. Whether they apply is checked only then, + // as that walks the expression. + const ProgramMemory::Values* stored = pm->getValues(expr->exprId()); + if (stored && stored->size() > 1 && !expr->hasKnownIntValue() && !dependsOnTrackedValue(expr)) return *stored; } - if (const Token* containerTok = getContainerFromSizeYield(expr)) { - ProgramMemory::Values sizes = executeContainerSizes(containerTok); - for (ValueFlow::Value& v : sizes) - v.valueType = ValueFlow::Value::ValueType::INT; - return sizes; - } + if (const Token* containerTok = settings.library.getContainerFromYield(expr, Library::Container::Yield::SIZE)) + return executeSizeYield(containerTok); + ProgramMemory::Values values; ValueFlow::Value v = execute(expr); - if (v.isUninitValue()) - return {}; - return {std::move(v)}; + if (!v.isUninitValue()) + values.push_back(std::move(v)); + return values; } ValueFlow::Value executeImpl(const Token* expr) @@ -1852,21 +1815,13 @@ namespace { } if (expr->isBoolean()) return ValueFlow::Value{expr->str() == "true"}; - if (Token::Match(expr->tokAt(-2), ". %name% (") && astIsContainer(expr->tokAt(-2)->astOperand1())) { - const Token* containerTok = expr->tokAt(-2)->astOperand1(); - const Library::Container::Yield yield = containerTok->valueType()->container->getYield(expr->strAt(-1)); - if (yield == Library::Container::Yield::SIZE) { - ValueFlow::Value v = representative(executeContainerSizes(containerTok)); - if (!v.isContainerSizeValue()) - return unknown(); - v.valueType = ValueFlow::Value::ValueType::INT; + if (const Token* containerTok = settings.library.getContainerFromYield(expr, Library::Container::Yield::SIZE)) { + return representative(executeSizeYield(containerTok)); + } + if (const Token* containerTok = settings.library.getContainerFromYield(expr, Library::Container::Yield::EMPTY)) { + ValueFlow::Value v = containerEmptyValue(executeContainerSizes(containerTok)); + if (!v.isUninitValue()) return v; - } - if (yield == Library::Container::Yield::EMPTY) { - ValueFlow::Value v = containerEmptyValue(executeContainerSizes(containerTok)); - if (!v.isUninitValue()) - return v; - } } else if (expr->isAssignmentOp() && expr->astOperand1() && expr->astOperand2() && expr->astOperand1()->exprId() > 0) { ValueFlow::Value rhs = execute(expr->astOperand2()); @@ -1875,12 +1830,11 @@ namespace { if (expr->str() != "=") { if (!pm->hasValue(expr->astOperand1()->exprId())) return unknown(); - // Apply the operation to every value of the variable ProgramMemory::Values& lhs = pm->at(expr->astOperand1()->exprId()); for (ValueFlow::Value& v : lhs) { const ValueFlow::Value r = evaluate(expr, v, rhs, /*removeAssign*/ true); if (r.isUninitValue()) { - pm->setUnknown(expr->astOperand1()); + lhs.assign(1, unknown()); return unknown(); } if (v.isIntValue()) @@ -1906,25 +1860,17 @@ namespace { } else if (expr->tokType() == Token::eIncDecOp && expr->astOperand1() && expr->astOperand1()->exprId() != 0) { if (!pm->hasValue(expr->astOperand1()->exprId())) return ValueFlow::Value::unknown(); - const ProgramMemory::Values& values = utils::as_const(*pm).at(expr->astOperand1()->exprId()); - if (!std::all_of(values.cbegin(), values.cend(), std::mem_fn(&ValueFlow::Value::isIntValue))) + ProgramMemory::Values& lhs = pm->at(expr->astOperand1()->exprId()); + // The values of an expression all have the same type + if (!lhs.front().isIntValue()) + return unknown(); + // An unsigned value wraps around when it is decremented and may be zero + if (expr->str() == "--" && astIsUnsigned(expr->astOperand1()) && std::none_of(lhs.cbegin(), lhs.cend(), &isTrue)) { + lhs.assign(1, unknown()); return unknown(); - // An unsigned value wraps around when zero is decremented - if (expr->str() == "--" && astIsUnsigned(expr->astOperand1())) { - const bool zero = std::any_of(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) { - return !v.isImpossible() && v.intvalue == 0; - }); - const bool excludesZero = std::any_of(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) { - return (isLowerBound(v) && lowerBound(v) >= 1) || (isImpossiblePoint(v) && v.intvalue == 0); - }); - if (zero || (values.front().isImpossible() && !excludesZero)) { - pm->setUnknown(expr->astOperand1()); - return unknown(); - } } // Shift every value of the variable; bounds and impossible values move along - ProgramMemory::Values& lhs = pm->at(expr->astOperand1()->exprId()); for (ValueFlow::Value& v : lhs) { if (expr->str() == "++") v.intvalue++; @@ -1956,22 +1902,22 @@ namespace { if (index == strValue.size()) return ValueFlow::Value{}; } else if (Token::Match(expr, "%cop%") && expr->astOperand1() && expr->astOperand2()) { - const ProgramMemory::Values lhsValues = executeValues(expr->astOperand1()); + ProgramMemory::Values lhsValues = executeValues(expr->astOperand1()); if (lhsValues.empty()) return unknown(); - const ProgramMemory::Values rhsValues = executeValues(expr->astOperand2()); + ProgramMemory::Values rhsValues = executeValues(expr->astOperand2()); if (rhsValues.empty()) return unknown(); - // Compare ranges: an operand with constraints is compared as the interval they describe - if (expr->isComparisonOp() && - (std::any_of(lhsValues.cbegin(), lhsValues.cend(), std::mem_fn(&ValueFlow::Value::isImpossible)) || - std::any_of(rhsValues.cbegin(), rhsValues.cend(), std::mem_fn(&ValueFlow::Value::isImpossible)))) { - std::vector result = infer(makeIntegralInferModel(), expr->str(), lhsValues, rhsValues); + ValueFlow::Value lhs = representative(lhsValues); + ValueFlow::Value rhs = representative(rhsValues); + // Compare ranges: an operand with constraints (the values of an operand are either one + // value or all constraints) is compared as the interval they describe + if (expr->isComparisonOp() && (lhs.isImpossible() || rhs.isImpossible())) { + std::vector result = + infer(makeIntegralInferModel(), expr->str(), std::move(lhsValues), std::move(rhsValues)); if (!result.empty()) return std::move(result.front()); } - ValueFlow::Value lhs = representative(lhsValues); - ValueFlow::Value rhs = representative(rhsValues); ValueFlow::Value r = evaluate(expr, lhs, rhs); if (expr->isComparisonOp() && (r.isUninitValue() || r.isImpossible())) { if (rhs.isIntValue() && !expr->astOperand1()->values().empty()) { @@ -2044,9 +1990,9 @@ namespace { } if (const ProgramMemory::Values* stored = getStoredValues(expr)) { // An impossible value that excludes zero makes the expression true as a bool - if (isUsedAsBool(expr, settings) && std::any_of(stored->cbegin(), stored->cend(), [](const ValueFlow::Value& v) { + if (std::any_of(stored->cbegin(), stored->cend(), [](const ValueFlow::Value& v) { return v.isImpossible() && v.isIntValue() && isTrue(v); - })) { + }) && isUsedAsBool(expr, settings)) { ValueFlow::Value result{1}; result.setKnown(); return result; @@ -2104,9 +2050,10 @@ namespace { } // Check if function modifies argument visitAstNodes(expr->astOperand2(), [&](const Token* child) { - if (child->exprId() > 0 && pm->hasValue(child->exprId())) { + const ProgramMemory::Values* values = child->exprId() > 0 ? pm->getValues(child->exprId()) : nullptr; + if (values) { // The values of an expression all have the same type - const ValueFlow::Value& v = utils::as_const(*pm).at(child->exprId()).front(); + const ValueFlow::Value& v = values->front(); if (v.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) { if (ValueFlow::isContainerSizeChanged(child, v.indirect, settings)) pm->setUnknown(child); @@ -2164,9 +2111,11 @@ namespace { return v; if (!expr) return v; - if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) { - if (updateValue(v, representative(utils::as_const(*pm).at(expr->exprId())))) - return v; + if (expr->exprId() > 0) { + if (const ProgramMemory::Values* stored = pm->getValues(expr->exprId())) { + if (updateValue(v, representative(*stored))) + return v; + } } // Find symbolic values for (const ValueFlow::Value& value : expr->values()) { @@ -2174,11 +2123,10 @@ namespace { continue; if (!value.isKnown()) continue; - if (value.tokvalue->exprId() > 0 && !pm->hasValue(value.tokvalue->exprId())) - continue; - ValueFlow::Value v2 = representative(utils::as_const(*pm).at(value.tokvalue->exprId())); - if (!v2.isIntValue() && value.intvalue != 0) + const ProgramMemory::Values* stored = pm->getValues(value.tokvalue->exprId()); + if (!stored || (!stored->front().isIntValue() && value.intvalue != 0)) continue; + ValueFlow::Value v2 = stored->front(); v2.intvalue += value.intvalue; return v2; } diff --git a/lib/programmemory.h b/lib/programmemory.h index 276a29126e8..b2b05513eb3 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -109,8 +109,8 @@ struct CPPCHECKLIB ProgramMemory { * value with a bound is still its value; the bound is extra information about the range it lies * in) or a set of constraints that hold at the same time: impossible values, where a bound makes * the value an impossible range, so that "x > 3" is recorded as "values <= 3 are impossible". - * The constraints of one expression all have the same value type. A list, so that references to - * the values stay valid while values are added. + * The constraints of one expression all have the same value type. A list, so that adding a + * constraint does not move the values already recorded. */ using Values = std::list; using Map = std::map; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 0eba32eafab..bf2b4fa8b11 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -270,11 +270,6 @@ static void setConditionalValues(const Token* tok, setValueBound(false_value, tok, !lhs); } -static bool isSaturated(MathLib::bigint value) -{ - return value == std::numeric_limits::max() || value == std::numeric_limits::min(); -} - static void parseCompareEachInt( const Token* tok, const std::function& each, @@ -292,7 +287,7 @@ static void parseCompareEachInt( value1.clear(); } for (const ValueFlow::Value& v1 : value1) { - if (isSaturated(v1.intvalue) || astIsFloat(tok->astOperand2(), /*unknown*/ false)) + if (ValueFlow::isSaturated(v1.intvalue) || astIsFloat(tok->astOperand2(), /*unknown*/ false)) continue; ValueFlow::Value true_value = v1; ValueFlow::Value false_value = v1; @@ -300,7 +295,7 @@ static void parseCompareEachInt( each(tok->astOperand2(), std::move(true_value), std::move(false_value)); } for (const ValueFlow::Value& v2 : value2) { - if (isSaturated(v2.intvalue) || astIsFloat(tok->astOperand1(), /*unknown*/ false)) + if (ValueFlow::isSaturated(v2.intvalue) || astIsFloat(tok->astOperand1(), /*unknown*/ false)) continue; ValueFlow::Value true_value = v2; ValueFlow::Value false_value = v2; @@ -6278,24 +6273,12 @@ static MathLib::bigint ceilDiv(MathLib::bigint x, MathLib::bigint y) } // Solve "x * divisor" for x when the value is a bound: divide the end of the range, rounding towards -// the inside of the range so that it stays exact, and turn the range around for a negative divisor. +// the inside of the range so that it stays exact; a negative divisor turns the range around. static void divideBound(ValueFlow::Value& value, MathLib::bigint divisor) { - // Is the value the lower end of the range? A possible lower bound is, and so is an impossible - // upper bound, as the values up to it are impossible. - const bool lower = (value.bound == ValueFlow::Value::Bound::Lower) != value.isImpossible(); - // The end of the range: the first value that is possible - MathLib::bigint edge = value.intvalue; - if (value.isImpossible()) - edge += lower ? 1 : -1; - const bool lowerAfter = (divisor > 0) == lower; - edge = lowerAfter ? ceilDiv(edge, divisor) : floorDiv(edge, divisor); - if (divisor < 0) - value.invertBound(); - if (value.isImpossible()) - value.intvalue = lowerAfter ? edge - 1 : edge + 1; - else - value.intvalue = edge; + const bool lowerAfter = (divisor > 0) == value.isLowerEdge(); + const MathLib::bigint edge = value.rangeEdge(); + value.setRangeEdge(lowerAfter ? ceilDiv(edge, divisor) : floorDiv(edge, divisor), lowerAfter); } const Token* ValueFlow::solveExprValue(const Token* expr, @@ -6330,7 +6313,7 @@ const Token* ValueFlow::solveExprValue(const Token* expr, return ValueFlow::solveExprValue(binaryTok, eval, value); } case '*': { - if (intval == 0 || isSaturated(value.intvalue)) + if (intval == 0 || ValueFlow::isSaturated(value.intvalue)) break; if (value.bound == ValueFlow::Value::Bound::Point) { // x * k is v only for a v that k divides diff --git a/lib/vf_common.h b/lib/vf_common.h index aca438a0dff..95448fde724 100644 --- a/lib/vf_common.h +++ b/lib/vf_common.h @@ -25,6 +25,7 @@ #include "symboldatabase.h" #include +#include #include class Token; @@ -43,6 +44,12 @@ namespace ValueFlow MathLib::bigint truncateIntValue(MathLib::bigint value, size_t value_size, ValueType::Sign dst_sign); + /** Is the value at a limit of its type, standing for any value beyond? */ + inline bool isSaturated(MathLib::bigint value) + { + return value == std::numeric_limits::max() || value == std::numeric_limits::min(); + } + Token * valueFlowSetConstantValue(Token *tok, const Settings &settings); Value castValue(Value value, ValueType::Sign sign, nonneg int bit); diff --git a/lib/vfvalue.h b/lib/vfvalue.h index 9eab775de24..76ef3b85b5e 100644 --- a/lib/vfvalue.h +++ b/lib/vfvalue.h @@ -194,6 +194,32 @@ namespace ValueFlow decreaseRange(); } + /** + * Is a value with a bound the lower end of its range? A possible lower bound is; so is an + * impossible upper bound, as the values up to it are impossible. + */ + bool isLowerEdge() const { + return (bound == Bound::Lower) != isImpossible(); + } + + /** The first value inside the range of a value with a bound */ + MathLib::bigint rangeEdge() const { + if (!isImpossible()) + return intvalue; + return isLowerEdge() ? intvalue + 1 : intvalue - 1; + } + + /** Let the range start (lower edge) or end at the given value, keeping the kind of the value */ + void setRangeEdge(MathLib::bigint edge, bool lowerEdge) { + if (isImpossible()) { + bound = lowerEdge ? Bound::Upper : Bound::Lower; + intvalue = lowerEdge ? edge - 1 : edge + 1; + } else { + bound = lowerEdge ? Bound::Lower : Bound::Upper; + intvalue = edge; + } + } + void assumeCondition(const Token* tok); std::string infoString() const; diff --git a/oss-fuzz/Makefile b/oss-fuzz/Makefile index e6966747958..b1b19d9a2b9 100644 --- a/oss-fuzz/Makefile +++ b/oss-fuzz/Makefile @@ -315,7 +315,7 @@ $(libcppdir)/platform.o: ../lib/platform.cpp ../externals/tinyxml2/tinyxml2.h .. $(libcppdir)/preprocessor.o: ../lib/preprocessor.cpp ../externals/simplecpp/simplecpp.h ../lib/checkers.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/preprocessor.h ../lib/settings.h ../lib/standards.h ../lib/suppressions.h ../lib/utils.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/preprocessor.cpp -$(libcppdir)/programmemory.o: ../lib/programmemory.cpp ../lib/astutils.h ../lib/calculate.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/infer.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/programmemory.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/valueptr.h ../lib/vfvalue.h +$(libcppdir)/programmemory.o: ../lib/programmemory.cpp ../lib/astutils.h ../lib/calculate.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/infer.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/programmemory.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/valueptr.h ../lib/vf_common.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/programmemory.cpp $(libcppdir)/regex.o: ../lib/regex.cpp ../lib/config.h ../lib/regex.h diff --git a/test/testprogrammemory.cpp b/test/testprogrammemory.cpp index 32a394ee96e..9d0a3d6ad49 100644 --- a/test/testprogrammemory.cpp +++ b/test/testprogrammemory.cpp @@ -75,11 +75,10 @@ class TestProgramMemory : public TestFixture { return v; } - static const ValueFlow::Value* findValue(const ProgramMemory::Values& values, MathLib::bigint x, ValueFlow::Value::Bound bound) { - const auto it = std::find_if(values.cbegin(), values.cend(), [&](const ValueFlow::Value& v) { + static bool hasValue(const ProgramMemory::Values& values, MathLib::bigint x, ValueFlow::Value::Bound bound) { + return std::any_of(values.cbegin(), values.cend(), [&](const ValueFlow::Value& v) { return v.intvalue == x && v.bound == bound; }); - return it == values.cend() ? nullptr : &*it; } void copyOnWrite() const { @@ -160,8 +159,8 @@ class TestProgramMemory : public TestFixture { const ProgramMemory::Values* values = pm.getValues(id); ASSERT(values); ASSERT_EQUALS(2U, values->size()); - ASSERT(findValue(*values, 3, ValueFlow::Value::Bound::Upper)); - ASSERT(findValue(*values, 10, ValueFlow::Value::Bound::Lower)); + ASSERT(hasValue(*values, 3, ValueFlow::Value::Bound::Upper)); + ASSERT(hasValue(*values, 10, ValueFlow::Value::Bound::Lower)); // several constraints are not a single value ASSERT(!pm.getValue(id)); @@ -176,24 +175,24 @@ class TestProgramMemory : public TestFixture { // a weaker bound is dropped pm.setValue(tok, greaterThan(1)); ASSERT_EQUALS(2U, pm.at(id).size()); - ASSERT(findValue(pm.at(id), 3, ValueFlow::Value::Bound::Upper)); + ASSERT(hasValue(pm.at(id), 3, ValueFlow::Value::Bound::Upper)); // a stronger bound replaces the bound pm.setValue(tok, greaterThan(5)); ASSERT_EQUALS(2U, pm.at(id).size()); - ASSERT(findValue(pm.at(id), 5, ValueFlow::Value::Bound::Upper)); - ASSERT(!findValue(pm.at(id), 3, ValueFlow::Value::Bound::Upper)); + ASSERT(hasValue(pm.at(id), 5, ValueFlow::Value::Bound::Upper)); + ASSERT(!hasValue(pm.at(id), 3, ValueFlow::Value::Bound::Upper)); // an impossible value inside the range is kept pm.setValue(tok, impossible(7)); ASSERT_EQUALS(3U, pm.at(id).size()); - ASSERT(findValue(pm.at(id), 7, ValueFlow::Value::Bound::Point)); + ASSERT(hasValue(pm.at(id), 7, ValueFlow::Value::Bound::Point)); // x > 5 and x != 6 is x > 6, and then x != 7 makes it x > 7 pm.setValue(tok, impossible(6)); ASSERT_EQUALS(2U, pm.at(id).size()); - ASSERT(findValue(pm.at(id), 7, ValueFlow::Value::Bound::Upper)); - ASSERT(findValue(pm.at(id), 10, ValueFlow::Value::Bound::Lower)); + ASSERT(hasValue(pm.at(id), 7, ValueFlow::Value::Bound::Upper)); + ASSERT(hasValue(pm.at(id), 10, ValueFlow::Value::Bound::Lower)); } void setValueReplacesConstraints() const { @@ -303,18 +302,30 @@ class TestProgramMemory : public TestFixture { return result; } - // Evaluate the expression with the program memory built from the conditions enclosing it. - // The result as a string, empty if it is unknown. - std::string evaluate(const Token* expr) const { - ProgramMemoryState pms(settings); - pms.addState(expr, {}); - ProgramMemory pm = pms.state; + // Evaluate the expression with the program memory. The result as a string, empty if it is unknown. + std::string evaluate(const Token* expr, ProgramMemory pm) const { MathLib::bigint result = 0; bool error = false; execute(expr, pm, &result, &error, settings); - if (error) - return ""; - return std::to_string(result); + return error ? "" : std::to_string(result); + } + + // Evaluate the expression with the program memory built from the conditions enclosing it + std::string evaluate(const Token* expr) const { + ProgramMemoryState pms(settings); + pms.addState(expr, {}); + return evaluate(expr, pms.state); + } + + // The results of the expressions assigned to y in the code, each evaluated at its position + std::vector evaluateAssignments(const char code[]) { + SimpleTokenizer tokenizer(settings, *this); + ASSERT(tokenizer.tokenize(code)); + clearValues(tokenizer); + std::vector results; + for (const Token* expr : assignedExpressions(tokenizer.tokens(), "y")) + results.push_back(evaluate(expr)); + return results; } void executeRange() { @@ -332,22 +343,19 @@ class TestProgramMemory : public TestFixture { " }\n" " }\n" "}\n"; - SimpleTokenizer tokenizer(settings, *this); - ASSERT(tokenizer.tokenize(code)); - clearValues(tokenizer); - const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); - ASSERT_EQUALS(8U, exprs.size()); + const std::vector results = evaluateAssignments(code); + ASSERT_EQUALS(8U, results.size()); // 3 < x < 10 - ASSERT_EQUALS("0", evaluate(exprs[0])); - ASSERT_EQUALS("", evaluate(exprs[1])); - ASSERT_EQUALS("1", evaluate(exprs[2])); - ASSERT_EQUALS("1", evaluate(exprs[3])); + ASSERT_EQUALS("0", results[0]); + ASSERT_EQUALS("", results[1]); + ASSERT_EQUALS("1", results[2]); + ASSERT_EQUALS("1", results[3]); // the range is shifted by arithmetic - ASSERT_EQUALS("1", evaluate(exprs[4])); - ASSERT_EQUALS("1", evaluate(exprs[5])); - ASSERT_EQUALS("1", evaluate(exprs[6])); + ASSERT_EQUALS("1", results[4]); + ASSERT_EQUALS("1", results[5]); + ASSERT_EQUALS("1", results[6]); // a range is not a value - ASSERT_EQUALS("", evaluate(exprs[7])); + ASSERT_EQUALS("", results[7]); } void executeScaledRange() { @@ -366,24 +374,21 @@ class TestProgramMemory : public TestFixture { " y = x / -2 < -2;\n" " }\n" "}\n"; - SimpleTokenizer tokenizer(settings, *this); - ASSERT(tokenizer.tokenize(code)); - clearValues(tokenizer); - const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); - ASSERT_EQUALS(9U, exprs.size()); + const std::vector results = evaluateAssignments(code); + ASSERT_EQUALS(9U, results.size()); // x > 3: x * 2 >= 8 - ASSERT_EQUALS("1", evaluate(exprs[0])); - ASSERT_EQUALS("0", evaluate(exprs[1])); - ASSERT_EQUALS("1", evaluate(exprs[2])); + ASSERT_EQUALS("1", results[0]); + ASSERT_EQUALS("0", results[1]); + ASSERT_EQUALS("1", results[2]); // x * 0 is not "not zero" - ASSERT_EQUALS("", evaluate(exprs[3])); - ASSERT_EQUALS("1", evaluate(exprs[4])); - // remainder and right shift do not keep the range - ASSERT_EQUALS("", evaluate(exprs[5])); - ASSERT_EQUALS("", evaluate(exprs[6])); + ASSERT_EQUALS("", results[3]); + ASSERT_EQUALS("1", results[4]); + // the remainder does not keep the range; x >> 1 >= 2 + ASSERT_EQUALS("", results[5]); + ASSERT_EQUALS("0", results[6]); // x > 6: x / 2 >= 3 - ASSERT_EQUALS("1", evaluate(exprs[7])); - ASSERT_EQUALS("1", evaluate(exprs[8])); + ASSERT_EQUALS("1", results[7]); + ASSERT_EQUALS("1", results[8]); } void executeSolvedRange() { @@ -405,23 +410,20 @@ class TestProgramMemory : public TestFixture { " y = x == 0;\n" " }\n" "}\n"; - SimpleTokenizer tokenizer(settings, *this); - ASSERT(tokenizer.tokenize(code)); - clearValues(tokenizer); - const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); - ASSERT_EQUALS(8U, exprs.size()); + const std::vector results = evaluateAssignments(code); + ASSERT_EQUALS(8U, results.size()); // x * 2 < 3: x <= 1 - ASSERT_EQUALS("1", evaluate(exprs[0])); - ASSERT_EQUALS("", evaluate(exprs[1])); - ASSERT_EQUALS("0", evaluate(exprs[2])); + ASSERT_EQUALS("1", results[0]); + ASSERT_EQUALS("", results[1]); + ASSERT_EQUALS("0", results[2]); // x * 3 >= 7: x >= 3 - ASSERT_EQUALS("1", evaluate(exprs[3])); - ASSERT_EQUALS("0", evaluate(exprs[4])); + ASSERT_EQUALS("1", results[3]); + ASSERT_EQUALS("0", results[4]); // -2 * x > 3: x <= -2 - ASSERT_EQUALS("1", evaluate(exprs[5])); - ASSERT_EQUALS("0", evaluate(exprs[6])); + ASSERT_EQUALS("1", results[5]); + ASSERT_EQUALS("0", results[6]); // (x ^ 4) > 3 does not give a range for x - ASSERT_EQUALS("", evaluate(exprs[7])); + ASSERT_EQUALS("", results[7]); } void executeCompoundAssignment() { @@ -444,29 +446,21 @@ class TestProgramMemory : public TestFixture { // x > 3, then x *= -1: x < -3 pm.setValue(xtok, greaterThan(3)); execute(xtok->next(), pm, nullptr, nullptr, settings); - MathLib::bigint result = 0; - bool error = false; - execute(exprs[0], pm, &result, &error, settings); - ASSERT(!error); - ASSERT_EQUALS(1, result); + ASSERT_EQUALS("1", evaluate(exprs[0], pm)); // u < 1, then u--: the value wraps around, nothing is known pm.setValue(utok, lessThan(1)); execute(utok->next(), pm, nullptr, nullptr, settings); - error = false; - execute(exprs[1], pm, &result, &error, settings); - ASSERT(error); + ASSERT_EQUALS("", evaluate(exprs[1], pm)); // u > 3, then u--: u > 2 pm.setValue(utok, greaterThan(3)); execute(utok->next(), pm, nullptr, nullptr, settings); - error = false; - execute(exprs[1], pm, &result, &error, settings); - ASSERT(error); + ASSERT_EQUALS("", evaluate(exprs[1], pm)); const ProgramMemory::Values* values = pm.getValues(utok->exprId()); ASSERT(values); ASSERT_EQUALS(1U, values->size()); - ASSERT(findValue(*values, 2, ValueFlow::Value::Bound::Upper)); + ASSERT(hasValue(*values, 2, ValueFlow::Value::Bound::Upper)); } void executeContainerSizeRange() { @@ -480,16 +474,13 @@ class TestProgramMemory : public TestFixture { " }\n" " }\n" "}\n"; - SimpleTokenizer tokenizer(settings, *this); - ASSERT(tokenizer.tokenize(code)); - clearValues(tokenizer); - const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); - ASSERT_EQUALS(4U, exprs.size()); + const std::vector results = evaluateAssignments(code); + ASSERT_EQUALS(4U, results.size()); // 3 < s.size() < 10 - ASSERT_EQUALS("0", evaluate(exprs[0])); - ASSERT_EQUALS("1", evaluate(exprs[1])); - ASSERT_EQUALS("0", evaluate(exprs[2])); - ASSERT_EQUALS("", evaluate(exprs[3])); + ASSERT_EQUALS("0", results[0]); + ASSERT_EQUALS("1", results[1]); + ASSERT_EQUALS("0", results[2]); + ASSERT_EQUALS("", results[3]); } }; From 9010116812ea201a32f29a2a6be123295cdb97fe Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 27 Sep 2026 16:33:22 -0500 Subject: [PATCH 5/9] Make executeImpl return a list --- lib/programmemory.cpp | 399 +++++++++++++++++++++---------------- test/testprogrammemory.cpp | 13 +- 2 files changed, 235 insertions(+), 177 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index a2a7b36c1c2..0a0465d007a 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -449,6 +449,25 @@ static bool isTrueOrFalse(const ValueFlow::Value& v, bool b) return isFalse(v); } +// Is the expression with these values known to be nonzero? It is when its value is, or when one of +// its constraints excludes zero. +static bool isTrue(const ProgramMemory::Values& values) +{ + return std::any_of(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) { + return isTrue(v); + }); +} + +static bool isFalse(const ProgramMemory::Values& values) +{ + return values.size() == 1 && isFalse(values.front()); +} + +static bool isTrueOrFalse(const ProgramMemory::Values& values, bool b) +{ + return b ? isTrue(values) : isFalse(values); +} + // If the scope is a non-range for loop static bool isBasicForLoop(const Token* tok) { @@ -1557,6 +1576,8 @@ static void pruneConditions(std::vector& conds, namespace { struct Executor { + using Values = ProgramMemory::Values; + ProgramMemory* pm; const Settings& settings; // Values tracked by the forward/reverse analysis. A tracked value is the authoritative @@ -1571,7 +1592,7 @@ namespace { } // The tracked values for this expression, if there are any - const ProgramMemory::Values* getTrackedValues(const Token* expr) const + const Values* getTrackedValues(const Token* expr) const { if (!vars || expr->exprId() == 0) return nullptr; @@ -1590,40 +1611,65 @@ namespace { }) != nullptr; } + static ValueFlow::Value unknown() { + return ValueFlow::Value::unknown(); + } + // The one value to read for an expression: its value, or the first of its constraints (every // one of them holds for the expression) - static ValueFlow::Value representative(const ProgramMemory::Values& values) + static ValueFlow::Value representative(const Values& values) { return values.empty() ? unknown() : values.front(); } + // The values of an expression that has the one value; none when it is unknown + static Values single(ValueFlow::Value value) + { + Values values; + if (!value.isUninitValue()) + values.push_back(std::move(value)); + return values; + } + + // The value of the expression when it has exactly one, and not constraints + static const ValueFlow::Value* getSingleValue(const Values& values) + { + if (values.size() != 1 || values.front().isImpossible()) + return nullptr; + return &values.front(); + } + + // The value of a condition: constraints that exclude zero are true, a value stands for itself + static ValueFlow::Value conditionValue(const Values& values) + { + if (!values.empty() && values.front().isImpossible() && isTrue(values)) + return ValueFlow::Value{1}; + return representative(values); + } + // The values recorded for the expression, when it is read from the program memory: it has no // known value and does not depend on a tracked value - const ProgramMemory::Values* getStoredValues(const Token* expr) const + const Values* getStoredValues(const Token* expr) const { if (expr->exprId() == 0) return nullptr; - const ProgramMemory::Values* stored = pm->getValues(expr->exprId()); + const Values* stored = pm->getValues(expr->exprId()); if (!stored || expr->hasKnownIntValue() || dependsOnTrackedValue(expr)) return nullptr; return stored; } - 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()) + const Values r = state.execute(tok); + if (r.empty()) continue; const bool brk = b && isTrueOrFalse(r, *b); - result.emplace(tok->exprId(), std::move(r)); + result.emplace(tok->exprId(), conditionValue(r)); // Short-circuit evaluation if (brk) break; @@ -1656,14 +1702,14 @@ namespace { // 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()); + const Values lhs = execute(expr->astOperand1()); if (isTrueOrFalse(lhs, b)) - return lhs; - ValueFlow::Value rhs = execute(expr->astOperand2()); + return conditionValue(lhs); + const Values rhs = execute(expr->astOperand2()); if (isTrueOrFalse(rhs, b)) - return rhs; + return conditionValue(rhs); if (isTrueOrFalse(lhs, !b) && isTrueOrFalse(rhs, !b)) - return lhs; + return conditionValue(lhs); return unknown(); } @@ -1734,9 +1780,9 @@ namespace { // Get the size values of the container. If the container itself is not tracked in the // program memory then check if it is symbolically equal to a container whose size is tracked. - ProgramMemory::Values executeContainerSizes(const Token* containerTok) + Values executeContainerSizes(const Token* containerTok) { - ProgramMemory::Values sizes = executeValues(containerTok); + Values sizes = execute(containerTok); sizes.remove_if([](const ValueFlow::Value& v) { return !v.isContainerSizeValue(); }); @@ -1763,111 +1809,92 @@ namespace { } // The size values of the container, as ints - ProgramMemory::Values executeSizeYield(const Token* containerTok) + Values executeSizeYield(const Token* containerTok) { - ProgramMemory::Values sizes = executeContainerSizes(containerTok); + Values sizes = executeContainerSizes(containerTok); for (ValueFlow::Value& v : sizes) v.valueType = ValueFlow::Value::ValueType::INT; return sizes; } - // All values of the expression: the constraints of a range read from the program memory, - // or the single result of execute() - ProgramMemory::Values executeValues(const Token* expr) - { - if (expr->exprId() > 0) { - // Several constraints are read as they are. Whether they apply is checked only then, - // as that walks the expression. - const ProgramMemory::Values* stored = pm->getValues(expr->exprId()); - if (stored && stored->size() > 1 && !expr->hasKnownIntValue() && !dependsOnTrackedValue(expr)) - return *stored; - } - if (const Token* containerTok = settings.library.getContainerFromYield(expr, Library::Container::Yield::SIZE)) - return executeSizeYield(containerTok); - ProgramMemory::Values values; - ValueFlow::Value v = execute(expr); - if (!v.isUninitValue()) - values.push_back(std::move(v)); - return values; - } - - ValueFlow::Value executeImpl(const Token* expr) + // The values of the expression: its one value, or the constraints it is known to satisfy + Values executeImpl(const Token* expr) { const ValueFlow::Value* value = nullptr; - if (!expr) - return unknown(); if (expr->hasKnownIntValue() && !expr->isAssignmentOp() && expr->str() != ",") - return *expr->getKnownValue(ValueFlow::Value::ValueType::INT); + return single(*expr->getKnownValue(ValueFlow::Value::ValueType::INT)); if ((value = expr->getKnownValue(ValueFlow::Value::ValueType::FLOAT)) || (value = expr->getKnownValue(ValueFlow::Value::ValueType::TOK)) || (value = expr->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_START)) || (value = expr->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_END)) || (value = expr->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))) { - return *value; + return single(*value); } if (expr->isNumber()) { if (MathLib::isFloat(expr->str())) - return unknown(); + return {}; MathLib::bigint i = MathLib::toBigNumber(expr); if (i < 0 && astIsUnsigned(expr)) - return unknown(); - return ValueFlow::Value{i}; + return {}; + return single(ValueFlow::Value{i}); } if (expr->isBoolean()) - return ValueFlow::Value{expr->str() == "true"}; + return single(ValueFlow::Value{expr->str() == "true"}); if (const Token* containerTok = settings.library.getContainerFromYield(expr, Library::Container::Yield::SIZE)) { - return representative(executeSizeYield(containerTok)); + return executeSizeYield(containerTok); } if (const Token* containerTok = settings.library.getContainerFromYield(expr, Library::Container::Yield::EMPTY)) { - ValueFlow::Value v = containerEmptyValue(executeContainerSizes(containerTok)); + const ValueFlow::Value v = containerEmptyValue(executeContainerSizes(containerTok)); if (!v.isUninitValue()) - return v; + return single(v); } else if (expr->isAssignmentOp() && expr->astOperand1() && expr->astOperand2() && expr->astOperand1()->exprId() > 0) { - ValueFlow::Value rhs = execute(expr->astOperand2()); - if (rhs.isUninitValue()) - return unknown(); + Values rhs = execute(expr->astOperand2()); + if (rhs.empty()) + return {}; if (expr->str() != "=") { if (!pm->hasValue(expr->astOperand1()->exprId())) - return unknown(); - ProgramMemory::Values& lhs = pm->at(expr->astOperand1()->exprId()); + return {}; + // Constraints of the right hand side cannot be combined with the values + const ValueFlow::Value& delta = rhs.front(); + Values& lhs = pm->at(expr->astOperand1()->exprId()); for (ValueFlow::Value& v : lhs) { - const ValueFlow::Value r = evaluate(expr, v, rhs, /*removeAssign*/ true); + const ValueFlow::Value r = evaluate(expr, v, delta, /*removeAssign*/ true); if (r.isUninitValue()) { lhs.assign(1, unknown()); - return unknown(); + return {}; } if (v.isIntValue()) ValueFlow::Value::visitValue(r, std::bind(assign{}, std::ref(v.intvalue), std::placeholders::_1)); else if (v.isFloatValue()) ValueFlow::Value::visitValue(r, std::bind(assign{}, std::ref(v.floatValue), std::placeholders::_1)); else - return unknown(); + return {}; // The operation may have turned the range around or dissolved it v.bound = r.bound; } - return representative(lhs); + return lhs; } - pm->setValue(expr->astOperand1(), rhs); + pm->setValues(expr->astOperand1(), rhs); return rhs; } else if (expr->str() == "&&" && expr->astOperand1() && expr->astOperand2()) { - return executeMultiCondition(false, expr); + return single(executeMultiCondition(false, expr)); } else if (expr->str() == "||" && expr->astOperand1() && expr->astOperand2()) { - return executeMultiCondition(true, expr); + return single(executeMultiCondition(true, expr)); } else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) { execute(expr->astOperand1()); return execute(expr->astOperand2()); } else if (expr->tokType() == Token::eIncDecOp && expr->astOperand1() && expr->astOperand1()->exprId() != 0) { if (!pm->hasValue(expr->astOperand1()->exprId())) - return ValueFlow::Value::unknown(); - ProgramMemory::Values& lhs = pm->at(expr->astOperand1()->exprId()); + return {}; + Values& lhs = pm->at(expr->astOperand1()->exprId()); // The values of an expression all have the same type if (!lhs.front().isIntValue()) - return unknown(); + return {}; // An unsigned value wraps around when it is decremented and may be zero - if (expr->str() == "--" && astIsUnsigned(expr->astOperand1()) && std::none_of(lhs.cbegin(), lhs.cend(), &isTrue)) { + if (expr->str() == "--" && astIsUnsigned(expr->astOperand1()) && !isTrue(lhs)) { lhs.assign(1, unknown()); - return unknown(); + return {}; } // Shift every value of the variable; bounds and impossible values move along @@ -1877,7 +1904,7 @@ namespace { else v.intvalue--; } - return representative(lhs); + return lhs; } else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) { const Token* tokvalue = nullptr; if (!pm->getTokValue(expr->astOperand1()->exprId(), tokvalue)) { @@ -1885,38 +1912,52 @@ namespace { expr->astOperand1()->values().cend(), std::mem_fn(&ValueFlow::Value::isTokValue)); if (tokvalue_it == expr->astOperand1()->values().cend() || !tokvalue_it->isKnown()) { - return unknown(); + return {}; } tokvalue = tokvalue_it->tokvalue; } if (!tokvalue || !tokvalue->isLiteral()) { - return unknown(); + return {}; } const std::string strValue = tokvalue->strValue(); - ValueFlow::Value rhs = execute(expr->astOperand2()); - if (!rhs.isIntValue() || rhs.isImpossible()) - return unknown(); - const MathLib::bigint index = rhs.intvalue; - if (index >= 0 && index < strValue.size()) - return ValueFlow::Value{strValue[index]}; - if (index == strValue.size()) - return ValueFlow::Value{}; + const Values rhs = execute(expr->astOperand2()); + const ValueFlow::Value* index = getSingleValue(rhs); + if (!index || !index->isIntValue()) + return {}; + if (index->intvalue >= 0 && index->intvalue < strValue.size()) + return single(ValueFlow::Value{strValue[index->intvalue]}); + if (index->intvalue == strValue.size()) + return single(ValueFlow::Value{}); } else if (Token::Match(expr, "%cop%") && expr->astOperand1() && expr->astOperand2()) { - ProgramMemory::Values lhsValues = executeValues(expr->astOperand1()); + Values lhsValues = execute(expr->astOperand1()); if (lhsValues.empty()) - return unknown(); - ProgramMemory::Values rhsValues = executeValues(expr->astOperand2()); + return {}; + Values rhsValues = execute(expr->astOperand2()); if (rhsValues.empty()) - return unknown(); - ValueFlow::Value lhs = representative(lhsValues); - ValueFlow::Value rhs = representative(rhsValues); - // Compare ranges: an operand with constraints (the values of an operand are either one - // value or all constraints) is compared as the interval they describe - if (expr->isComparisonOp() && (lhs.isImpossible() || rhs.isImpossible())) { + return {}; + // The values of an operand are either one value or all constraints + const bool lhsConstraints = lhsValues.front().isImpossible(); + const bool rhsConstraints = rhsValues.front().isImpossible(); + if (!expr->isComparisonOp() && lhsConstraints != rhsConstraints) { + // Apply the operation to each constraint; a constraint it cannot transform is dropped + const Values& constraints = lhsConstraints ? lhsValues : rhsValues; + const ValueFlow::Value& other = lhsConstraints ? rhsValues.front() : lhsValues.front(); + Values result; + for (const ValueFlow::Value& constraint : constraints) { + ValueFlow::Value r = lhsConstraints ? evaluate(expr, constraint, other) : evaluate(expr, other, constraint); + if (!r.isUninitValue()) + result.push_back(std::move(r)); + } + return result; + } + ValueFlow::Value lhs = lhsValues.front(); + ValueFlow::Value rhs = rhsValues.front(); + // Compare ranges: an operand with constraints is compared as the interval they describe + if (expr->isComparisonOp() && (lhsConstraints || rhsConstraints)) { std::vector result = infer(makeIntegralInferModel(), expr->str(), std::move(lhsValues), std::move(rhsValues)); if (!result.empty()) - return std::move(result.front()); + return single(std::move(result.front())); } ValueFlow::Value r = evaluate(expr, lhs, rhs); if (expr->isComparisonOp() && (r.isUninitValue() || r.isImpossible())) { @@ -1926,7 +1967,7 @@ namespace { expr->astOperand1()->values(), {std::move(rhs)}); if (!result.empty() && result.front().isKnown()) - return std::move(result.front()); + return single(std::move(result.front())); } if (lhs.isIntValue() && !expr->astOperand2()->values().empty()) { std::vector result = infer(makeIntegralInferModel(), @@ -1934,123 +1975,131 @@ namespace { {std::move(lhs)}, expr->astOperand2()->values()); if (!result.empty() && result.front().isKnown()) - return std::move(result.front()); + return single(std::move(result.front())); } - return unknown(); + return {}; } - return r; + return single(std::move(r)); } // Unary ops else if (Token::Match(expr, "!|+|-") && expr->astOperand1() && !expr->astOperand2()) { - ValueFlow::Value lhs = execute(expr->astOperand1()); - if (!lhs.isIntValue()) - return unknown(); + Values lhs = execute(expr->astOperand1()); + if (lhs.empty() || !lhs.front().isIntValue()) + return {}; if (expr->str() == "!") { + ValueFlow::Value result = lhs.front(); if (isTrue(lhs)) { - lhs.intvalue = 0; + result.intvalue = 0; } else if (isFalse(lhs)) { - lhs.intvalue = 1; + result.intvalue = 1; } else { - return unknown(); + return {}; } - lhs.setPossible(); - lhs.bound = ValueFlow::Value::Bound::Point; + result.setPossible(); + result.bound = ValueFlow::Value::Bound::Point; + return single(std::move(result)); } if (expr->str() == "-") { - lhs.intvalue = -lhs.intvalue; - lhs.invertBound(); + for (ValueFlow::Value& v : lhs) { + v.intvalue = -v.intvalue; + v.invertBound(); + } } return lhs; } else if (expr->str() == "?" && expr->astOperand1() && expr->astOperand2()) { - ValueFlow::Value cond = execute(expr->astOperand1()); - if (!cond.isIntValue()) - return unknown(); + const Values cond = execute(expr->astOperand1()); + if (cond.empty() || !cond.front().isIntValue()) + return {}; const Token* child = expr->astOperand2(); if (isFalse(cond)) return execute(child->astOperand2()); if (isTrue(cond)) return execute(child->astOperand1()); - return unknown(); + return {}; } else if (expr->str() == "(" && expr->isCast()) { if (expr->astOperand2()) { if (expr->astOperand1()->str() != "dynamic_cast") return execute(expr->astOperand2()); - return unknown(); + return {}; } return execute(expr->astOperand1()); } - // Return the tracked value and write it back when it differs, so later reads see the - // same value (as fillProgramMemoryFromAssignments used to do). - if (const ProgramMemory::Values* tracked = getTrackedValues(expr)) { - const ProgramMemory::Values* stored = pm->getValues(expr->exprId()); + // Return the tracked values and write them back when they differ, so later reads see the + // same values (as fillProgramMemoryFromAssignments used to do). + if (const Values* tracked = getTrackedValues(expr)) { + const Values* stored = pm->getValues(expr->exprId()); if (!stored || *stored != *tracked) pm->setValues(expr, *tracked); - return representative(*tracked); + return *tracked; } - if (const ProgramMemory::Values* stored = getStoredValues(expr)) { + if (const Values* stored = getStoredValues(expr)) { // An impossible value that excludes zero makes the expression true as a bool if (std::any_of(stored->cbegin(), stored->cend(), [](const ValueFlow::Value& v) { return v.isImpossible() && v.isIntValue() && isTrue(v); }) && isUsedAsBool(expr, settings)) { ValueFlow::Value result{1}; result.setKnown(); - return result; + return single(std::move(result)); } - return representative(*stored); + return *stored; } if (Token::Match(expr->previous(), ">|%name% {|(")) { const Token* ftok = expr->previous(); const Function* f = ftok->function(); - ValueFlow::Value result = unknown(); + Values result; if (expr->str() == "(") { - std::vector tokArgs = getArguments(expr); - std::vector args(tokArgs.size()); - std::transform( - tokArgs.cbegin(), tokArgs.cend(), args.begin(), [&](const Token* tok) { - return execute(tok); - }); + const std::vector tokArgs = getArguments(expr); + std::vector args; + for (const Token* tok : tokArgs) + args.push_back(execute(tok)); if (f) { if (fdepth >= 0 && !f->isImplicitlyVirtual()) { ProgramMemory functionState; for (std::size_t i = 0; i < args.size(); ++i) { const Variable* const arg = f->getArgumentVar(i); if (!arg) - return unknown(); - functionState.setValue(arg->nameToken(), args[i]); + return {}; + functionState.setValues(arg->nameToken(), args[i]); } Executor ex = *this; ex.pm = &functionState; ex.fdepth--; - auto r = ex.execute(f->functionScope); - if (!r.empty()) - result = std::move(r.front()); + for (const ValueFlow::Value& v : ex.execute(f->functionScope)) { + if (!v.isUninitValue()) + result.push_back(v); + } // TODO: Track values changed by reference } } else { - BuiltinLibraryFunction lf = getBuiltinLibraryFunction(ftok->str()); - // The builtin functions compute with values, not with constraints - if (lf && std::none_of(args.cbegin(), args.cend(), std::mem_fn(&ValueFlow::Value::isImpossible))) - return lf(args); - if (lf) - return unknown(); + if (BuiltinLibraryFunction lf = getBuiltinLibraryFunction(ftok->str())) { + // The builtin functions compute with values, not with constraints + std::vector argValues; + for (const Values& a : args) { + const ValueFlow::Value* v = getSingleValue(a); + if (!v) + return {}; + argValues.push_back(*v); + } + return single(lf(argValues)); + } const std::string& returnValue = settings.library.returnValue(ftok); if (!returnValue.empty()) { std::unordered_map arg_map; int argn = 0; - for (const ValueFlow::Value& v : args) { - if (!v.isUninitValue()) - arg_map[argn] = v; + for (const Values& a : args) { + if (!a.empty()) + arg_map[argn] = a.front(); argn++; } - return evaluateLibraryFunction(arg_map, returnValue, settings, ftok->isCpp()); + return single(evaluateLibraryFunction(arg_map, returnValue, settings, ftok->isCpp())); } } } // Check if function modifies argument visitAstNodes(expr->astOperand2(), [&](const Token* child) { - const ProgramMemory::Values* values = child->exprId() > 0 ? pm->getValues(child->exprId()) : nullptr; + const Values* values = child->exprId() > 0 ? pm->getValues(child->exprId()) : nullptr; if (values) { // The values of an expression all have the same type const ValueFlow::Value& v = values->front(); @@ -2067,7 +2116,7 @@ namespace { return result; } - return unknown(); + return {}; } static const ValueFlow::Value* getImpossibleValue(const Token* tok) { @@ -2090,31 +2139,25 @@ namespace { return *it; } - static bool updateValue(ValueFlow::Value& v, ValueFlow::Value x) - { - const bool returnValue = !x.isUninitValue() && !x.isImpossible(); - if (v.isUninitValue() || returnValue) - v = std::move(x); - return returnValue; - } - - ValueFlow::Value execute(const Token* expr) + // The values of the expression. When it does not evaluate to a value, the program memory and + // the values of the token may still constrain it. + Values execute(const Token* expr) { depth--; OnExit onExit{[&] { depth++; }}; - if (depth < 0) - return unknown(); - ValueFlow::Value v = unknown(); - if (updateValue(v, executeImpl(expr))) - return v; - if (!expr) - return v; + if (depth < 0 || !expr) + return {}; + Values values = executeImpl(expr); + if (!values.empty() && !values.front().isImpossible()) + return values; if (expr->exprId() > 0) { - if (const ProgramMemory::Values* stored = pm->getValues(expr->exprId())) { - if (updateValue(v, representative(*stored))) - return v; + if (const Values* stored = pm->getValues(expr->exprId())) { + if (!stored->front().isImpossible()) + return *stored; + if (values.empty()) + values = *stored; } } // Find symbolic values @@ -2123,18 +2166,18 @@ namespace { continue; if (!value.isKnown()) continue; - const ProgramMemory::Values* stored = pm->getValues(value.tokvalue->exprId()); + const Values* stored = pm->getValues(value.tokvalue->exprId()); if (!stored || (!stored->front().isIntValue() && value.intvalue != 0)) continue; ValueFlow::Value v2 = stored->front(); v2.intvalue += value.intvalue; - return v2; + return single(std::move(v2)); } - if (v.isImpossible() && v.isIntValue()) - return v; - if (const ValueFlow::Value* value = getImpossibleValue(expr)) - return *value; - return v; + if (!values.empty() && values.front().isIntValue()) + return values; + if (const ValueFlow::Value* impossible = getImpossibleValue(expr)) + return single(*impossible); + return values; } std::vector execute(const Scope* scope) @@ -2146,11 +2189,15 @@ namespace { for (const Token* tok = scope->bodyStart->next(); precedes(tok, scope->bodyEnd); tok = tok->next()) { const Token* top = tok->astTop(); - if (Token::simpleMatch(top, "return") && top->astOperand1()) - return {execute(top->astOperand1())}; + if (Token::simpleMatch(top, "return") && top->astOperand1()) { + const Values values = execute(top->astOperand1()); + if (values.empty()) + return {unknown()}; + return std::vector(values.cbegin(), values.cend()); + } if (Token::Match(top, "%op%")) { - if (execute(top).isUninitValue()) + if (execute(top).empty()) return {unknown()}; const Token* next = nextAfterAstRightmostLeaf(top); if (!next) @@ -2158,8 +2205,8 @@ namespace { tok = next; } else if (Token::simpleMatch(top->previous(), "if (")) { const Token* condTok = top->astOperand2(); - ValueFlow::Value v = execute(condTok); - if (!v.isIntValue()) + const Values cond = execute(condTok); + if (cond.empty() || !cond.front().isIntValue()) return {unknown()}; const Token* thenStart = top->link()->next(); const Token* next = thenStart->link(); @@ -2169,9 +2216,9 @@ namespace { next = elseStart->link(); } std::vector result; - if (isTrue(v)) { + if (isTrue(cond)) { result = execute(thenStart->scope()); - } else if (isFalse(v)) { + } else if (isFalse(cond)) { if (elseStart) result = execute(elseStart->scope()); } else { @@ -2196,7 +2243,7 @@ static ValueFlow::Value execute(const Token* expr, { Executor ex{&pm, settings}; ex.vars = &vars; - return ex.execute(expr); + return Executor::representative(ex.execute(expr)); } static ProgramMemory::Values executeValues(const Token* expr, @@ -2206,7 +2253,7 @@ static ProgramMemory::Values executeValues(const Token* expr, { Executor ex{&pm, settings}; ex.vars = &vars; - return ex.executeValues(expr); + return ex.execute(expr); } std::vector execute(const Scope* scope, ProgramMemory& pm, const Settings& settings) diff --git a/test/testprogrammemory.cpp b/test/testprogrammemory.cpp index 9d0a3d6ad49..2deaee43a2f 100644 --- a/test/testprogrammemory.cpp +++ b/test/testprogrammemory.cpp @@ -340,11 +340,16 @@ class TestProgramMemory : public TestFixture { " y = 10 - x < 7;\n" " y = -x < 0;\n" " y = x;\n" + " y = x + 1 < 20;\n" + " y = -x > -20;\n" + " y = (long)x < 20;\n" + " y = (x > 0 ? x : 0) < 20;\n" + " y = 2 * x - 1 == 3;\n" " }\n" " }\n" "}\n"; const std::vector results = evaluateAssignments(code); - ASSERT_EQUALS(8U, results.size()); + ASSERT_EQUALS(13U, results.size()); // 3 < x < 10 ASSERT_EQUALS("0", results[0]); ASSERT_EQUALS("", results[1]); @@ -356,6 +361,12 @@ class TestProgramMemory : public TestFixture { ASSERT_EQUALS("1", results[6]); // a range is not a value ASSERT_EQUALS("", results[7]); + // both bounds follow the value through arithmetic, casts and conditionals + ASSERT_EQUALS("1", results[8]); + ASSERT_EQUALS("1", results[9]); + ASSERT_EQUALS("1", results[10]); + ASSERT_EQUALS("1", results[11]); + ASSERT_EQUALS("0", results[12]); } void executeScaledRange() { From 8d4f066dfca0174538a663c1e3cf3664de6b91aa Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 27 Sep 2026 17:28:37 -0500 Subject: [PATCH 6/9] Add test for 15042 --- test/testcondition.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/testcondition.cpp b/test/testcondition.cpp index c7f39584939..76513d639c3 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -934,6 +934,20 @@ class TestCondition : public TestFixture { "}\n"); ASSERT_EQUALS("", errout_str()); + // the condition 'length > 1' does not make length equal to 2, so 'length > 2' is not dead + check("void f(int length, unsigned int& dst) {\n" + " unsigned int src2 = 0U;\n" + " if (length > 1) {\n" + " if (length > 2) {\n" + " src2 = 15U;\n" + " }\n" + " }\n" + " if (length >= 2) {\n" + " dst = ((0x80) | (src2 >> 2));\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + check("const int FEATURE_BITS = x |\n" "#if FOO_ENABLED\n" " FEATURE_FOO |\n" From f6f61fb074f7a950e787407a454443d0e7da3eb1 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 28 Sep 2026 09:18:32 -0500 Subject: [PATCH 7/9] Fix CI issues --- lib/programmemory.cpp | 9 ++++++++- lib/programmemory.h | 14 +++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 0a0465d007a..a3f76277ce6 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -369,13 +369,18 @@ void ProgramMemory::replace(ProgramMemory pm, bool skipUnknown) copyOnWrite(); + // The values can be moved out of the given memory only when no other memory shares them + const bool owned = pm.mValues.use_count() == 1; for (auto&& p : (*pm.mValues)) { if (skipUnknown) { auto it = mValues->find(p.first); if (it != mValues->end() && isUnknown(it->second)) continue; } - (*mValues)[p.first] = std::move(p.second); + if (owned) + (*mValues)[p.first] = std::move(p.second); + else + (*mValues)[p.first] = p.second; } } @@ -2052,6 +2057,7 @@ namespace { if (expr->str() == "(") { const std::vector tokArgs = getArguments(expr); std::vector args; + args.reserve(tokArgs.size()); for (const Token* tok : tokArgs) args.push_back(execute(tok)); if (f) { @@ -2076,6 +2082,7 @@ namespace { if (BuiltinLibraryFunction lf = getBuiltinLibraryFunction(ftok->str())) { // The builtin functions compute with values, not with constraints std::vector argValues; + argValues.reserve(args.size()); for (const Values& a : args) { const ValueFlow::Value* v = getSingleValue(a); if (!v) diff --git a/lib/programmemory.h b/lib/programmemory.h index b2b05513eb3..27ea9cfbf80 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -190,7 +190,7 @@ struct CPPCHECKLIB ProgramMemory { std::shared_ptr mValues; }; -struct ProgramMemoryState { +struct CPPCHECKLIB ProgramMemoryState { struct ChangedKeyHash { std::size_t operator()(const std::tuple& t) const { @@ -230,12 +230,12 @@ struct ProgramMemoryState { std::vector execute(const Scope* scope, ProgramMemory& pm, const Settings& settings); -void execute(const Token* expr, - ProgramMemory& programMemory, - MathLib::bigint* result, - bool* error, - const Settings& settings, - const ProgramMemory::Map& vars = {}); +CPPCHECKLIB void execute(const Token* expr, + ProgramMemory& programMemory, + MathLib::bigint* result, + bool* error, + const Settings& settings, + const ProgramMemory::Map& vars = {}); /** * Is condition always false when variable has given value? From 25e26ff8c4670c90c88bc914a3f64724b214f15b Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 28 Sep 2026 15:09:17 -0500 Subject: [PATCH 8/9] Fix CI failures --- lib/programmemory.cpp | 19 ++++++++++--------- test/testprogrammemory.cpp | 7 +++++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index a3f76277ce6..2451cd12385 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -273,10 +273,10 @@ bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint& r const Values* values = getValues(exprid); if (!values) return false; - const ValueFlow::Value empty = containerEmptyValue(*values); - if (empty.isUninitValue()) + const ValueFlow::Value isEmpty = containerEmptyValue(*values); + if (isEmpty.isUninitValue()) return false; - result = empty.intvalue; + result = isEmpty.intvalue; return true; } @@ -2058,8 +2058,9 @@ namespace { const std::vector tokArgs = getArguments(expr); std::vector args; args.reserve(tokArgs.size()); - for (const Token* tok : tokArgs) - args.push_back(execute(tok)); + std::transform(tokArgs.cbegin(), tokArgs.cend(), std::back_inserter(args), [&](const Token* tok) { + return execute(tok); + }); if (f) { if (fdepth >= 0 && !f->isImplicitlyVirtual()) { ProgramMemory functionState; @@ -2072,10 +2073,10 @@ namespace { Executor ex = *this; ex.pm = &functionState; ex.fdepth--; - for (const ValueFlow::Value& v : ex.execute(f->functionScope)) { - if (!v.isUninitValue()) - result.push_back(v); - } + const std::vector returned = ex.execute(f->functionScope); + std::copy_if(returned.cbegin(), returned.cend(), std::back_inserter(result), [](const ValueFlow::Value& v) { + return !v.isUninitValue(); + }); // TODO: Track values changed by reference } } else { diff --git a/test/testprogrammemory.cpp b/test/testprogrammemory.cpp index 2deaee43a2f..acc38f235e2 100644 --- a/test/testprogrammemory.cpp +++ b/test/testprogrammemory.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -322,9 +323,11 @@ class TestProgramMemory : public TestFixture { SimpleTokenizer tokenizer(settings, *this); ASSERT(tokenizer.tokenize(code)); clearValues(tokenizer); + const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); std::vector results; - for (const Token* expr : assignedExpressions(tokenizer.tokens(), "y")) - results.push_back(evaluate(expr)); + std::transform(exprs.cbegin(), exprs.cend(), std::back_inserter(results), [&](const Token* expr) { + return evaluate(expr); + }); return results; } From 9c933a04c16883f8e1245d83be774606f94880ad Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Tue, 29 Sep 2026 09:09:32 -0500 Subject: [PATCH 9/9] Limit values size to a maximum of 10 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- lib/programmemory.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 2451cd12385..1f977dcd218 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -183,6 +183,8 @@ void ProgramMemory::record(const Token* expr, const ValueFlow::Value& value) !values.front().isImpossible()) { values.assign(1, value); } else { + if (values.size() >= 10U) + return; values.push_back(value); Token::removeContradictions(values); }