From a1e0ef9b38f42f6055bc39a33b9006338731d28b Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 9 Oct 2021 09:19:06 -0500 Subject: [PATCH] Refactor: Use interval analysis for symbolic values for inferCondtion (#3488) --- Makefile | 6 +- lib/infer.cpp | 362 +++++++++++++++++++++++++++++++ lib/infer.h | 52 +++++ lib/lib.pri | 2 + lib/valueflow.cpp | 481 +++-------------------------------------- test/testcondition.cpp | 21 +- 6 files changed, 466 insertions(+), 458 deletions(-) create mode 100644 lib/infer.cpp create mode 100644 lib/infer.h diff --git a/Makefile b/Makefile index 51631663a..47e5fecc9 100644 --- a/Makefile +++ b/Makefile @@ -194,6 +194,7 @@ LIBOBJ = $(libcppdir)/analyzerinfo.o \ $(libcppdir)/exprengine.o \ $(libcppdir)/forwardanalyzer.o \ $(libcppdir)/importproject.o \ + $(libcppdir)/infer.o \ $(libcppdir)/library.o \ $(libcppdir)/mathlib.o \ $(libcppdir)/path.o \ @@ -521,6 +522,9 @@ $(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/analyzer.h lib/astut $(libcppdir)/importproject.o: lib/importproject.cpp externals/picojson/picojson.h externals/tinyxml2/tinyxml2.h lib/astutils.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/importproject.o $(libcppdir)/importproject.cpp +$(libcppdir)/infer.o: lib/infer.cpp lib/astutils.h lib/calculate.h lib/config.h lib/errortypes.h lib/infer.h lib/mathlib.h lib/utils.h lib/valueflow.h lib/valueptr.h + $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/infer.o $(libcppdir)/infer.cpp + $(libcppdir)/library.o: lib/library.cpp externals/tinyxml2/tinyxml2.h lib/astutils.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/library.o $(libcppdir)/library.cpp @@ -578,7 +582,7 @@ $(libcppdir)/tokenlist.o: lib/tokenlist.cpp externals/simplecpp/simplecpp.h lib/ $(libcppdir)/utils.o: lib/utils.cpp lib/config.h lib/utils.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/utils.o $(libcppdir)/utils.cpp -$(libcppdir)/valueflow.o: lib/valueflow.cpp lib/analyzer.h lib/astutils.h lib/calculate.h lib/check.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/programmemory.h lib/reverseanalyzer.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h +$(libcppdir)/valueflow.o: lib/valueflow.cpp lib/analyzer.h lib/astutils.h lib/calculate.h lib/check.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/infer.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/programmemory.h lib/reverseanalyzer.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/valueflow.o $(libcppdir)/valueflow.cpp cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlineparser.h cli/cppcheckexecutor.h cli/filelister.h cli/threadexecutor.h externals/tinyxml2/tinyxml2.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h diff --git a/lib/infer.cpp b/lib/infer.cpp new file mode 100644 index 000000000..a7e5a7112 --- /dev/null +++ b/lib/infer.cpp @@ -0,0 +1,362 @@ +#include "infer.h" +#include "calculate.h" +#include "valueptr.h" +#include +#include + +template +static const ValueFlow::Value* getCompareValue(const std::list& values, Predicate pred, Compare compare) +{ + const ValueFlow::Value* result = nullptr; + for (const ValueFlow::Value& value : values) { + if (!pred(value)) + continue; + if (result) + result = &std::min(value, *result, [compare](const ValueFlow::Value& x, const ValueFlow::Value& y) { + return compare(x.intvalue, y.intvalue); + }); + else + result = &value; + } + return result; +} + +struct Interval { + std::vector minvalue = {}; + std::vector maxvalue = {}; + std::vector minRef = {}; + std::vector maxRef = {}; + + std::string str() const + { + std::string result = "["; + if (minvalue.size() == 1) + result += std::to_string(minvalue.front()); + else + result += "*"; + result += ","; + if (maxvalue.size() == 1) + result += std::to_string(maxvalue.front()); + else + result += "*"; + result += "]"; + return result; + } + + void setMinValue(MathLib::bigint x, const ValueFlow::Value* ref = nullptr) + { + minvalue = {x}; + if (ref) + minRef = {ref}; + } + + void setMaxValue(MathLib::bigint x, const ValueFlow::Value* ref = nullptr) + { + maxvalue = {x}; + if (ref) + maxRef = {ref}; + } + + bool isLessThan(MathLib::bigint x, std::vector* ref = nullptr) const + { + if (!this->maxvalue.empty() && this->maxvalue.front() < x) { + if (ref) + *ref = maxRef; + return true; + } + return false; + } + + bool isGreaterThan(MathLib::bigint x, std::vector* ref = nullptr) const + { + if (!this->minvalue.empty() && this->minvalue.front() > x) { + if (ref) + *ref = minRef; + return true; + } + return false; + } + + bool isScalar() const { + return minvalue.size() == 1 && minvalue == maxvalue; + } + + bool empty() const { + return minvalue.empty() && maxvalue.empty(); + } + + bool isScalarOrEmpty() const { + return empty() || isScalar(); + } + + MathLib::bigint getScalar() const + { + assert(isScalar()); + return minvalue.front(); + } + + std::vector getScalarRef() const + { + assert(isScalar()); + if (minRef != maxRef) + return merge(minRef, maxRef); + return minRef; + } + + static Interval fromInt(MathLib::bigint x, const ValueFlow::Value* ref = nullptr) + { + Interval result; + result.setMinValue(x, ref); + result.setMaxValue(x, ref); + return result; + } + + template + static Interval fromValues(const std::list& values, Predicate predicate) + { + Interval result; + const ValueFlow::Value* minValue = getCompareValue(values, predicate, std::less{}); + if (minValue) { + if (minValue->isImpossible() && minValue->bound == ValueFlow::Value::Bound::Upper) + result.setMinValue(minValue->intvalue + 1, minValue); + if (minValue->isPossible() && minValue->bound == ValueFlow::Value::Bound::Lower) + result.setMinValue(minValue->intvalue, minValue); + if (minValue->isKnown()) + return Interval::fromInt(minValue->intvalue, minValue); + } + const ValueFlow::Value* maxValue = getCompareValue(values, predicate, std::greater{}); + if (maxValue) { + if (maxValue->isImpossible() && maxValue->bound == ValueFlow::Value::Bound::Lower) + result.setMaxValue(maxValue->intvalue - 1, maxValue); + if (maxValue->isPossible() && maxValue->bound == ValueFlow::Value::Bound::Upper) + result.setMaxValue(maxValue->intvalue, maxValue); + assert(!maxValue->isKnown()); + } + return result; + } + + static Interval fromValues(const std::list& values) + { + return Interval::fromValues(values, [](const ValueFlow::Value&) { + return true; + }); + } + + template + static std::vector apply(const std::vector& x, + const std::vector& y, + F f) + { + if (x.empty()) + return {}; + if (y.empty()) + return {}; + return {f(x.front(), y.front())}; + } + + static std::vector merge(std::vector x, + const std::vector& y) + { + x.insert(x.end(), y.begin(), y.end()); + return x; + } + + friend Interval operator-(const Interval& lhs, const Interval& rhs) + { + Interval result; + result.minvalue = Interval::apply(lhs.minvalue, rhs.maxvalue, std::minus{}); + result.maxvalue = Interval::apply(lhs.maxvalue, rhs.minvalue, std::minus{}); + if (!result.minvalue.empty()) + result.minRef = merge(lhs.minRef, rhs.maxRef); + if (!result.maxvalue.empty()) + result.maxRef = merge(lhs.maxRef, rhs.minRef); + return result; + } + + static std::vector equal(const Interval& lhs, + const Interval& rhs, + std::vector* ref = nullptr) + { + if (!lhs.isScalar()) + return {}; + if (!rhs.isScalar()) + return {}; + if (ref) + *ref = merge(lhs.getScalarRef(), rhs.getScalarRef()); + return {lhs.minvalue == rhs.minvalue}; + } + + static std::vector compare(const Interval& lhs, + const Interval& rhs, + std::vector* ref = nullptr) + { + Interval diff = lhs - rhs; + if (diff.isGreaterThan(0, ref)) + return {1}; + if (diff.isLessThan(0, ref)) + return {-1}; + std::vector eq = Interval::equal(lhs, rhs, ref); + if (!eq.empty()) { + if (eq.front() == 0) + return {1, -1}; + else + return {0}; + } + if (diff.isGreaterThan(-1, ref)) + return {0, 1}; + if (diff.isLessThan(1, ref)) + return {0, -1}; + return {}; + } + + static std::vector compare(const std::string& op, + const Interval& lhs, + const Interval& rhs, + std::vector* ref = nullptr) + { + std::vector r = compare(lhs, rhs, ref); + if (r.empty()) + return {}; + bool b = calculate(op, r.front(), 0); + if (std::all_of(r.begin() + 1, r.end(), [&](int i) { + return b == calculate(op, i, 0); + })) + return {b}; + return {}; + } +}; + +std::string toString(const Interval& i) { + return i.str(); +} + +static void addToErrorPath(ValueFlow::Value& value, const std::vector& refs) +{ + std::unordered_set locations; + for (const ValueFlow::Value* ref : refs) { + if (ref->condition && !value.condition) + value.condition = ref->condition; + std::copy_if(ref->errorPath.begin(), + ref->errorPath.end(), + std::back_inserter(value.errorPath), + [&](const ErrorPathItem& e) { + return locations.insert(e.first).second; + }); + } +} + +static void setValueKind(ValueFlow::Value& value, const std::vector& refs) +{ + bool isPossible = false; + bool isInconclusive = false; + for (const ValueFlow::Value* ref : refs) { + if (ref->isPossible()) + isPossible = true; + if (ref->isInconclusive()) + isInconclusive = true; + } + if (isInconclusive) + value.setInconclusive(); + else if (isPossible) + value.setPossible(); + else + value.setKnown(); +} + +static bool inferNotEqual(const std::list& values, MathLib::bigint x) +{ + return std::any_of(values.begin(), values.end(), [&](const ValueFlow::Value& value) { + return value.isImpossible() && value.intvalue == x; + }); +} + +std::vector infer(const ValuePtr& model, + const std::string& op, + std::list lhsValues, + std::list rhsValues) +{ + std::vector result; + auto notMatch = [&](const ValueFlow::Value& value) { + return !model->match(value); + }; + lhsValues.remove_if(notMatch); + rhsValues.remove_if(notMatch); + if (lhsValues.empty() || 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(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(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(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(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(value); + } + } + } else { + std::vector refs; + std::vector r = Interval::compare(op, lhs, rhs, &refs); + if (!r.empty()) { + ValueFlow::Value value(r.front()); + addToErrorPath(value, refs); + setValueKind(value, refs); + result.push_back(value); + } + } + + return result; +} + +std::vector infer(const ValuePtr& model, + const std::string& op, + MathLib::bigint lhs, + std::list rhsValues) +{ + return infer(model, op, {model->yield(lhs)}, std::move(rhsValues)); +} + +std::vector infer(const ValuePtr& model, + const std::string& op, + std::list lhsValues, + MathLib::bigint rhs) +{ + return infer(model, op, std::move(lhsValues), {model->yield(rhs)}); +} diff --git a/lib/infer.h b/lib/infer.h new file mode 100644 index 000000000..b77e611cc --- /dev/null +++ b/lib/infer.h @@ -0,0 +1,52 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2021 Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef inferH +#define inferH + +#include "mathlib.h" +#include "valueflow.h" + +struct Interval; +template +class ValuePtr; + +struct InferModel { + virtual bool match(const ValueFlow::Value& value) const = 0; + virtual ValueFlow::Value yield(MathLib::bigint value) const = 0; + virtual ~InferModel() {} +}; + +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, + MathLib::bigint lhs, + std::list rhsValues); + +std::vector infer(const ValuePtr& model, + const std::string& op, + std::list lhsValues, + MathLib::bigint rhs); + +std::string toString(const Interval& i); + +#endif diff --git a/lib/lib.pri b/lib/lib.pri index d05e87668..b2c957825 100644 --- a/lib/lib.pri +++ b/lib/lib.pri @@ -41,6 +41,7 @@ HEADERS += $${PWD}/analyzerinfo.h \ $${PWD}/exprengine.h \ $${PWD}/forwardanalyzer.h \ $${PWD}/importproject.h \ + $${PWD}/infer.h \ $${PWD}/library.h \ $${PWD}/mathlib.h \ $${PWD}/path.h \ @@ -100,6 +101,7 @@ SOURCES += $${PWD}/analyzerinfo.cpp \ $${PWD}/exprengine.cpp \ $${PWD}/forwardanalyzer.cpp \ $${PWD}/importproject.cpp \ + $${PWD}/infer.cpp \ $${PWD}/library.cpp \ $${PWD}/mathlib.cpp \ $${PWD}/path.cpp \ diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index d570cc0a1..8fc60108c 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -85,6 +85,7 @@ #include "errorlogger.h" #include "errortypes.h" #include "forwardanalyzer.h" +#include "infer.h" #include "library.h" #include "mathlib.h" #include "path.h" @@ -4424,322 +4425,6 @@ static void valueFlowSymbolicAbs(TokenList* tokenlist, SymbolDatabase* symboldat } } -template -static const ValueFlow::Value* getCompareValue(const std::list& values, - Predicate pred, - Compare compare) -{ - const ValueFlow::Value* result = nullptr; - for (const ValueFlow::Value& value : values) { - if (!pred(value)) - continue; - if (result) - result = &std::min(value, *result, [compare](const ValueFlow::Value& x, const ValueFlow::Value& y) { - return compare(x.intvalue, y.intvalue); - }); - else - result = &value; - } - return result; -} - -struct Interval { - std::vector minvalue = {}; - std::vector maxvalue = {}; - std::vector minRef = {}; - std::vector maxRef = {}; - - void setMinValue(MathLib::bigint x, const ValueFlow::Value* ref = nullptr) - { - minvalue = {x}; - if (ref) - minRef = {ref}; - } - - void setMaxValue(MathLib::bigint x, const ValueFlow::Value* ref = nullptr) - { - maxvalue = {x}; - if (ref) - maxRef = {ref}; - } - - bool isLessThan(MathLib::bigint x, std::vector* ref = nullptr) const - { - if (!this->maxvalue.empty() && this->maxvalue.front() < x) { - if (ref) - *ref = maxRef; - return true; - } - return false; - } - - bool isGreaterThan(MathLib::bigint x, std::vector* ref = nullptr) const - { - if (!this->minvalue.empty() && this->minvalue.front() > x) { - if (ref) - *ref = minRef; - return true; - } - return false; - } - - bool isScalar() const { - return minvalue.size() == 1 && minvalue == maxvalue; - } - - bool empty() const { - return minvalue.empty() && maxvalue.empty(); - } - - bool isScalarOrEmpty() const { - return empty() || isScalar(); - } - - MathLib::bigint getScalar() const - { - assert(isScalar()); - return minvalue.front(); - } - - std::vector getScalarRef() const - { - assert(isScalar()); - if (!minRef.empty()) - return minRef; - if (!maxRef.empty()) - return maxRef; - return {}; - } - - static Interval fromInt(MathLib::bigint x, const ValueFlow::Value* ref = nullptr) - { - Interval result; - result.setMinValue(x, ref); - result.setMaxValue(x, ref); - return result; - } - - template - static Interval fromValues(const std::list& values, Predicate predicate) - { - Interval result; - const ValueFlow::Value* minValue = getCompareValue(values, predicate, std::less{}); - if (minValue) { - if (minValue->isImpossible() && minValue->bound == ValueFlow::Value::Bound::Upper) - result.setMinValue(minValue->intvalue + 1, minValue); - if (minValue->isPossible() && minValue->bound == ValueFlow::Value::Bound::Lower) - result.setMinValue(minValue->intvalue, minValue); - if (minValue->isKnown()) - return Interval::fromInt(minValue->intvalue, minValue); - } - const ValueFlow::Value* maxValue = getCompareValue(values, predicate, std::greater{}); - if (maxValue) { - if (maxValue->isImpossible() && maxValue->bound == ValueFlow::Value::Bound::Lower) - result.setMaxValue(maxValue->intvalue - 1, maxValue); - if (maxValue->isPossible() && maxValue->bound == ValueFlow::Value::Bound::Upper) - result.setMaxValue(maxValue->intvalue, maxValue); - assert(!maxValue->isKnown()); - } - return result; - } - - static Interval fromValues(const std::list& values) - { - return Interval::fromValues(values, [](const ValueFlow::Value&) { - return true; - }); - } - - template - static std::vector apply(const std::vector& x, - const std::vector& y, - F f) - { - if (x.empty()) - return {}; - if (y.empty()) - return {}; - return {f(x.front(), y.front())}; - } - - static std::vector merge(std::vector x, - const std::vector& y) - { - x.insert(x.end(), y.begin(), y.end()); - return x; - } - - friend Interval operator-(const Interval& lhs, const Interval& rhs) - { - Interval result; - result.minvalue = Interval::apply(lhs.minvalue, rhs.maxvalue, std::minus{}); - result.maxvalue = Interval::apply(lhs.maxvalue, rhs.minvalue, std::minus{}); - if (!result.minvalue.empty()) - result.minRef = merge(lhs.minRef, rhs.maxRef); - if (!result.maxvalue.empty()) - result.maxRef = merge(lhs.maxRef, rhs.minRef); - return result; - } - - static std::vector equal(const Interval& lhs, - const Interval& rhs, - std::vector* ref = nullptr) - { - if (!lhs.isScalar()) - return {}; - if (!rhs.isScalar()) - return {}; - if (ref) - *ref = merge(lhs.minRef, rhs.minRef); - return {lhs.minvalue == rhs.minvalue}; - } - - static std::vector compare(const Interval& lhs, - const Interval& rhs, - std::vector* ref = nullptr) - { - Interval diff = lhs - rhs; - if (diff.isGreaterThan(0, ref)) - return {1}; - if (diff.isLessThan(0, ref)) - return {-1}; - std::vector eq = Interval::equal(lhs, rhs, ref); - if (!eq.empty() && eq.front() != 0) - return {0}; - return {}; - } -}; - -static void addToErrorPath(ValueFlow::Value& value, const std::vector& refs) -{ - for (const ValueFlow::Value* ref : refs) { - value.errorPath.insert(value.errorPath.end(), ref->errorPath.begin(), ref->errorPath.end()); - } -} - -static void setValueKind(ValueFlow::Value& value, const std::vector& refs) -{ - bool isPossible = false; - bool isInconclusive = false; - for (const ValueFlow::Value* ref : refs) { - if (ref->isPossible()) - isPossible = true; - if (ref->isInconclusive()) - isInconclusive = true; - } - if (isInconclusive) - value.setInconclusive(); - else if (isPossible) - value.setPossible(); - else - value.setKnown(); -} - -struct InferModel { - virtual bool match(const ValueFlow::Value& value) const = 0; - virtual ValueFlow::Value yield(MathLib::bigint value) const = 0; - virtual ~InferModel() {} -}; - -static bool inferNotEqual(const std::list& values, MathLib::bigint x) -{ - return std::any_of(values.begin(), values.end(), [&](const ValueFlow::Value& value) { - return value.isImpossible() && value.intvalue == x; - }); -} - -static std::vector infer(const ValuePtr& model, - const std::string& op, - std::list lhsValues, - std::list rhsValues) -{ - std::vector result; - auto notMatch = [&](const ValueFlow::Value& value) { - return !model->match(value); - }; - lhsValues.remove_if(notMatch); - rhsValues.remove_if(notMatch); - if (lhsValues.empty() || 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(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(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(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(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(value); - } - } - } else { - std::vector refs; - std::vector r = Interval::compare(lhs, rhs, &refs); - if (!r.empty()) { - int x = r.front(); - ValueFlow::Value value(calculate(op, x, 0)); - addToErrorPath(value, refs); - setValueKind(value, refs); - result.push_back(value); - } - } - - return result; -} - -static std::vector infer(const ValuePtr& model, - const std::string& op, - MathLib::bigint lhs, - std::list rhsValues) -{ - return infer(model, op, {model->yield(lhs)}, std::move(rhsValues)); -} - -static std::vector infer(const ValuePtr& model, - const std::string& op, - std::list lhsValues, - MathLib::bigint rhs) -{ - return infer(model, op, std::move(lhsValues), {model->yield(rhs)}); -} - struct SymbolicInferModel : InferModel { const Token* expr; explicit SymbolicInferModel(const Token* tok) : expr(tok) { @@ -5543,82 +5228,18 @@ struct SimpleConditionHandler : ConditionHandler { } }; -static bool isInBounds(const ValueFlow::Value& value, MathLib::bigint x) -{ - if (value.intvalue == x) - return true; - if (value.bound == ValueFlow::Value::Bound::Lower && value.intvalue > x) - return false; - if (value.bound == ValueFlow::Value::Bound::Upper && value.intvalue < x) - return false; - // Checking for equality is not necessary since we already know the value is not equal - if (value.bound == ValueFlow::Value::Bound::Point) - return false; - return true; -} - -static const ValueFlow::Value* getCompareIntValue(const std::list& values, std::function compare) -{ - const ValueFlow::Value* result = nullptr; - for (const ValueFlow::Value& value : values) { - if (!value.isIntValue()) - continue; - if (result) - result = &std::min(value, *result, [compare](const ValueFlow::Value& x, const ValueFlow::Value& y) { - return compare(x.intvalue, y.intvalue); - }); - else - result = &value; +struct IntegralInferModel : InferModel { + virtual bool match(const ValueFlow::Value& value) const OVERRIDE { + return value.isIntValue(); } - return result; -} - -static const ValueFlow::Value* proveLessThan(const std::list& values, MathLib::bigint x) -{ - const ValueFlow::Value* result = nullptr; - const ValueFlow::Value* maxValue = getCompareIntValue(values, std::greater {}); - if (maxValue && maxValue->isImpossible() && maxValue->bound == ValueFlow::Value::Bound::Lower) { - if (maxValue->intvalue <= x) - result = maxValue; + virtual ValueFlow::Value yield(MathLib::bigint value) const OVERRIDE + { + ValueFlow::Value result(value); + result.valueType = ValueFlow::Value::ValueType::INT; + result.setKnown(); + return result; } - return result; -} - -static const ValueFlow::Value* proveGreaterThan(const std::list& values, MathLib::bigint x) -{ - const ValueFlow::Value* result = nullptr; - const ValueFlow::Value* minValue = getCompareIntValue(values, std::less {}); - if (minValue && minValue->isImpossible() && minValue->bound == ValueFlow::Value::Bound::Upper) { - if (minValue->intvalue >= x) - result = minValue; - } - return result; -} - -static const ValueFlow::Value* proveNotEqual(const std::list& values, MathLib::bigint x) -{ - const ValueFlow::Value* result = nullptr; - for (const ValueFlow::Value& value : values) { - if (value.valueType != ValueFlow::Value::ValueType::INT) - continue; - if (result && !isInBounds(value, result->intvalue)) - continue; - if (value.isImpossible()) { - if (value.intvalue == x) - return &value; - if (!isInBounds(value, x)) - continue; - result = &value; - } else { - if (value.intvalue == x) - return nullptr; - if (!isInBounds(value, x)) - continue; - result = nullptr; - } - } - return result; -} +}; ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, MathLib::bigint val) { @@ -5626,52 +5247,22 @@ ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, Math return ValueFlow::Value{}; if (varTok->hasKnownIntValue()) return ValueFlow::Value{}; - if (std::none_of(varTok->values().begin(), varTok->values().end(), [](const ValueFlow::Value& v) { - return v.isImpossible() && v.valueType == ValueFlow::Value::ValueType::INT; - })) { - return ValueFlow::Value{}; - } - const ValueFlow::Value* result = nullptr; - bool known = false; - if (op == "==" || op == "!=") { - result = proveNotEqual(varTok->values(), val); - known = op == "!="; - } else if (op == "<" || op == ">=") { - result = proveLessThan(varTok->values(), val); - known = op == "<"; - if (!result && !isSaturated(val)) { - result = proveGreaterThan(varTok->values(), val - 1); - known = op == ">="; - } - } else if (op == ">" || op == "<=") { - result = proveGreaterThan(varTok->values(), val); - known = op == ">"; - if (!result && !isSaturated(val)) { - result = proveLessThan(varTok->values(), val + 1); - known = op == "<="; - } - } - if (!result) - return ValueFlow::Value{}; - ValueFlow::Value value = *result; - value.intvalue = known; - value.bound = ValueFlow::Value::Bound::Point; - value.setKnown(); - return value; + std::vector r = infer(IntegralInferModel{}, op, varTok->values(), val); + if (r.size() == 1 && r.front().isKnown()) + return r.front(); + return ValueFlow::Value{}; } ValueFlow::Value inferCondition(std::string op, MathLib::bigint val, const Token* varTok) { - // Flip the operator - if (op == ">") - op = "<"; - else if (op == "<") - op = ">"; - else if (op == ">=") - op = "<="; - else if (op == "<=") - op = ">="; - return inferCondition(op, varTok, val); + if (!varTok) + return ValueFlow::Value{}; + if (varTok->hasKnownIntValue()) + return ValueFlow::Value{}; + std::vector r = infer(IntegralInferModel{}, op, val, varTok->values()); + if (r.size() == 1 && r.front().isKnown()) + return r.front(); + return ValueFlow::Value{}; } static void valueFlowInferCondition(TokenList* tokenlist, @@ -5684,30 +5275,20 @@ static void valueFlowInferCondition(TokenList* tokenlist, continue; if (tok->variable() && (Token::Match(tok->astParent(), "?|&&|!|%oror%") || Token::Match(tok->astParent()->previous(), "if|while ("))) { - const ValueFlow::Value* result = proveNotEqual(tok->values(), 0); - if (!result) + std::vector result = infer(IntegralInferModel{}, "!=", tok->values(), 0); + if (result.size() != 1) continue; - ValueFlow::Value value = *result; + ValueFlow::Value value = result.front(); value.intvalue = 1; value.bound = ValueFlow::Value::Bound::Point; value.setKnown(); setTokenValue(tok, value, settings); - } else if (tok->isComparisonOp()) { - ValueFlow::Value value{}; - std::string op = tok->str(); - if (tok->astOperand1()->hasKnownIntValue()) { - MathLib::bigint val = tok->astOperand1()->values().front().intvalue; - const Token* varTok = tok->astOperand2(); - value = inferCondition(tok->str(), val, varTok); - } else if (tok->astOperand2()->hasKnownIntValue()) { - MathLib::bigint val = tok->astOperand2()->values().front().intvalue; - const Token* varTok = tok->astOperand1(); - value = inferCondition(tok->str(), varTok, val); + } else if (Token::Match(tok, "%comp%|-") && tok->astOperand1() && tok->astOperand2()) { + std::vector result = + infer(IntegralInferModel{}, tok->str(), tok->astOperand1()->values(), tok->astOperand2()->values()); + for (const ValueFlow::Value& value : result) { + setTokenValue(tok, value, settings); } - - if (!value.isKnown()) - continue; - setTokenValue(tok, value, settings); } } } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index a46e0b2d2..8ab8fe07c 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -954,7 +954,7 @@ private: " }\n" " return false;\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'c!=a' is always false\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'c!=a' is always false\n", errout.str()); } void incorrectLogicOperator2() { @@ -3330,7 +3330,7 @@ private: " if (handle) return 1;\n" " else return 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'handle' is always true\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'handle' is always true\n", errout.str()); check("int f(void *handle) {\n" " if (handle != 0) return 0;\n" @@ -3393,7 +3393,7 @@ private: " if(array){}\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style) Condition 'array' is always true\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (style) Condition 'array' is always true\n", errout.str()); check("void f(int *array, int size ) {\n" " for(int i = 0; i < size; ++i) {\n" @@ -3402,7 +3402,7 @@ private: " else if(array){}\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style) Condition 'array' is always true\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (style) Condition 'array' is always true\n", errout.str()); // #9277 check("int f() {\n" @@ -3770,7 +3770,7 @@ private: check("void f(int i) {\n" " if(i < 0 && abs(i) == i) {}\n" "}\n"); - ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'abs(i)==i' is always false\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'abs(i)==i' is always false\n", errout.str()); check("void f(int i) {\n" " if(i > -3 && abs(i) == i) {}\n" @@ -3810,7 +3810,7 @@ private: " if(x == y) {}\n" " }\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'x==y' is always false\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'x==y' is always false\n", errout.str()); check("void f(bool a, bool b) { if (a == b && a && !b){} }"); ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Condition '!b' is always false\n", errout.str()); @@ -3824,7 +3824,7 @@ private: " if (z < 1) {}\n" " }\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'z<1' is always false\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'z<1' is always false\n", errout.str()); check("struct a {\n" " a *b() const;\n" @@ -4002,6 +4002,13 @@ private: " if(i<=18) {}\n" "}\n"); ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'i<=18' is always true\n", errout.str()); + + check("void f(unsigned int u1, unsigned int u2) {\n" + " if (u1 <= 10 && u2 >= 20) {\n" + " if (u1 != u2) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'u1!=u2' is always true\n", errout.str()); } void alwaysTrueContainer() {