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 142385ac641..2451cd12385 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -31,12 +31,15 @@ #include "utils.h" #include "valueflow.h" #include "valueptr.h" +#include "vf_common.h" #include #include #include +#include #include #include +#include #include #include #include @@ -63,12 +66,89 @@ 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; +} + +// Is the value (a range when it is impossible with a bound) known to be nonzero? +static bool isTrue(const ValueFlow::Value& v) +{ + 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; +} + +static bool isFalse(const ValueFlow::Value& v) +{ + if (v.isUninitValue()) + return false; + if (v.isImpossible()) + return false; + return v.intvalue == 0; +} + +// Does the value satisfy the constraint of the same type? +static bool satisfies(const ValueFlow::Value& value, const ValueFlow::Value& constraint) +{ + if (isImpossiblePoint(constraint)) + return !value.equalValue(constraint); + if (isLowerBound(constraint)) + return value.intvalue >= constraint.rangeEdge(); + if (isUpperBound(constraint)) + return value.intvalue <= constraint.rangeEdge(); + return false; +} + +static bool sameValue(const ValueFlow::Value& x, const ValueFlow::Value& y) +{ + return x == y && x.bound == y.bound; +} + +// 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) +{ + 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) { if (!expr) return; - copyOnWrite(); - ValueFlow::Value subvalue = value; const Token* subexpr = solveExprValue( expr, @@ -81,19 +161,56 @@ void ProgramMemory::setValue(const Token* expr, const ValueFlow::Value& value) { return {}; }, subvalue); + if (expr != subexpr) - (*mValues)[expr] = value; + record(expr, value); if (subexpr) - (*mValues)[subexpr] = std::move(subvalue); + record(subexpr, subvalue); +} + +void ProgramMemory::record(const Token* expr, const ValueFlow::Value& value) +{ + const Values* existing = getValues(expr->exprId()); + if (existing && isRecorded(*existing, value)) + return; + copyOnWrite(); + 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) +{ + 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; } bool ProgramMemory::getIntValue(nonneg int exprid, MathLib::bigint& result) const @@ -134,20 +251,33 @@ bool ProgramMemory::getContainerSizeValue(nonneg int exprid, MathLib::bigint& re } 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 (isUpperBound(value) && value.rangeEdge() <= 0) + return ValueFlow::Value{1}; + if (isTrue(value)) + 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 isEmpty = containerEmptyValue(*values); + if (isEmpty.isUninitValue()) + return false; + result = isEmpty.intvalue; + return true; } void ProgramMemory::setContainerSizeValue(const Token* expr, MathLib::bigint value, bool equal) @@ -162,7 +292,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 +301,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 +309,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 +355,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) { @@ -233,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() && it->second.isUninitValue()) + 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; } } @@ -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, @@ -300,29 +447,30 @@ static bool frontIs(const std::vector& v, bool i) return !i; } -static bool isTrue(const ValueFlow::Value& v) +static bool isTrueOrFalse(const ValueFlow::Value& v, bool b) { - if (v.isUninitValue()) - return false; - if (v.isImpossible()) - return v.intvalue == 0; - return v.intvalue != 0; + if (b) + return isTrue(v); + return isFalse(v); } -static bool isFalse(const ValueFlow::Value& 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) { - if (v.isUninitValue()) - return false; - if (v.isImpossible()) - return false; - return v.intvalue == 0; + return std::any_of(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) { + return isTrue(v); + }); } -static bool isTrueOrFalse(const ValueFlow::Value& v, bool b) +static bool isFalse(const ProgramMemory::Values& values) { - if (b) - return isTrue(v); - return isFalse(v); + 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 @@ -381,11 +529,17 @@ 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(std::move(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) { + v.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + pm.setValue(containerTok, v); + } } else if (Token::simpleMatch(tok, "!")) { programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then, findChanged); } else if (then && Token::simpleMatch(tok, "&&")) { @@ -458,7 +612,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 +701,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 +813,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,15 +846,83 @@ 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 bool multiplyOverflows(MathLib::bigint x, MathLib::bigint y) +{ + if (x == 0 || y == 0) + return false; + if (ValueFlow::isSaturated(x) || ValueFlow::isSaturated(y)) + return true; + return std::abs(x) > std::numeric_limits::max() / std::abs(y); +} + +// 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 MathLib::bigint edge = range.rangeEdge(); + if (ValueFlow::isSaturated(edge) || ValueFlow::isSaturated(k)) + return ValueFlow::Value::unknown(); + 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 == "/") { + if (k == 0 || !rangeIsLhs) + return ValueFlow::Value::unknown(); + // Truncation towards zero keeps the order of the values + increasing = k > 0; + result = edge / k; + } else { + // 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 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) { 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(); + // 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 - if (contains({"%", "/", "&", "|"}, opStr)) + // The image of an impossible value is impossible only for an injective operation + if (contains({"%", "/", "&", "|", ">>"}, opStr)) + return ValueFlow::Value::unknown(); + const ValueFlow::Value& factor = lhs.isImpossible() ? rhs : lhs; + if (opStr == "*" && factor.equalTo(0)) return ValueFlow::Value::unknown(); result.setImpossible(); } @@ -1357,6 +1581,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 @@ -1370,8 +1596,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 Values* getTrackedValues(const Token* expr) const { if (!vars || expr->exprId() == 0) return nullptr; @@ -1386,7 +1612,7 @@ namespace { if (!vars || vars->empty()) return false; return findAstNode(expr, [&](const Token* tok) { - return getTrackedValue(tok) != nullptr; + return getTrackedValues(tok) != nullptr; }) != nullptr; } @@ -1394,17 +1620,61 @@ namespace { 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 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 Values* getStoredValues(const Token* expr) const + { + if (expr->exprId() == 0) + return nullptr; + const Values* stored = pm->getValues(expr->exprId()); + if (!stored || expr->hasKnownIntValue() || dependsOnTrackedValue(expr)) + return nullptr; + return stored; + } + 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; @@ -1429,23 +1699,22 @@ 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 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(); } @@ -1477,7 +1746,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 +1783,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. + Values executeContainerSizes(const Token* containerTok) { - ValueFlow::Value v = execute(containerTok); - if (v.isContainerSizeValue()) - return v; + Values sizes = execute(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; @@ -1531,97 +1805,110 @@ 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 unknown(); + return sizes; + } + + // The size values of the container, as ints + Values executeSizeYield(const Token* containerTok) + { + Values sizes = executeContainerSizes(containerTok); + for (ValueFlow::Value& v : sizes) + v.valueType = ValueFlow::Value::ValueType::INT; + return sizes; } - 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"}; - 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 = executeContainerSize(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}; - } + return single(ValueFlow::Value{expr->str() == "true"}); + if (const Token* containerTok = settings.library.getContainerFromYield(expr, Library::Container::Yield::SIZE)) { + return executeSizeYield(containerTok); + } + if (const Token* containerTok = settings.library.getContainerFromYield(expr, Library::Container::Yield::EMPTY)) { + const ValueFlow::Value v = containerEmptyValue(executeContainerSizes(containerTok)); + if (!v.isUninitValue()) + 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(); - 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 {}; + // 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, delta, /*removeAssign*/ true); + if (r.isUninitValue()) { + lhs.assign(1, 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 {}; + // The operation may have turned the range around or dissolved it + v.bound = r.bound; + } 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(); - ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId()); - if (!lhs.isIntValue()) - return unknown(); - // overflow - if (!lhs.isImpossible() && lhs.intvalue == 0 && expr->str() == "--" && astIsUnsigned(expr->astOperand1())) - return unknown(); - - if (expr->str() == "++") - lhs.intvalue++; - else - lhs.intvalue--; + return {}; + Values& lhs = pm->at(expr->astOperand1()->exprId()); + // The values of an expression all have the same type + if (!lhs.front().isIntValue()) + return {}; + // An unsigned value wraps around when it is decremented and may be zero + if (expr->str() == "--" && astIsUnsigned(expr->astOperand1()) && !isTrue(lhs)) { + lhs.assign(1, unknown()); + return {}; + } + + // Shift every value of the variable; bounds and impossible values move along + for (ValueFlow::Value& v : lhs) { + if (expr->str() == "++") + v.intvalue++; + else + v.intvalue--; + } return lhs; } else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) { const Token* tokvalue = nullptr; @@ -1630,29 +1917,53 @@ 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()) - 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()) { - ValueFlow::Value lhs = execute(expr->astOperand1()); - if (lhs.isUninitValue()) - return unknown(); - ValueFlow::Value rhs = execute(expr->astOperand2()); - if (rhs.isUninitValue()) - return unknown(); + Values lhsValues = execute(expr->astOperand1()); + if (lhsValues.empty()) + return {}; + Values rhsValues = execute(expr->astOperand2()); + if (rhsValues.empty()) + 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 single(std::move(result.front())); + } ValueFlow::Value r = evaluate(expr, lhs, rhs); if (expr->isComparisonOp() && (r.isUninitValue() || r.isImpossible())) { if (rhs.isIntValue() && !expr->astOperand1()->values().empty()) { @@ -1661,7 +1972,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(), @@ -1669,76 +1980,85 @@ 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 {}; + } + result.setPossible(); + result.bound = ValueFlow::Value::Bound::Point; + return single(std::move(result)); + } + if (expr->str() == "-") { + for (ValueFlow::Value& v : lhs) { + v.intvalue = -v.intvalue; + v.invertBound(); } - lhs.setPossible(); - lhs.bound = ValueFlow::Value::Bound::Point; } - if (expr->str() == "-") - lhs.intvalue = -lhs.intvalue; 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 ValueFlow::Value* tracked = getTrackedValue(expr)) { - const ValueFlow::Value* stored = pm->getValue(expr->exprId(), /*impossible*/ true); + // 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->setValue(expr, *tracked); + pm->setValues(expr, *tracked); return *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 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 single(std::move(result)); } - return result; + 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) { + const std::vector tokArgs = getArguments(expr); + std::vector args; + args.reserve(tokArgs.size()); + std::transform(tokArgs.cbegin(), tokArgs.cend(), std::back_inserter(args), [&](const Token* tok) { return execute(tok); }); if (f) { @@ -1747,44 +2067,56 @@ namespace { 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()); + 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 { - BuiltinLibraryFunction lf = getBuiltinLibraryFunction(ftok->str()); - if (lf) - return lf(args); + 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) + 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) { - if (child->exprId() > 0 && pm->hasValue(child->exprId())) { - ValueFlow::Value& v = pm->at(child->exprId()); + 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(); 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; @@ -1792,7 +2124,7 @@ namespace { return result; } - return unknown(); + return {}; } static const ValueFlow::Value* getImpossibleValue(const Token* tok) { @@ -1815,30 +2147,26 @@ 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 (expr->exprId() > 0 && pm->hasValue(expr->exprId())) { - if (updateValue(v, utils::as_const(*pm).at(expr->exprId()))) - 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 Values* stored = pm->getValues(expr->exprId())) { + if (!stored->front().isImpossible()) + return *stored; + if (values.empty()) + values = *stored; + } } // Find symbolic values for (const ValueFlow::Value& value : expr->values()) { @@ -1846,20 +2174,18 @@ namespace { continue; if (!value.isKnown()) 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) + const Values* stored = pm->getValues(value.tokvalue->exprId()); + if (!stored || (!stored->front().isIntValue() && value.intvalue != 0)) continue; - ValueFlow::Value v2 = v_ref; + 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) @@ -1871,11 +2197,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) @@ -1883,8 +2213,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(); @@ -1894,9 +2224,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 { @@ -1918,6 +2248,16 @@ static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars) +{ + Executor ex{&pm, settings}; + ex.vars = &vars; + return Executor::representative(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; diff --git a/lib/programmemory.h b/lib/programmemory.h index de81d0bc901..27ea9cfbf80 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 adding a + * constraint does not move the values already recorded. + */ + 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); @@ -154,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); @@ -161,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 { @@ -201,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? 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..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; @@ -5485,32 +5480,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); } } } @@ -6264,6 +6256,31 @@ 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; a negative divisor turns the range around. +static void divideBound(ValueFlow::Value& value, MathLib::bigint divisor) +{ + 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, const std::function(const Token*)>& eval, ValueFlow::Value& value) @@ -6287,19 +6304,31 @@ 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); } case '*': { - if (intval == 0) + if (intval == 0 || ValueFlow::isSaturated(value.intvalue)) break; - value.intvalue /= intval; + 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/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/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/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" diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index ca5c183c911..0da1dbb6982 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,101 @@ 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()); + + // 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 check("void f() {\n" " struct X *x = 0;\n" diff --git a/test/testprogrammemory.cpp b/test/testprogrammemory.cpp index c37431de608..acc38f235e2 100644 --- a/test/testprogrammemory.cpp +++ b/test/testprogrammemory.cpp @@ -19,23 +19,67 @@ #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 +#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(executeScaledRange); + TEST_CASE(executeSolvedRange); + TEST_CASE(executeCompoundAssignment); + 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 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; + }); } void copyOnWrite() const { @@ -94,6 +138,7 @@ class TestProgramMemory : public TestFixture { void getValue() const { ProgramMemory pm; ASSERT(!pm.getValue(123)); + ASSERT(!pm.getValues(123)); } void at() const { @@ -101,6 +146,356 @@ 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(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)); + 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(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(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(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(hasValue(pm.at(id), 7, ValueFlow::Value::Bound::Upper)); + ASSERT(hasValue(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. + // Numbers keep their value, as they always have it. + static void clearValues(SimpleTokenizer& tokenizer) { + 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 + 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. 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); + 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); + const std::vector exprs = assignedExpressions(tokenizer.tokens(), "y"); + std::vector results; + std::transform(exprs.cbegin(), exprs.cend(), std::back_inserter(results), [&](const Token* expr) { + return evaluate(expr); + }); + return results; + } + + 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" + " 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(13U, results.size()); + // 3 < x < 10 + 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", results[4]); + ASSERT_EQUALS("1", results[5]); + 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() { + 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"; + const std::vector results = evaluateAssignments(code); + ASSERT_EQUALS(9U, results.size()); + // x > 3: x * 2 >= 8 + ASSERT_EQUALS("1", results[0]); + ASSERT_EQUALS("0", results[1]); + ASSERT_EQUALS("1", results[2]); + // x * 0 is not "not zero" + 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", results[7]); + ASSERT_EQUALS("1", results[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"; + const std::vector results = evaluateAssignments(code); + ASSERT_EQUALS(8U, results.size()); + // x * 2 < 3: x <= 1 + ASSERT_EQUALS("1", results[0]); + ASSERT_EQUALS("", results[1]); + ASSERT_EQUALS("0", results[2]); + // x * 3 >= 7: x >= 3 + ASSERT_EQUALS("1", results[3]); + ASSERT_EQUALS("0", results[4]); + // -2 * x > 3: x <= -2 + ASSERT_EQUALS("1", results[5]); + ASSERT_EQUALS("0", results[6]); + // (x ^ 4) > 3 does not give a range for x + ASSERT_EQUALS("", results[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); + 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); + ASSERT_EQUALS("", evaluate(exprs[1], pm)); + + // u > 3, then u--: u > 2 + pm.setValue(utok, greaterThan(3)); + execute(utok->next(), pm, nullptr, nullptr, settings); + ASSERT_EQUALS("", evaluate(exprs[1], pm)); + const ProgramMemory::Values* values = pm.getValues(utok->exprId()); + ASSERT(values); + ASSERT_EQUALS(1U, values->size()); + ASSERT(hasValue(*values, 2, ValueFlow::Value::Bound::Upper)); + } + + 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"; + const std::vector results = evaluateAssignments(code); + ASSERT_EQUALS(4U, results.size()); + // 3 < s.size() < 10 + ASSERT_EQUALS("0", results[0]); + ASSERT_EQUALS("1", results[1]); + ASSERT_EQUALS("0", results[2]); + ASSERT_EQUALS("", results[3]); + } }; REGISTER_TEST(TestProgramMemory) 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;