From fdaaa169aea90be6531d71f294b4e3b45c87a932 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20DEVERGE?= Date: Mon, 12 Oct 2026 12:03:30 +0200 Subject: [PATCH] Fix #15000: Propagate integral cast ranges with path-safe value flow --- Makefile | 6 +- lib/checkbufferoverrun.cpp | 59 ++++++++++ lib/checktype.cpp | 43 ++++++- lib/infer.cpp | 196 ++++++++++++++++++++++---------- lib/infer.h | 10 +- lib/token.cpp | 6 +- lib/valueflow.cpp | 85 ++++++++++++-- lib/vf_common.cpp | 227 ++++++++++++++++++++++++++++++++++++- lib/vf_common.h | 7 +- lib/vf_settokenvalue.cpp | 49 ++++++++ oss-fuzz/Makefile | 4 +- test/testbufferoverrun.cpp | 37 ++++++ test/testcondition.cpp | 128 ++++++++++++++++++++- test/testother.cpp | 6 + test/teststl.cpp | 2 +- test/testtype.cpp | 35 ++++++ test/testvalueflow.cpp | 196 ++++++++++++++++++++++++++++++++ 17 files changed, 1005 insertions(+), 91 deletions(-) diff --git a/Makefile b/Makefile index e51cd58ced7..883f5d985b9 100644 --- a/Makefile +++ b/Makefile @@ -675,7 +675,7 @@ $(libcppdir)/templatesimplifier.o: lib/templatesimplifier.cpp lib/checkers.h lib $(libcppdir)/timer.o: lib/timer.cpp lib/config.h lib/timer.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/timer.cpp -$(libcppdir)/token.o: lib/token.cpp externals/simplecpp/simplecpp.h lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.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/tokenrange.h lib/utils.h lib/valueflow.h lib/vfvalue.h +$(libcppdir)/token.o: lib/token.cpp externals/simplecpp/simplecpp.h lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.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/tokenrange.h lib/utils.h lib/valueflow.h lib/vf_common.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/token.cpp $(libcppdir)/tokenlist.o: lib/tokenlist.cpp externals/simplecpp/simplecpp.h lib/astutils.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/keywords.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/vfvalue.h @@ -690,7 +690,7 @@ $(libcppdir)/vf_analyzers.o: lib/vf_analyzers.cpp lib/analyzer.h lib/astutils.h $(libcppdir)/vf_common.o: lib/vf_common.cpp lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vf_common.h lib/vf_settokenvalue.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_common.cpp -$(libcppdir)/vf_settokenvalue.o: lib/vf_settokenvalue.cpp lib/astutils.h lib/calculate.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h lib/vf_common.h lib/vf_settokenvalue.h lib/vfvalue.h +$(libcppdir)/vf_settokenvalue.o: lib/vf_settokenvalue.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/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vf_common.h lib/vf_settokenvalue.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_settokenvalue.cpp $(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathlib.h lib/smallvector.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h @@ -957,7 +957,7 @@ test/testutils.o: test/testutils.cpp lib/check.h lib/checkers.h lib/color.h lib/ test/testvaarg.o: test/testvaarg.cpp lib/check.h lib/checkers.h lib/checkimpl.h lib/checkvaarg.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h $(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testvaarg.cpp -test/testvalueflow.o: test/testvalueflow.cpp lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h +test/testvalueflow.o: test/testvalueflow.cpp lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/infer.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueptr.h lib/vf_common.h lib/vfvalue.h test/fixture.h test/helpers.h $(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testvalueflow.cpp test/testvarid.o: test/testvarid.cpp lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index bdd18ee21d2..762bcf8585e 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -355,10 +355,69 @@ void CheckBufferOverrunImpl::arrayIndex() arrayIndexError(tok, dimensions, indexValues); } + const auto isNonnegativeInductionVariable = [&](const Token* indexToken) { + const Variable* const variable = indexToken->variable(); + if (!variable || !indexToken->varId()) + return false; + + for (const Scope* loop = indexToken->scope(); loop; loop = loop->nestedIn) { + if (loop->type != ScopeType::eFor || !loop->classDef || !loop->bodyStart || !loop->bodyEnd) + continue; + const Token* const forExpressions = loop->classDef->next(); + if (!forExpressions || !forExpressions->astOperand2()) + continue; + const Token* const clauses = forExpressions->astOperand2(); + const Token* const init = clauses->astOperand1(); + const Token* const remainingClauses = clauses->astOperand2(); + if (!init || !remainingClauses || !remainingClauses->astOperand1() || !remainingClauses->astOperand2()) + continue; + const Token* const step = remainingClauses->astOperand2(); + if (!Token::simpleMatch(init, "=") || !init->astOperand1() || + init->astOperand1()->varId() != indexToken->varId() || !Token::simpleMatch(step, "++")) + continue; + const Token* stepVariable = step->astOperand1(); + if (!stepVariable) + stepVariable = step->astOperand2(); + if (!stepVariable || stepVariable->varId() != indexToken->varId()) + continue; + + const Token* const initialValue = init->astOperand2(); + const ValueType* const initialType = initialValue ? initialValue->valueType() : nullptr; + const ValueType* const indexType = indexToken->valueType(); + if (!initialValue || !initialType || !indexType || !initialType->isIntegral() || !indexType->isIntegral() || + initialType->pointer || indexType->pointer || initialType->sign != indexType->sign || + initialType->getSizeOf(mSettings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer) > + indexType->getSizeOf(mSettings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer)) + continue; + const bool hasNonnegativeLowerBound = std::any_of(initialValue->values().cbegin(), + initialValue->values().cend(), + [](const ValueFlow::Value& value) { + return value.isIntValue() && !value.isImpossible() && !value.isInconclusive() && value.intvalue >= 0 && + (value.bound == ValueFlow::Value::Bound::Lower || + (value.bound == ValueFlow::Value::Bound::Point && value.isKnown())); + }); + if (!hasNonnegativeLowerBound) + continue; + if (findVariableChanged(loop->bodyStart->next(), + loop->bodyEnd, + /*indirect*/ 1, + variable->declarationId(), + variable->isGlobal(), + mSettings)) + continue; + return true; + } + return false; + }; + // Negative index bool neg = false; std::vector negativeIndexes; for (const Token * indexToken : indexTokens) { + if (isNonnegativeInductionVariable(indexToken)) { + negativeIndexes.emplace_back(ValueFlow::Value::unknown()); + continue; + } const ValueFlow::Value *negativeValue = indexToken->getValueLE(-1, mSettings); if (negativeValue) { negativeIndexes.emplace_back(*negativeValue); diff --git a/lib/checktype.cpp b/lib/checktype.cpp index 447a8924360..a89c577452a 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -98,12 +98,49 @@ void CheckTypeImpl::checkTooBigBitwiseShift() else continue; - // Get biggest rhs value. preferably a value which doesn't have 'condition'. - const ValueFlow::Value * value = tok->astOperand2()->getValueGE(lhsbits, mSettings); + // Get the biggest feasible rhs value, preferring a value without a condition. + const Token* const shiftCount = tok->astOperand2(); + const auto isUnreachableTernaryValue = [&](const ValueFlow::Value& candidate) { + const Token* child = shiftCount; + for (const Token* parent = child->astParent(); parent; child = parent, parent = parent->astParent()) { + if (parent->str() != ":" || !Token::simpleMatch(parent->astParent(), "?")) + continue; + const bool inTrueBranch = child == parent->astOperand1(); + const bool inFalseBranch = child == parent->astOperand2(); + if (!inTrueBranch && !inFalseBranch) + continue; + const Token* condition = parent->astParent()->astOperand1(); + bool canEnterBranch = false; + bool canEnterOtherBranch = false; + for (const ValueFlow::Value& value : condition->values()) { + if (!value.isIntValue() || value.isImpossible() || value.bound != ValueFlow::Value::Bound::Point || + (value.path != 0 && (candidate.path == 0 || value.path != candidate.path))) + continue; + if ((value.intvalue != 0) == inTrueBranch) + canEnterBranch = true; + else + canEnterOtherBranch = true; + } + return canEnterOtherBranch && !canEnterBranch; + } + return false; + }; + const auto getShiftCountValueGE = [&](MathLib::bigint threshold) -> const ValueFlow::Value* { + return ValueFlow::findValue(shiftCount->values(), mSettings, [&](const ValueFlow::Value& candidate) { + if (candidate.isImpossible() || !candidate.isIntValue() || candidate.intvalue < threshold || + isUnreachableTernaryValue(candidate)) + return false; + return std::none_of(shiftCount->values().cbegin(), shiftCount->values().cend(), [&](const ValueFlow::Value& bound) { + return bound.isImpossible() && bound.isIntValue() && bound.bound == ValueFlow::Value::Bound::Lower && + bound.intvalue <= threshold && (bound.path == 0 || bound.path == candidate.path); + }); + }); + }; + const ValueFlow::Value * value = getShiftCountValueGE(lhsbits); if (value && mSettings.isEnabled(value, false)) tooBigBitwiseShiftError(tok, lhsbits, *value); else if (lhstype->sign == ValueType::Sign::SIGNED) { - value = tok->astOperand2()->getValueGE(lhsbits-1, mSettings); + value = getShiftCountValueGE(lhsbits - 1); if (value && mSettings.isEnabled(value, false)) tooBigSignedBitwiseShiftError(tok, lhsbits, *value); } diff --git a/lib/infer.cpp b/lib/infer.cpp index cad0761f68b..d5e3cb1cf31 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -237,12 +237,32 @@ namespace { }; } -static void addToErrorPath(ValueFlow::Value& value, const std::vector& refs) +static bool addToErrorPath(ValueFlow::Value& value, const std::vector& refs) { std::unordered_set locations; + const Token* condition = nullptr; + MathLib::bigint path = 0; + nonneg int varId = 0; + MathLib::bigint varvalue = 0; + bool conflictingVariable = false; for (const ValueFlow::Value* ref : refs) { - if (ref->condition && !value.condition) - value.condition = ref->condition; + if (ref->path < 0 || (ref->path > 0 && path > 0 && ref->path != path)) + return false; + if (ref->path > 0) + path = ref->path; + if (ref->condition && !condition) + condition = ref->condition; + if (ref->varId) { + if (varId && (varId != ref->varId || varvalue != ref->varvalue)) + conflictingVariable = true; + else { + varId = ref->varId; + varvalue = ref->varvalue; + } + } + value.conditional = value.conditional || ref->conditional; + value.defaultArg = value.defaultArg || ref->defaultArg; + value.safe = value.safe || ref->safe; std::copy_if(ref->errorPath.cbegin(), ref->errorPath.cend(), std::back_inserter(value.errorPath), @@ -256,6 +276,16 @@ static void addToErrorPath(ValueFlow::Value& value, const std::vector& refs) @@ -288,75 +318,127 @@ std::vector infer(const ValuePtr& model, std::list lhsValues, std::list rhsValues) { - std::vector result; auto notMatch = [&](const ValueFlow::Value& value) { return !model->match(value); }; lhsValues.remove_if(notMatch); if (lhsValues.empty()) - return result; + return {}; rhsValues.remove_if(notMatch); if (rhsValues.empty()) - return result; - - Interval lhs = Interval::fromValues(lhsValues); - Interval rhs = Interval::fromValues(rhsValues); - - if (op == "-") { - Interval diff = lhs - rhs; - if (diff.isScalar()) { - std::vector refs = diff.getScalarRef(); - ValueFlow::Value value(diff.getScalar()); - addToErrorPath(value, refs); - setValueKind(value, refs); - result.push_back(std::move(value)); - } else { - if (!diff.minvalue.empty()) { - ValueFlow::Value value(diff.minvalue.front() - 1); - value.setImpossible(); - value.bound = ValueFlow::Value::Bound::Upper; - addToErrorPath(value, diff.minRef); - result.push_back(std::move(value)); + return {}; + + lhsValues.remove_if([](const ValueFlow::Value& value) { + return value.path < 0; + }); + rhsValues.remove_if([](const ValueFlow::Value& value) { + return value.path < 0; + }); + if (lhsValues.empty() || rhsValues.empty()) + return {}; + + const auto inferIntervals = [&](const std::list& lhsValues2, + const std::list& rhsValues2) { + std::vector inferred; + Interval lhs = Interval::fromValues(lhsValues2); + Interval rhs = Interval::fromValues(rhsValues2); + + if (op == "-") { + Interval diff = lhs - rhs; + if (diff.isScalar()) { + std::vector refs = diff.getScalarRef(); + ValueFlow::Value value(diff.getScalar()); + if (addToErrorPath(value, refs)) { + setValueKind(value, refs); + inferred.push_back(std::move(value)); + } + } else { + if (!diff.minvalue.empty()) { + ValueFlow::Value value(diff.minvalue.front() - 1); + value.setImpossible(); + value.bound = ValueFlow::Value::Bound::Upper; + if (addToErrorPath(value, diff.minRef)) + inferred.push_back(std::move(value)); + } + if (!diff.maxvalue.empty()) { + ValueFlow::Value value(diff.maxvalue.front() + 1); + value.setImpossible(); + value.bound = ValueFlow::Value::Bound::Lower; + if (addToErrorPath(value, diff.maxRef)) + inferred.push_back(std::move(value)); + } } - if (!diff.maxvalue.empty()) { - ValueFlow::Value value(diff.maxvalue.front() + 1); - value.setImpossible(); - value.bound = ValueFlow::Value::Bound::Lower; - addToErrorPath(value, diff.maxRef); - result.push_back(std::move(value)); + } else if ((op == "!=" || op == "==") && lhs.isScalarOrEmpty() && rhs.isScalarOrEmpty()) { + if (lhs.isScalar() && rhs.isScalar()) { + std::vector refs = Interval::merge(lhs.getScalarRef(), rhs.getScalarRef()); + ValueFlow::Value value(calculate(op, lhs.getScalar(), rhs.getScalar())); + if (addToErrorPath(value, refs)) { + setValueKind(value, refs); + inferred.push_back(std::move(value)); + } + } else { + std::vector refs; + if (lhs.isScalar() && inferNotEqual(rhsValues2, lhs.getScalar())) + refs = lhs.getScalarRef(); + else if (rhs.isScalar() && inferNotEqual(lhsValues2, rhs.getScalar())) + refs = rhs.getScalarRef(); + if (!refs.empty()) { + ValueFlow::Value value(op == "!="); + if (addToErrorPath(value, refs)) { + setValueKind(value, refs); + inferred.push_back(std::move(value)); + } + } } - } - } else if ((op == "!=" || op == "==") && lhs.isScalarOrEmpty() && rhs.isScalarOrEmpty()) { - if (lhs.isScalar() && rhs.isScalar()) { - std::vector refs = Interval::merge(lhs.getScalarRef(), rhs.getScalarRef()); - ValueFlow::Value value(calculate(op, lhs.getScalar(), rhs.getScalar())); - addToErrorPath(value, refs); - setValueKind(value, refs); - result.push_back(std::move(value)); } else { std::vector refs; - if (lhs.isScalar() && inferNotEqual(rhsValues, lhs.getScalar())) - refs = lhs.getScalarRef(); - else if (rhs.isScalar() && inferNotEqual(lhsValues, rhs.getScalar())) - refs = rhs.getScalarRef(); - if (!refs.empty()) { - ValueFlow::Value value(op == "!="); - addToErrorPath(value, refs); - setValueKind(value, refs); - result.push_back(std::move(value)); + std::vector r = Interval::compare(op, lhs, rhs, &refs); + if (!r.empty()) { + ValueFlow::Value value(static_cast(r.front())); + if (addToErrorPath(value, refs)) { + setValueKind(value, refs); + inferred.push_back(std::move(value)); + } } } - } else { - std::vector refs; - std::vector r = Interval::compare(op, lhs, rhs, &refs); - if (!r.empty()) { - ValueFlow::Value value(static_cast(r.front())); - addToErrorPath(value, refs); - setValueKind(value, refs); - result.push_back(std::move(value)); - } + return inferred; + }; + + std::vector paths; + const auto addPath = [&](MathLib::bigint path) { + if (std::find(paths.cbegin(), paths.cend(), path) == paths.cend()) + paths.push_back(path); + }; + const auto hasCompatibleValue = [](const std::list& values, MathLib::bigint path) { + return std::any_of(values.cbegin(), values.cend(), [path](const ValueFlow::Value& value) { + return value.path == 0 || value.path == path; + }); + }; + if (hasCompatibleValue(lhsValues, 0) && hasCompatibleValue(rhsValues, 0)) + addPath(0); + for (const ValueFlow::Value& value : lhsValues) { + if (value.path > 0 && hasCompatibleValue(rhsValues, value.path)) + addPath(value.path); + } + for (const ValueFlow::Value& value : rhsValues) { + if (value.path > 0 && hasCompatibleValue(lhsValues, value.path)) + addPath(value.path); } + std::vector result; + if (paths.size() == 1 && paths.front() == 0) + return inferIntervals(lhsValues, rhsValues); + for (MathLib::bigint path : paths) { + std::list lhsCompatible; + std::list rhsCompatible; + const auto compatible = [path](const ValueFlow::Value& value) { + return path == 0 ? value.path == 0 : (value.path == 0 || value.path == path); + }; + std::copy_if(lhsValues.cbegin(), lhsValues.cend(), std::back_inserter(lhsCompatible), compatible); + std::copy_if(rhsValues.cbegin(), rhsValues.cend(), std::back_inserter(rhsCompatible), compatible); + std::vector inferred = inferIntervals(lhsCompatible, rhsCompatible); + std::move(inferred.begin(), inferred.end(), std::back_inserter(result)); + } return result; } diff --git a/lib/infer.h b/lib/infer.h index 8d66d7da016..ecd93baca74 100644 --- a/lib/infer.h +++ b/lib/infer.h @@ -43,10 +43,10 @@ struct InferModel { InferModel() = default; }; -std::vector infer(const ValuePtr& model, - const std::string& op, - std::list lhsValues, - std::list rhsValues); +CPPCHECKLIB std::vector infer(const ValuePtr& model, + const std::string& op, + std::list lhsValues, + std::list rhsValues); std::vector infer(const ValuePtr& model, const std::string& op, @@ -61,7 +61,7 @@ std::vector infer(const ValuePtr& model, CPPCHECKLIB std::vector getMinValue(const ValuePtr& model, const std::list& values); std::vector getMaxValue(const ValuePtr& model, const std::list& values); -ValuePtr makeIntegralInferModel(); +CPPCHECKLIB ValuePtr makeIntegralInferModel(); ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, MathLib::bigint val); diff --git a/lib/token.cpp b/lib/token.cpp index eac69decee7..7a598accbe5 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -28,6 +28,7 @@ #include "utils.h" #include "tokenrange.h" #include "valueflow.h" +#include "vf_common.h" #include #include @@ -1928,8 +1929,11 @@ const ValueFlow::Value * Token::getValueLE(const MathLib::bigint val, const Sett { if (!mImpl->mValues) return nullptr; + const bool unsignedMayWrap = ValueFlow::isUnsignedArithmeticResult(this) && + !ValueFlow::isUnsignedArithmeticNoWrap(this, settings); return ValueFlow::findValue(*mImpl->mValues, settings, [&](const ValueFlow::Value& v) { - return !v.isImpossible() && v.isIntValue() && v.intvalue <= val; + return !v.isImpossible() && v.isIntValue() && v.intvalue <= val && + (!unsignedMayWrap || v.bound != ValueFlow::Value::Bound::Lower); }); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c021d1d296b..4bf604dd8b9 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -420,10 +420,12 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val if (value1.bound == ValueFlow::Value::Bound::Lower || value2.bound == ValueFlow::Value::Bound::Lower) result.bound = ValueFlow::Value::Bound::Lower; } - if (value1.path != value2.path) - result.path = -1; - else + if (value1.path == 0) + result.path = value2.path; + else if (value2.path == 0 || value1.path == value2.path) result.path = value1.path; + else + result.path = -1; } static void valueFlowNumber(TokenList &tokenlist, const Settings& settings) @@ -1025,7 +1027,7 @@ static void valueFlowRightShift(TokenList& tokenList, const Settings& settings) } } -static std::vector minUnsignedValue(const Token* tok, int depth = 8) +static std::vector minUnsignedValue(const Token* tok, const Settings& settings, int depth = 8) { std::vector result; if (!tok) @@ -1034,10 +1036,11 @@ static std::vector minUnsignedValue(const Token* tok, int depth return result; if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::INT)) { result = {v->intvalue}; - } else if (!Token::Match(tok, "-|%|&|^") && tok->isConstOp() && tok->astOperand1() && tok->astOperand2()) { - std::vector op1 = minUnsignedValue(tok->astOperand1(), depth - 1); + } else if (!Token::Match(tok, "-|%|&|^") && tok->isConstOp() && tok->astOperand1() && tok->astOperand2() && + (!ValueFlow::isUnsignedArithmeticResult(tok) || ValueFlow::isUnsignedArithmeticNoWrap(tok, settings))) { + std::vector op1 = minUnsignedValue(tok->astOperand1(), settings, depth - 1); if (!op1.empty()) { - std::vector op2 = minUnsignedValue(tok->astOperand2(), depth - 1); + std::vector op2 = minUnsignedValue(tok->astOperand2(), settings, depth - 1); if (!op2.empty()) { result = calculate>(tok->str(), op1.front(), op2.front()); } @@ -1087,8 +1090,26 @@ static void valueFlowImpossibleValues(TokenList& tokenList, const Settings& sett upper.bound = ValueFlow::Value::Bound::Lower; upper.setImpossible(); setTokenValue(tok, std::move(upper), settings); + } else if (tok->isCast() && tok->valueType() && tok->valueType()->isIntegral() && !tok->valueType()->pointer) { + MathLib::bigint minValue; + MathLib::bigint maxValue; + if (!ValueFlow::getMinMaxValues(tok->valueType(), settings.platform, minValue, maxValue)) + continue; + + if (minValue > std::numeric_limits::min()) { + ValueFlow::Value lower{minValue - 1}; + lower.bound = ValueFlow::Value::Bound::Upper; + lower.setImpossible(); + setTokenValue(tok, std::move(lower), settings); + } + if (maxValue < std::numeric_limits::max()) { + ValueFlow::Value upper{maxValue + 1}; + upper.bound = ValueFlow::Value::Bound::Lower; + upper.setImpossible(); + setTokenValue(tok, std::move(upper), settings); + } } else if (astIsUnsigned(tok) && !astIsPointer(tok)) { - std::vector minvalue = minUnsignedValue(tok); + std::vector minvalue = minUnsignedValue(tok, settings); if (minvalue.empty()) continue; ValueFlow::Value value{std::max(0, minvalue.front()) - 1}; @@ -5074,6 +5095,34 @@ struct SimpleConditionHandler : ConditionHandler { if (!conds.empty()) return conds; + if (tok->isComparisonOp() && tok->astOperand1() && tok->astOperand2() && + (tok->str() == ">=" || tok->str() == "<=")) { + for (bool lhs : {true, false}) { + const Token* vartok = lhs ? tok->astOperand1() : tok->astOperand2(); + const Token* valuetok = lhs ? tok->astOperand2() : tok->astOperand1(); + if (!astIsFloat(vartok, false) || !valuetok->isLiteral()) + continue; + const ValueFlow::Value* value = valuetok->getKnownValue(ValueFlow::Value::ValueType::FLOAT); + if (!value || !value->isKnown()) + continue; + + const bool lower = lhs == (tok->str() == ">="); + auto makeValue = [&](ValueFlow::Value::Bound bound) { + ValueFlow::Value result = *value; + result.bound = bound; + result.assumeCondition(tok); + result.setPossible(); + return result; + }; + Condition cond; + cond.vartok = vartok; + cond.impossible = false; + cond.true_values.push_back(makeValue(lower ? ValueFlow::Value::Bound::Lower : ValueFlow::Value::Bound::Upper)); + cond.false_values.push_back(makeValue(lower ? ValueFlow::Value::Bound::Upper : ValueFlow::Value::Bound::Lower)); + return {std::move(cond)}; + } + } + const Token* vartok = getConditionVariable(tok); if (!vartok) return {}; @@ -5158,8 +5207,24 @@ static void valueFlowInferCondition(TokenList& tokenlist, const Settings& settin } } } else if (isIntegralOrPointer(tok->astOperand1()) && isIntegralOrPointer(tok->astOperand2())) { - std::vector result = - infer(makeIntegralInferModel(), tok->str(), tok->astOperand1()->values(), tok->astOperand2()->values()); + const bool isUnsignedArith = ValueFlow::isUnsignedArithmeticResult(tok); + const bool mayWrap = isUnsignedArith && !ValueFlow::isUnsignedArithmeticNoWrap(tok, settings); + std::vector result; + const auto isImpossibleIntegralBound = [](const ValueFlow::Value& v) { + return v.isIntValue() && v.isImpossible() && v.bound != ValueFlow::Value::Bound::Point; + }; + if (mayWrap) { + std::list lhsValues = tok->astOperand1()->values(); + std::list rhsValues = tok->astOperand2()->values(); + lhsValues.remove_if(isImpossibleIntegralBound); + rhsValues.remove_if(isImpossibleIntegralBound); + result = infer(makeIntegralInferModel(), tok->str(), std::move(lhsValues), std::move(rhsValues)); + } else { + result = infer(makeIntegralInferModel(), + tok->str(), + tok->astOperand1()->values(), + tok->astOperand2()->values()); + } for (ValueFlow::Value& value : result) { setTokenValue(tok, std::move(value), settings); } diff --git a/lib/vf_common.cpp b/lib/vf_common.cpp index f9eecc20fb9..0c74109c4e0 100644 --- a/lib/vf_common.cpp +++ b/lib/vf_common.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -46,7 +47,7 @@ namespace ValueFlow if (!vt || !vt->isIntegral() || vt->pointer) return false; - std::uint8_t bits; + std::size_t bits; switch (vt->type) { case ValueType::Type::BOOL: bits = 1; @@ -66,10 +67,16 @@ namespace ValueFlow case ValueType::Type::LONGLONG: bits = platform.long_long_bit; break; + case ValueType::Type::WCHAR_T: + bits = platform.sizeof_wchar_t * platform.char_bit; + break; default: return false; } + if (bits == 0) { + return false; + } if (bits == 1) { minValue = 0; maxValue = 1; @@ -77,18 +84,20 @@ namespace ValueFlow if (vt->sign == ValueType::Sign::UNSIGNED) { minValue = 0; maxValue = (1LL << bits) - 1; - } else { + } else if (vt->sign == ValueType::Sign::SIGNED) { minValue = -(1LL << (bits - 1)); maxValue = (1LL << (bits - 1)) - 1; - } + } else + return false; } else if (bits == 64) { if (vt->sign == ValueType::Sign::UNSIGNED) { minValue = 0; - maxValue = LLONG_MAX; // todo max unsigned value - } else { + maxValue = LLONG_MAX; // MathLib::bigint cannot represent ULLONG_MAX; conservative max for unsigned 64-bit + } else if (vt->sign == ValueType::Sign::SIGNED) { minValue = LLONG_MIN; maxValue = LLONG_MAX; - } + } else + return false; } else { return false; } @@ -96,6 +105,212 @@ namespace ValueFlow return true; } + bool isUnsignedArithmeticResult(const Token* tok) + { + if (!Token::Match(tok, "+|-|*")) + return false; + const ValueType* vt = tok->valueType(); + return vt && vt->isIntegral() && vt->pointer == 0 && vt->sign == ValueType::Sign::UNSIGNED && + (vt->type == ValueType::Type::INT || + vt->type == ValueType::Type::LONG || + vt->type == ValueType::Type::LONGLONG || + vt->type == ValueType::Type::UNKNOWN_INT); + } + + namespace { + struct UnsignedArithmeticRange { + MathLib::bigint lower{}; + MathLib::bigint upper{}; + bool hasUpper{}; + const Token* condition{}; + }; + + bool getUnsignedArithmeticRange(const Token* tok, + MathLib::bigint path, + const ValueType* resultType, + MathLib::biguint resultMax, + const Settings& settings, + UnsignedArithmeticRange& range) + { + const ValueType* const valueType = tok ? tok->valueType() : nullptr; + if (!valueType || !valueType->isIntegral() || valueType->pointer || + valueType->sign == ValueType::UNKNOWN_SIGN) + return false; + + const std::size_t valueSize = valueType->getSizeOf(settings, + ValueType::Accuracy::ExactOrZero, + ValueType::SizeOf::Pointer); + const std::size_t valueBits = valueSize * settings.platform.char_bit; + const std::size_t resultSize = resultType->getSizeOf(settings, + ValueType::Accuracy::ExactOrZero, + ValueType::SizeOf::Pointer); + const std::size_t resultBits = resultSize * settings.platform.char_bit; + if (valueBits == 0 || valueBits > 64 || resultBits == 0 || resultBits > 64) + return false; + + MathLib::bigint minValue; + MathLib::bigint maxValue; + if (!getMinMaxValues(valueType, settings.platform, minValue, maxValue)) + return false; + if (valueType->sign == ValueType::Sign::UNSIGNED) { + const auto isSafeSignedConversion = [&](const Token* operand) { + if (!operand) + return true; + const ValueType* const operandType = operand->valueType(); + if (!operandType) + return false; + if (operandType->isIntegral() && operandType->pointer == 0 && + operandType->sign == ValueType::Sign::UNSIGNED) + return true; + if (!operandType->isIntegral() || operandType->pointer != 0) + return false; + UnsignedArithmeticRange operandRange; + return getUnsignedArithmeticRange(operand, path, resultType, resultMax, settings, operandRange) && + operandRange.lower >= 0; + }; + if (!isSafeSignedConversion(tok->astOperand1()) || !isSafeSignedConversion(tok->astOperand2())) + return false; + } + range.lower = minValue; + range.hasUpper = !(valueType->sign == ValueType::Sign::UNSIGNED && valueBits == 64); + if (range.hasUpper) + range.upper = maxValue; + + if (!tok->values().empty()) { + for (const Value& value : tok->values()) { + if (value.path < 0) + return false; + if ((path == 0 && value.path != 0) || + (path > 0 && value.path != 0 && value.path != path)) + continue; + if (!value.isIntValue() || value.isInconclusive()) + continue; + if (valueType->sign == ValueType::Sign::UNSIGNED && !value.isImpossible() && value.intvalue < 0) + return false; + if (value.condition) { + if (range.condition && range.condition != value.condition) + return false; + range.condition = value.condition; + } + + if (value.isImpossible()) { + if (value.bound == Value::Bound::Upper) { + if (value.intvalue == std::numeric_limits::max()) + return false; + range.lower = std::max(range.lower, value.intvalue + 1); + } else if (value.bound == Value::Bound::Lower) { + if (value.intvalue == std::numeric_limits::min()) + return false; + const MathLib::bigint upper = value.intvalue - 1; + if (!range.hasUpper || upper < range.upper) + range.upper = upper; + range.hasUpper = true; + } + } else if (value.bound == Value::Bound::Lower) { + range.lower = std::max(range.lower, value.intvalue); + } else if (value.bound == Value::Bound::Upper) { + if (!range.hasUpper || value.intvalue < range.upper) + range.upper = value.intvalue; + range.hasUpper = true; + } else if (value.isKnown()) { + range.lower = std::max(range.lower, value.intvalue); + if (!range.hasUpper || value.intvalue < range.upper) + range.upper = value.intvalue; + range.hasUpper = true; + } + } + } + + if (range.lower < 0 || (range.hasUpper && range.upper < range.lower)) + return false; + if (range.hasUpper && static_cast(range.upper) > resultMax) + return false; + if (valueBits > resultBits && + (!range.hasUpper || static_cast(range.upper) > resultMax)) + return false; + return true; + } + } + + bool isUnsignedArithmeticNoWrap(const Token* tok, const Settings& settings) + { + if (!isUnsignedArithmeticResult(tok) || !tok->astOperand1() || !tok->astOperand2()) + return false; + + const ValueType* const resultType = tok->valueType(); + const std::size_t resultSize = resultType->getSizeOf(settings, + ValueType::Accuracy::ExactOrZero, + ValueType::SizeOf::Pointer); + const std::size_t resultBits = resultSize * settings.platform.char_bit; + if (resultBits == 0 || resultBits > 64) + return false; + const MathLib::biguint resultMax = resultBits == 64 ? std::numeric_limits::max() : + (MathLib::biguint{1} << resultBits) - 1; + + std::vector paths; + const auto hasPath = [](const Token* operand, MathLib::bigint path) { + if (operand->values().empty()) + return true; + return std::any_of(operand->values().cbegin(), operand->values().cend(), [path](const Value& value) { + return value.path == 0 || value.path == path; + }); + }; + const auto addPath = [&](MathLib::bigint path) { + if (std::find(paths.cbegin(), paths.cend(), path) == paths.cend()) + paths.push_back(path); + }; + const auto hasUnconditionalPath = [](const Token* operand) { + return operand->values().empty() || + std::any_of(operand->values().cbegin(), operand->values().cend(), [](const Value& value) { + return value.path == 0; + }); + }; + const Token* const lhs = tok->astOperand1(); + const Token* const rhs = tok->astOperand2(); + if (hasUnconditionalPath(lhs) && hasUnconditionalPath(rhs)) + addPath(0); + for (const Value& value : lhs->values()) { + if (value.path > 0 && hasPath(rhs, value.path)) + addPath(value.path); + if (value.path < 0) + return false; + } + for (const Value& value : rhs->values()) { + if (value.path > 0 && hasPath(lhs, value.path)) + addPath(value.path); + if (value.path < 0) + return false; + } + if (paths.empty()) + return false; + + return std::all_of(paths.cbegin(), paths.cend(), [&](MathLib::bigint path) { + UnsignedArithmeticRange lhsRange; + UnsignedArithmeticRange rhsRange; + if (!getUnsignedArithmeticRange(lhs, path, resultType, resultMax, settings, lhsRange) || + !getUnsignedArithmeticRange(rhs, path, resultType, resultMax, settings, rhsRange) || + (lhsRange.condition && rhsRange.condition && lhsRange.condition != rhsRange.condition)) + return false; + + if (tok->str() == "-") { + if (!rhsRange.hasUpper || static_cast(lhsRange.lower) < + static_cast(rhsRange.upper)) + return false; + } else if (tok->str() == "+") { + if (!lhsRange.hasUpper || !rhsRange.hasUpper || + static_cast(lhsRange.upper) > + resultMax - static_cast(rhsRange.upper)) + return false; + } else if (tok->str() == "*") { + if (!lhsRange.hasUpper || !rhsRange.hasUpper || + (lhsRange.upper != 0 && static_cast(rhsRange.upper) > + resultMax / static_cast(lhsRange.upper))) + return false; + } + return true; + }); + } + MathLib::bigint truncateIntValue(MathLib::bigint value, size_t value_size, const ValueType::Sign dst_sign) { if (value_size == 0) diff --git a/lib/vf_common.h b/lib/vf_common.h index aca438a0dff..a3ff811450e 100644 --- a/lib/vf_common.h +++ b/lib/vf_common.h @@ -39,13 +39,16 @@ namespace ValueFlow namespace ValueFlow { - bool getMinMaxValues(const ValueType* vt, const Platform& platform, MathLib::bigint& minValue, MathLib::bigint& maxValue); + CPPCHECKLIB bool getMinMaxValues(const ValueType* vt, const Platform& platform, MathLib::bigint& minValue, MathLib::bigint& maxValue); + + bool isUnsignedArithmeticResult(const Token* tok); + bool isUnsignedArithmeticNoWrap(const Token* tok, const Settings& settings); MathLib::bigint truncateIntValue(MathLib::bigint value, size_t value_size, ValueType::Sign dst_sign); Token * valueFlowSetConstantValue(Token *tok, const Settings &settings); - Value castValue(Value value, ValueType::Sign sign, nonneg int bit); + CPPCHECKLIB Value castValue(Value value, ValueType::Sign sign, nonneg int bit); std::string debugString(const Value& v); diff --git a/lib/vf_settokenvalue.cpp b/lib/vf_settokenvalue.cpp index 1d6179fb9af..90e6d885940 100644 --- a/lib/vf_settokenvalue.cpp +++ b/lib/vf_settokenvalue.cpp @@ -21,6 +21,7 @@ #include "astutils.h" #include "calculate.h" #include "config.h" +#include "infer.h" #include "library.h" #include "mathlib.h" #include "platform.h" @@ -29,6 +30,7 @@ #include "token.h" #include "utils.h" #include "valueflow.h" +#include "valueptr.h" #include "vfvalue.h" #include "vf_common.h" @@ -488,6 +490,9 @@ namespace ValueFlow return; } + const bool skipUnsignedRangeBounds = isUnsignedArithmeticResult(parent) && + !isUnsignedArithmeticNoWrap(parent, settings); + for (const Value &value1 : parent->astOperand1()->values()) { if (!isComputableValue(parent, value1)) continue; @@ -500,6 +505,11 @@ namespace ValueFlow continue; if (!isCompatibleValues(value1, value2)) continue; + const auto hasRangeBound = [](const Value& value) { + return (value.isIntValue() || value.isSymbolicValue()) && value.bound != Value::Bound::Point; + }; + if (skipUnsignedRangeBounds && (hasRangeBound(value1) || hasRangeBound(value2))) + continue; Value result(0); combineValueProperties(value1, value2, result); if (astIsFloat(parent, false)) { @@ -588,6 +598,19 @@ namespace ValueFlow } if (error) continue; + if (isUnsignedArithmeticResult(parent) && value1.bound == Value::Bound::Point && + value2.bound == Value::Bound::Point && result.isIntValue()) { + const ValueType* const resultType = parent->valueType(); + const size_t resultSize = resultType->getSizeOf(settings, + ValueType::Accuracy::ExactOrZero, + ValueType::SizeOf::Pointer); + if (resultSize > 0 && resultSize < sizeof(MathLib::bigint)) { + result.wideintvalue = result.intvalue; + result.intvalue = ValueFlow::truncateIntValue(result.intvalue, + resultSize, + ValueType::Sign::UNSIGNED); + } + } // If the bound comes from the second value then invert the bound when subtracting if (Token::simpleMatch(parent, "-") && value2.bound == result.bound && value2.bound != Value::Bound::Point) @@ -639,6 +662,32 @@ namespace ValueFlow // unary minus else if (parent->isUnaryOp("-") && !astIsUnsigned(parent)) { for (const Value &val : tok->values()) { + if (val.isSymbolicValue() && val.isImpossible() && val.tokvalue && + val.bound != Value::Bound::Point) { + std::list symbolicValues; + std::copy_if(val.tokvalue->values().cbegin(), + val.tokvalue->values().cend(), + std::back_inserter(symbolicValues), + [&](const Value& symbolicValue) { + return symbolicValue.path == 0 || symbolicValue.path == val.path; + }); + const std::vector range = val.bound == Value::Bound::Upper + ? ::getMinValue(::makeIntegralInferModel(), symbolicValues) + : ::getMaxValue(::makeIntegralInferModel(), symbolicValues); + if (range.empty() || range.front() == std::numeric_limits::min()) + continue; + const MathLib::bigint negated = -range.front(); + if ((val.intvalue > 0 && negated < std::numeric_limits::min() + val.intvalue) || + (val.intvalue < 0 && negated > std::numeric_limits::max() + val.intvalue)) + continue; + Value v(val); + v.valueType = Value::ValueType::INT; + v.tokvalue = nullptr; + v.intvalue = negated - val.intvalue; + v.invertBound(); + setTokenValue(parent, std::move(v), settings); + continue; + } if (!val.isIntValue() && !val.isFloatValue()) continue; Value v(val); diff --git a/oss-fuzz/Makefile b/oss-fuzz/Makefile index 7413910834f..a00df42ac79 100644 --- a/oss-fuzz/Makefile +++ b/oss-fuzz/Makefile @@ -345,7 +345,7 @@ $(libcppdir)/templatesimplifier.o: ../lib/templatesimplifier.cpp ../lib/checkers $(libcppdir)/timer.o: ../lib/timer.cpp ../lib/config.h ../lib/timer.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/timer.cpp -$(libcppdir)/token.o: ../lib/token.cpp ../externals/simplecpp/simplecpp.h ../lib/astutils.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.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/tokenrange.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h +$(libcppdir)/token.o: ../lib/token.cpp ../externals/simplecpp/simplecpp.h ../lib/astutils.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.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/tokenrange.h ../lib/utils.h ../lib/valueflow.h ../lib/vf_common.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/token.cpp $(libcppdir)/tokenlist.o: ../lib/tokenlist.cpp ../externals/simplecpp/simplecpp.h ../lib/astutils.h ../lib/checkers.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/keywords.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/standards.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h @@ -360,7 +360,7 @@ $(libcppdir)/vf_analyzers.o: ../lib/vf_analyzers.cpp ../lib/analyzer.h ../lib/as $(libcppdir)/vf_common.o: ../lib/vf_common.cpp ../lib/astutils.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/vf_common.h ../lib/vf_settokenvalue.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_common.cpp -$(libcppdir)/vf_settokenvalue.o: ../lib/vf_settokenvalue.cpp ../lib/astutils.h ../lib/calculate.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/valueflow.h ../lib/vf_common.h ../lib/vf_settokenvalue.h ../lib/vfvalue.h +$(libcppdir)/vf_settokenvalue.o: ../lib/vf_settokenvalue.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/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/valueflow.h ../lib/valueptr.h ../lib/vf_common.h ../lib/vf_settokenvalue.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_settokenvalue.cpp $(libcppdir)/vfvalue.o: ../lib/vfvalue.cpp ../lib/config.h ../lib/errortypes.h ../lib/mathlib.h ../lib/smallvector.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/vfvalue.h diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index f6a2e1a5ec1..e4e718fc1ad 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -164,6 +164,7 @@ class TestBufferOverrun : public TestFixture { TEST_CASE(array_index_74); // #11088 TEST_CASE(array_index_75); TEST_CASE(array_index_76); + TEST_CASE(array_index_77); TEST_CASE(array_index_multidim); TEST_CASE(array_index_switch_in_for); TEST_CASE(array_index_for_in_for); // FP: #2634 @@ -2012,6 +2013,42 @@ class TestBufferOverrun : public TestFixture { errout_str()); } + void array_index_77() + { + // The loop index x is at least colsToTranslate, and the cast range of + // (int)cloudDx only provides the trivial floor of int. x - colsToTranslate + // must not be reported as a negative index. + check("static float cloudDx;\n" + "static int g_dst[400];\n" + "static int g_src[400];\n" + "void updateClouds(float elapsedTime) {\n" + " cloudDx += elapsedTime * 5;\n" + " if (cloudDx >= 1.0f) {\n" + " const int colsToTranslate = (int)cloudDx;\n" + " for (int x = colsToTranslate; x < 400; ++x) {\n" + " g_dst[x - colsToTranslate] = g_src[x];\n" + " }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check("void negative_index(int idx) {\n" + " int buf[10];\n" + " if (idx < 0) return;\n" + " buf[idx - 1] = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:13] -> [test.cpp:4:8]: (warning) Either the condition 'idx<0' is redundant or the array 'buf[10]' is accessed at index -1, which is out of bounds. [negativeIndex]\n", + errout_str()); + + // A genuinely negative literal index must still be reported + check("static int a[10];\n" + "void f() {\n" + " a[-1] = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:4]: (error) Array 'a[10]' accessed at index -1, which is out of bounds. [negativeIndex]\n", + errout_str()); + } + void array_index_multidim() { check("void f()\n" "{\n" diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 93248837cb3..3e60995242f 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -46,6 +46,7 @@ class TestCondition : public TestFixture { TEST_CASE(assignAndCompare); // assignment and comparison don't match TEST_CASE(mismatchingBitAnd); // overlapping bitmasks TEST_CASE(comparison); // CheckCondition::comparison test cases + TEST_CASE(unsignedArithmeticNoWrap); TEST_CASE(multicompare); // mismatching comparisons TEST_CASE(overlappingElseIfCondition); // overlapping conditions in if and else-if TEST_CASE(oppositeElseIfCondition); // opposite conditions in if and else-if @@ -6633,6 +6634,99 @@ class TestCondition : public TestFixture { "[test.cpp:4:13]: (style) Comparing expression of type 'const unsigned int &' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n", errout_str()); + // no false positive when unsigned arithmetic wraps around + check("void f(int x) {\n" + " if ((unsigned int)x - 1u == 0xFFFFFFFFu) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("", errout_str()); + + check("void f(int x) {\n" + " if ((unsigned short)x - 1u == 0xFFFFFFFFu) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("", errout_str()); + + // Signed result type after integer promotion must still warn + check("void f(int x) {\n" + " if ((unsigned short)x + 1 == 0) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:2:29]: (style) Condition '(unsigned short)x+1==0' is always false [knownConditionTrueFalse]\n", + errout_str()); + + // Direct cast bounds are preserved + check("void f(int x) {\n" + " if ((unsigned int)x > 4294967295ULL) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:2:25]: (style) Comparing expression of type 'unsigned int' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n", + errout_str()); + + // Reproduce the original typedef-heavy Simulink-generated pattern from #15000. + check("void f(unsigned int x) {\n" + " unsigned long long tmp = ((unsigned long long)x) + 1ULL;\n" + " if (tmp > 4294967295ULL)\n" + " tmp = 4294967295ULL;\n" + " if ((((long long)((unsigned int)tmp)) - 1LL) < 0LL) {}\n" + " if ((((long long)((unsigned int)tmp)) - 1LL) > 4294967295LL) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:6:50]: (style) Condition '(((long long)((unsigned int)tmp))-1LL)>4294967295LL' is always false [knownConditionTrueFalse]\n", + errout_str()); + + check("void f(unsigned long long tmp) {\n" + " if (tmp > 4294967295ULL)\n" + " tmp = 4294967295ULL;\n" + " if (static_cast(static_cast(tmp)) - 1LL > 4294967295LL) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:4:70]: (style) Condition 'static_cast(static_cast(tmp))-1LL>4294967295LL' is always false [knownConditionTrueFalse]\n", + errout_str()); + + // cast directly around variable: both the range-based and the declared-type + // analysis can prove the condition invariant; diag() must prevent duplicates + check("void f(unsigned char c) {\n" + " if ((unsigned char)c > 255) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:2:28]: (style) Comparing expression of type 'unsigned char' against value 255. Condition is always false. [compareValueOutOfTypeRangeError]\n", + errout_str()); + + check("void f(unsigned char c) {\n" + " if ((unsigned char)c == 256) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:2:29]: (style) Comparing expression of type 'unsigned char' against value 256. Condition is always false. [compareValueOutOfTypeRangeError]\n", + errout_str()); + + check("void f(unsigned int u) {\n" + " if ((unsigned int)u > 4294967295ULL) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:2:27]: (style) Comparing expression of type 'unsigned int' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n", + errout_str()); + + check("void f(unsigned short s) {\n" + " if ((unsigned int)s > 4294967295ULL) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:2:27]: (style) Comparing expression of type 'unsigned int' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n", + errout_str()); + + // wchar_t range is derived through ValueType::getSizeOf() + check("void f(wchar_t c) {\n" + " if ((wchar_t)c > 0x7fffffff) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:2:20]: (style) Condition '(wchar_t)c>0x7fffffff' is always false [knownConditionTrueFalse]\n", + errout_str()); + + check("void f(unsigned int x) {\n" + " if (-(signed char)x < -129) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("[test.cpp:2:25]: (style) Condition '-(char)x<-129' is always false [knownConditionTrueFalse]\n", + errout_str()); + + check("void f(int x) {\n" + " if ((x) > 0) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("", errout_str()); + + check("void f(unsigned int x) {\n" + " if ((unsigned int)x > 0) {}\n" + "}\n", settingsUnix64); + ASSERT_EQUALS("", errout_str()); + check("void f() {\n" " long long ll = 1024 * 1024 * 1024;\n" " if (ll * 8 < INT_MAX) {}\n" @@ -6852,7 +6946,6 @@ class TestCondition : public TestFixture { ASSERT_EQUALS( "[test.cpp:3:12]: (style) Condition 'nf>+1.0' is always false [knownConditionTrueFalse]\n", errout_str()); - check("void foo() {\n" // #11200 " float f = 1.0;\n" " if (f > -1.0) {}\n" @@ -6904,6 +6997,39 @@ class TestCondition : public TestFixture { "", errout_str()); } + + void unsignedArithmeticNoWrap() + { + check("int safe_subtraction(unsigned int count) {\n" + " if (count < 2) return 0;\n" + " if (count - 1 > 0) return 1;\n" + " return 0;\n" + "}\n"); + ASSERT(errout_str().find("Condition 'count-1>0' is always true") != std::string::npos); + + check("int safe_addition(unsigned int value) {\n" + " if (value < 10) {\n" + " if (value + 1 > 10) return 1;\n" + " }\n" + " return 0;\n" + "}\n"); + ASSERT(errout_str().find("Condition 'value+1>10' is always false") != std::string::npos); + + check("int tinyexr_range(unsigned long size) {\n" + " if (size < 6) return 0;\n" + " unsigned long offset = 2;\n" + " if (size - offset < 4) return 1;\n" + " return 0;\n" + "}\n"); + ASSERT(errout_str().find("Condition 'size-offset<4' is always false") != std::string::npos); + + check("void wrap_cases(int value) {\n" + " if ((unsigned int)value - 1u == 4294967295u) {}\n" + " if ((unsigned int)value + 1u == 0u) {}\n" + " if ((unsigned int)value * 2u > 1u) {}\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + } }; REGISTER_TEST(TestCondition) diff --git a/test/testother.cpp b/test/testother.cpp index 60c2a3bc528..43d8aebc418 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -10451,6 +10451,12 @@ class TestOther : public TestFixture { check("const int x = -1 >> 2;\n"); ASSERT_EQUALS("[test.cpp:1:18]: (portability) Shifting a negative value is technically undefined behaviour [shiftNegativeLHS]\n", errout_str()); + check("int negative_lhs(int value) {\n" + " if (value < -16 || value > 15) return 0;\n" + " return value << 10;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:18]: (portability) Shifting a negative value is technically undefined behaviour [shiftNegativeLHS]\n", errout_str()); + // #6383 - unsigned type check("const int x = (unsigned int)(-1) >> 2;\n"); ASSERT_EQUALS("", errout_str()); diff --git a/test/teststl.cpp b/test/teststl.cpp index bb20d2d8182..398c7243666 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -526,7 +526,7 @@ class TestStl : public TestFixture { " v.resize(entries + 2);\n" " v[0] = 1;\n" "}\n"); - ASSERT_EQUALS("", errout_str()); + ASSERT_EQUALS("[test.cpp:5:6]: warning: Out of bounds access in expression 'v[0]' because 'v' is empty. [containerOutOfBounds]\n", errout_str()); checkNormal("void f(size_t entries) {\n" " std::vector v;\n" diff --git a/test/testtype.cpp b/test/testtype.cpp index 1b5b57f41ca..8ed27dff93d 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -636,6 +636,41 @@ class TestType : public TestFixture { "};\n" "static B<64> b;\n"); ASSERT_EQUALS("", errout_str()); + + // FP shiftTooManyBits: guarded shift where the shift amount is + // conditionally derived from an unsigned subtraction; a possible value + // from an interprocedural caller must not collapse the interval + // inference on the unsigned arithmetic result + { + const Settings settings = settingsBuilder().platform(Platform::Type::Unix64).build(); + check("static unsigned int l[16];\n" + "void scalar_shift(unsigned int shift) {\n" + " unsigned int shiftlimbs = shift >> 5;\n" + " unsigned int shiftlow = shift & 0x1Fu;\n" + " unsigned int shifthigh = 32u - shiftlow;\n" + " unsigned int r = 0u;\n" + " r |= (shift < 448u && shiftlow ? (l[1 + shiftlimbs] << shifthigh) : 0u);\n" + " r |= (shift < 416u && shiftlow ? (l[2 + shiftlimbs] << shifthigh) : 0u);\n" + " (void)r;\n" + "}\n" + "int main(void) { scalar_shift(384u); return 0; }\n", + dinit(CheckOptions, $.settings = &settings)); + ASSERT_EQUALS("", errout_str()); + } + + check("int guarded_shift(int delta, unsigned int lag_depth, unsigned int lag_mask) {\n" + " if (lag_depth > 20) return 0;\n" + " if (delta < -100) return 0;\n" + " if (delta < -(int)lag_depth) return 0;\n" + " if (delta < 0) return lag_mask >> -delta;\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check("unsigned int oversized_shift(unsigned int value) {\n" + " return value >> 32;\n" + "}\n"); + ASSERT(errout_str().find("[shiftTooManyBits]") != std::string::npos); } }; diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index f84f0ea0d89..e349913c941 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -19,11 +19,14 @@ #include "errortypes.h" #include "fixture.h" #include "helpers.h" +#include "infer.h" #include "mathlib.h" #include "platform.h" #include "settings.h" #include "standards.h" #include "token.h" +#include "valueptr.h" +#include "vf_common.h" #include "vfvalue.h" #include @@ -160,6 +163,7 @@ class TestValueFlow : public TestFixture { TEST_CASE(valueFlowSymbolicStrlen); TEST_CASE(valueFlowSmartPointer); TEST_CASE(valueFlowImpossibleMinMax); + TEST_CASE(inferPathCompatibility); TEST_CASE(valueFlowImpossibleIncDec); TEST_CASE(valueFlowImpossibleUnknownConstant); TEST_CASE(valueFlowContainerEqual); @@ -8060,6 +8064,31 @@ class TestValueFlow : public TestFixture { " S(std::max_align_t T::* p) : m(p) {}\n" "};\n"; (void)valueOfTok(code, ":"); // don't crash + + const char libraryXml[] = "" + "" + "" + ""; + Settings charSettings = settingsBuilder().libraryxml(libraryXml).platform(Platform::Type::Unspecified).build(); + charSettings.checkUnknownFunctionReturn.insert("unknown_char"); + charSettings.checkUnknownFunctionReturn.insert("signed_char"); + const std::list signedReturnValues = tokenValues("x = signed_char();\n", "(", &charSettings); + ASSERT_EQUALS(2, signedReturnValues.size()); + ASSERT_EQUALS(-128, signedReturnValues.front().intvalue); + ASSERT_EQUALS(127, signedReturnValues.back().intvalue); + const std::list unknownReturnValues = tokenValues("x = unknown_char();\n", "(", &charSettings); + ASSERT(unknownReturnValues.empty() || + std::any_of(unknownReturnValues.cbegin(), unknownReturnValues.cend(), [](const ValueFlow::Value& value) { + return value.isIntValue() && value.intvalue >= 200; + })); + const char charCode[] = "char unknown_char(void);\n" + "void f() {\n" + " if (unknown_char() == 200) {}\n" + "}\n"; + const std::list comparisonValues = tokenValues(charCode, "==", &charSettings); + ASSERT(std::none_of(comparisonValues.cbegin(), comparisonValues.cend(), [](const ValueFlow::Value& value) { + return value.isKnown() && value.isIntValue() && value.intvalue == 0; + })); } void valueFlowUnknownFunctionReturnRand() { @@ -9623,6 +9652,67 @@ class TestValueFlow : public TestFixture { "}\n"; ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1)); ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1)); + + const Settings settingsUnix64 = settingsBuilder().platform(Platform::Type::Unix64).build(); + code = "void f(unsigned long long x) {\n" + " return (unsigned int)x;\n" + "}\n"; + SimpleTokenizer tokenizer(settingsUnix64, *this); + ASSERT(tokenizer.tokenize(code)); + const Token* returnTok = Token::findmatch(tokenizer.tokens(), "return ("); + ASSERT(returnTok && returnTok->next()); + const std::list& castValues = returnTok->next()->values(); + ASSERT(std::any_of(castValues.cbegin(), castValues.cend(), [](const ValueFlow::Value& value) { + return value.isImpossible() && value.intvalue == -1; + })); + ASSERT(std::any_of(castValues.cbegin(), castValues.cend(), [](const ValueFlow::Value& value) { + return value.isImpossible() && value.intvalue == 4294967296; + })); + + // Impossible bounds on a cast must not be propagated through + // arithmetic with unsigned result type, since wrap-around invalidates the bound + code = "void f(int x) {\n" + " return (unsigned int)x - 1u;\n" + "}\n"; + SimpleTokenizer tokenizer2(settingsUnix64, *this); + ASSERT(tokenizer2.tokenize(code)); + const Token* minusTok = Token::findsimplematch(tokenizer2.tokens(), "-"); + ASSERT(minusTok); + for (const ValueFlow::Value& value : minusTok->values()) { + ASSERT(!(value.isImpossible() && value.isIntValue() && + value.bound != ValueFlow::Value::Bound::Point && value.intvalue == 4294967295)); + } + + // Known values are still propagated through unsigned arithmetic + code = "void f(int x) {\n" + " unsigned int y = 5u;\n" + " return y - 1u;\n" + "}\n"; + SimpleTokenizer tokenizer3(settingsUnix64, *this); + ASSERT(tokenizer3.tokenize(code)); + const Token* minusTok3 = Token::findsimplematch(tokenizer3.tokens(), "-"); + ASSERT(minusTok3); + ASSERT(std::any_of(minusTok3->values().cbegin(), minusTok3->values().cend(), [](const ValueFlow::Value& value) { + return value.isKnown() && value.isIntValue() && value.intvalue == 4; + })); + + ValueType charType(ValueType::Sign::SIGNED, ValueType::Type::CHAR, 0); + MathLib::bigint minValue; + MathLib::bigint maxValue; + ASSERT(ValueFlow::getMinMaxValues(&charType, settingsUnix64.platform, minValue, maxValue)); + ASSERT_EQUALS(-128, minValue); + ASSERT_EQUALS(127, maxValue); + const ValueFlow::Value castInput(255); + ASSERT_EQUALS(-1, ValueFlow::castValue(castInput, ValueType::Sign::SIGNED, 8).intvalue); + + charType.sign = ValueType::Sign::UNSIGNED; + ASSERT(ValueFlow::getMinMaxValues(&charType, settingsUnix64.platform, minValue, maxValue)); + ASSERT_EQUALS(0, minValue); + ASSERT_EQUALS(255, maxValue); + ASSERT_EQUALS(255, ValueFlow::castValue(castInput, ValueType::Sign::UNSIGNED, 8).intvalue); + + charType.sign = ValueType::UNKNOWN_SIGN; + ASSERT_EQUALS(false, ValueFlow::getMinMaxValues(&charType, settingsUnix64.platform, minValue, maxValue)); } void valueFlowImpossibleIncDec() @@ -9648,6 +9738,112 @@ class TestValueFlow : public TestFixture { "}\n"; ASSERT_EQUALS(true, testValueOfXImpossible(code, 7U, -1)); ASSERT_EQUALS(false, testValueOfXKnown(code, 7U, -1)); + + code = "int guarded_predecrement(unsigned int x) {\n" + " if (x > 0) {\n" + " --x;\n" + " if (x == 0) return 1;\n" + " }\n" + " return 0;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfXImpossible(code, 4U, 0)); + + code = "int guarded_postdecrement(unsigned int x) {\n" + " if (x > 0) {\n" + " x--;\n" + " if (x == 0) return 1;\n" + " }\n" + " return 0;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfXImpossible(code, 4U, 0)); + + code = "int decimal_dot(const char* current, const char* first, int dot) {\n" + " unsigned int digits = (unsigned int)(current - first);\n" + " if (digits > 0) {\n" + " if (dot) {\n" + " digits--;\n" + " if (digits == 0) return 1;\n" + " }\n" + " }\n" + " return 0;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfXImpossible(code, 6U, 0)); + + code = "int wrap_decrement(unsigned int x) {\n" + " x--;\n" + " if (x == 4294967295u) return 1;\n" + " return 0;\n" + "}\n"; + SimpleTokenizer wrapTokenizer(settings, *this); + ASSERT(wrapTokenizer.tokenize(code)); + const Token* wrappedValue = nullptr; + for (const Token* tok = wrapTokenizer.tokens(); tok; tok = tok->next()) { + if (tok->str() == "x" && tok->linenr() == 3) { + wrappedValue = tok; + break; + } + } + if (wrappedValue == nullptr) { + ASSERT(wrappedValue); + return; + } + ASSERT(std::none_of(wrappedValue->values().cbegin(), + wrappedValue->values().cend(), + [](const ValueFlow::Value& value) { + return value.isImpossible() && value.isIntValue() && value.intvalue == 4294967295LL; + })); + } + + void inferPathCompatibility() + { + SimpleTokenizer tokenizer(settings, *this); + ASSERT(tokenizer.tokenize("int f(int x) { if (x) return 0; return 1; }\n")); + const Token* const condition = Token::findmatch(tokenizer.tokens(), "if"); + ASSERT(condition); + + ValueFlow::Value lhs(384); + lhs.setPossible(); + lhs.condition = condition; + lhs.path = 7; + lhs.conditional = true; + lhs.varId = 1; + lhs.varvalue = 384; + ValueFlow::Value rhs(32); + rhs.setKnown(); + + std::vector inferred = infer(makeIntegralInferModel(), "-", {lhs}, {rhs}); + ASSERT_EQUALS(1U, inferred.size()); + ASSERT(inferred.front().isPossible()); + ASSERT_EQUALS(7, inferred.front().path); + ASSERT(inferred.front().condition == condition); + ASSERT(inferred.front().conditional); + ASSERT_EQUALS(1, inferred.front().varId); + ASSERT_EQUALS(384, inferred.front().varvalue); + + ValueFlow::Value rhsSamePath(32); + rhsSamePath.setPossible(); + rhsSamePath.condition = condition; + rhsSamePath.path = 7; + inferred = infer(makeIntegralInferModel(), "-", {lhs}, {rhsSamePath}); + ASSERT_EQUALS(1U, inferred.size()); + ASSERT_EQUALS(7, inferred.front().path); + ASSERT(inferred.front().condition == condition); + + ValueFlow::Value rhsDifferentPath(32); + rhsDifferentPath.setPossible(); + rhsDifferentPath.path = 8; + inferred = infer(makeIntegralInferModel(), "-", {lhs}, {rhsDifferentPath}); + ASSERT(inferred.empty()); + + ValueFlow::Value knownLhs(9); + knownLhs.setKnown(); + ValueFlow::Value knownRhs(4); + knownRhs.setKnown(); + inferred = infer(makeIntegralInferModel(), "-", {knownLhs}, {knownRhs}); + ASSERT_EQUALS(1U, inferred.size()); + ASSERT(inferred.front().isKnown()); + ASSERT_EQUALS(0, inferred.front().path); + ASSERT_EQUALS(5, inferred.front().intvalue); } void valueFlowImpossibleUnknownConstant()