From 83e5ff8af55b5f6591de7c439af7d38134e954a9 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 6 Oct 2021 21:12:42 -0500 Subject: [PATCH 01/13] Move infer to seperate cpp file --- lib/infer.cpp | 313 +++++++++++++++++++++++++++++++++++++++++++++ lib/infer.h | 48 +++++++ lib/valueflow.cpp | 317 +--------------------------------------------- 3 files changed, 362 insertions(+), 316 deletions(-) create mode 100644 lib/infer.cpp create mode 100644 lib/infer.h diff --git a/lib/infer.cpp b/lib/infer.cpp new file mode 100644 index 00000000000..f7883f07e34 --- /dev/null +++ b/lib/infer.cpp @@ -0,0 +1,313 @@ +#include "infer.h" +#include "calculate.h" +#include "valueptr.h" + +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(); +} + +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(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; +} + +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 00000000000..ba186163be3 --- /dev/null +++ b/lib/infer.h @@ -0,0 +1,48 @@ +/* + * 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" + +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); + +#endif diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index d570cc0a1ff..0ecd251e725 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) { From 2697151fbd6fbfad7be95680f6dc5ea3da2ab546 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 00:07:33 -0500 Subject: [PATCH 02/13] Merge all refs on equality --- lib/infer.cpp | 22 +++++++++++++++++++++- lib/infer.h | 3 +++ lib/valueflow.cpp | 34 +++++++++++++++++++--------------- 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/lib/infer.cpp b/lib/infer.cpp index f7883f07e34..ecaa11a764a 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -27,6 +27,21 @@ struct Interval { 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}; @@ -168,7 +183,7 @@ struct Interval { if (!rhs.isScalar()) return {}; if (ref) - *ref = merge(lhs.minRef, rhs.minRef); + *ref = merge(merge(lhs.minRef, rhs.minRef), merge(lhs.maxRef, rhs.maxRef)); return {lhs.minvalue == rhs.minvalue}; } @@ -188,6 +203,11 @@ struct Interval { } }; +std::string toString(const Interval& i) +{ + return i.str(); +} + static void addToErrorPath(ValueFlow::Value& value, const std::vector& refs) { for (const ValueFlow::Value* ref : refs) { diff --git a/lib/infer.h b/lib/infer.h index ba186163be3..33766cc5a74 100644 --- a/lib/infer.h +++ b/lib/infer.h @@ -22,6 +22,7 @@ #include "mathlib.h" #include "valueflow.h" +struct Interval; template class ValuePtr; struct InferModel { @@ -45,4 +46,6 @@ std::vector infer(const ValuePtr& model, std::list lhsValues, MathLib::bigint rhs); +std::string toString(const Interval& i); + #endif diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 0ecd251e725..49ad9a8bb8b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5305,6 +5305,20 @@ static const ValueFlow::Value* proveNotEqual(const std::list& return result; } +struct IntegralInferModel : InferModel { + virtual bool match(const ValueFlow::Value& value) const OVERRIDE + { + return value.isIntValue(); + } + virtual ValueFlow::Value yield(MathLib::bigint value) const OVERRIDE + { + ValueFlow::Value result(value); + result.valueType = ValueFlow::Value::ValueType::INT; + result.setKnown(); + return result; + } +}; + ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, MathLib::bigint val) { if (!varTok) @@ -5377,22 +5391,12 @@ static void valueFlowInferCondition(TokenList* tokenlist, 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()) { + // ValueFlow::Value value{}; + 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); } } } From 7edf0e73f36ac1857543fd63722418de0f72d572 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 09:57:18 -0500 Subject: [PATCH 03/13] Use scalarRef --- lib/infer.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/infer.cpp b/lib/infer.cpp index ecaa11a764a..e75bff7d123 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -97,11 +97,9 @@ struct Interval { std::vector getScalarRef() const { assert(isScalar()); - if (!minRef.empty()) - return minRef; - if (!maxRef.empty()) - return maxRef; - return {}; + if (minRef != maxRef) + return merge(minRef, maxRef); + return minRef; } static Interval fromInt(MathLib::bigint x, const ValueFlow::Value* ref = nullptr) @@ -183,7 +181,7 @@ struct Interval { if (!rhs.isScalar()) return {}; if (ref) - *ref = merge(merge(lhs.minRef, rhs.minRef), merge(lhs.maxRef, rhs.maxRef)); + *ref = merge(lhs.getScalarRef(), rhs.getScalarRef()); return {lhs.minvalue == rhs.minvalue}; } From a7b210e24b5012eac16b5e86317ee4a9bae494dd Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 11:52:48 -0500 Subject: [PATCH 04/13] Add test case --- test/testcondition.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/testcondition.cpp b/test/testcondition.cpp index a46e0b2d236..8605f04a087 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4002,6 +4002,13 @@ class TestCondition : public TestFixture { " 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("error", errout.str()); } void alwaysTrueContainer() { From 7a277c9db8549dc29f91271cfa26647bd8a71453 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 12:33:59 -0500 Subject: [PATCH 05/13] Propagate the condition var in valueflow --- lib/infer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/infer.cpp b/lib/infer.cpp index e75bff7d123..e05001c5f39 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -209,6 +209,8 @@ std::string toString(const Interval& i) static void addToErrorPath(ValueFlow::Value& value, const std::vector& refs) { for (const ValueFlow::Value* ref : refs) { + if (ref->condition && !value.condition) + value.condition = ref->condition; value.errorPath.insert(value.errorPath.end(), ref->errorPath.begin(), ref->errorPath.end()); } } From 081496358fa87974a0cd72d34cc270390fd4e6c9 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 13:21:54 -0500 Subject: [PATCH 06/13] Update error message --- test/testcondition.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 8605f04a087..78ca7a83c3a 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4008,7 +4008,7 @@ class TestCondition : public TestFixture { " if (u1 != u2) {}\n" " }\n" "}\n"); - ASSERT_EQUALS("error", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'u1!=u2' is always true\n", errout.str()); } void alwaysTrueContainer() { From d912146261d32be93779ca3876464d934a1ae8e6 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 18:19:14 -0500 Subject: [PATCH 07/13] Improve comparison with equality --- lib/infer.cpp | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/infer.cpp b/lib/infer.cpp index e05001c5f39..cd5af98ee3f 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -195,8 +195,30 @@ struct Interval { if (diff.isLessThan(0, ref)) return {-1}; std::vector eq = Interval::equal(lhs, rhs, ref); - if (!eq.empty() && eq.front() != 0) - return {0}; + 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 {}; } }; @@ -303,10 +325,9 @@ std::vector infer(const ValuePtr& model, } } else { std::vector refs; - std::vector r = Interval::compare(lhs, rhs, &refs); + std::vector r = Interval::compare(op, lhs, rhs, &refs); if (!r.empty()) { - int x = r.front(); - ValueFlow::Value value(calculate(op, x, 0)); + ValueFlow::Value value(r.front()); addToErrorPath(value, refs); setValueKind(value, refs); result.push_back(value); From 1f2ad71160e688f46ec9eaea2879f479e115e15e Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 18:30:07 -0500 Subject: [PATCH 08/13] Remove duplicate messages --- lib/infer.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/infer.cpp b/lib/infer.cpp index cd5af98ee3f..65c0db37091 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -1,6 +1,7 @@ #include "infer.h" #include "calculate.h" #include "valueptr.h" +#include template static const ValueFlow::Value* getCompareValue(const std::list& values, @@ -230,10 +231,13 @@ std::string toString(const Interval& i) 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; - value.errorPath.insert(value.errorPath.end(), ref->errorPath.begin(), ref->errorPath.end()); + std::copy_if(ref->errorPath.begin(), ref->errorPath.end(), std::back_inserter(value.errorPath), [&](const ErrorPathItem& e) { + return locations.insert(e.first).second; + }); } } From cdc7c71a02eefee605e3a45a11aac84576b81486 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 18:32:11 -0500 Subject: [PATCH 09/13] Update messages --- test/testcondition.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 78ca7a83c3a..b434db52ccc 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -954,7 +954,7 @@ class TestCondition : public TestFixture { " }\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() { @@ -3770,7 +3770,7 @@ class TestCondition : public TestFixture { 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 @@ class TestCondition : public TestFixture { " 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 @@ class TestCondition : public TestFixture { " 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" From 0c2590ad415f6cf3705b76ef5f07448742f04006 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 18:54:13 -0500 Subject: [PATCH 10/13] Remove unused methods --- lib/valueflow.cpp | 138 +++++------------------------------------ test/testcondition.cpp | 6 +- 2 files changed, 18 insertions(+), 126 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 49ad9a8bb8b..fe8d51fbce1 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5228,83 +5228,6 @@ 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; - } - 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; - } - 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; -} - struct IntegralInferModel : InferModel { virtual bool match(const ValueFlow::Value& value) const OVERRIDE { @@ -5325,52 +5248,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, @@ -5383,16 +5276,15 @@ 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 (Token::Match(tok, "%comp%|-") && tok->astOperand1() && tok->astOperand2()) { - // ValueFlow::Value value{}; std::vector result = infer(IntegralInferModel{}, tok->str(), tok->astOperand1()->values(), tok->astOperand2()->values()); for(const ValueFlow::Value& value:result) { setTokenValue(tok, value, settings); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index b434db52ccc..831e8b205e9 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -3330,7 +3330,7 @@ class TestCondition : public TestFixture { " 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 @@ class TestCondition : public TestFixture { " 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 @@ class TestCondition : public TestFixture { " 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" From 2f6e0a8e37e685f377e4161df0b6e2a56b355e32 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 18:54:42 -0500 Subject: [PATCH 11/13] Run dmake --- Makefile | 6 +++++- lib/lib.pri | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 51631663ad4..47e5fecc9d9 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/lib.pri b/lib/lib.pri index d05e87668e5..b2c95782524 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 \ From 938bde0f7646f7945a1ed884de3e4637a500d514 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 7 Oct 2021 18:56:33 -0500 Subject: [PATCH 12/13] Format --- lib/infer.cpp | 43 ++++++++++++++++++++++-------------------- lib/infer.h | 21 +++++++++++---------- lib/valueflow.cpp | 8 ++++---- test/testcondition.cpp | 8 ++++---- 4 files changed, 42 insertions(+), 38 deletions(-) diff --git a/lib/infer.cpp b/lib/infer.cpp index 65c0db37091..1b03b1639f1 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -4,9 +4,7 @@ #include template -static const ValueFlow::Value* getCompareValue(const std::list& values, - Predicate pred, - Compare compare) +static const ValueFlow::Value* getCompareValue(const std::list& values, Predicate pred, Compare compare) { const ValueFlow::Value* result = nullptr; for (const ValueFlow::Value& value : values) { @@ -28,7 +26,8 @@ struct Interval { std::vector minRef = {}; std::vector maxRef = {}; - std::string str() const { + std::string str() const + { std::string result = "["; if (minvalue.size() == 1) result += std::to_string(minvalue.front()); @@ -210,22 +209,23 @@ struct Interval { } static std::vector compare(const std::string& op, - const Interval& lhs, - const Interval& rhs, - std::vector* ref = nullptr) + 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); })) + 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) -{ +std::string toString(const Interval& i) { return i.str(); } @@ -235,7 +235,10 @@ static void addToErrorPath(ValueFlow::Value& value, const std::vectorcondition && !value.condition) value.condition = ref->condition; - std::copy_if(ref->errorPath.begin(), ref->errorPath.end(), std::back_inserter(value.errorPath), [&](const ErrorPathItem& e) { + std::copy_if(ref->errorPath.begin(), + ref->errorPath.end(), + std::back_inserter(value.errorPath), + [&](const ErrorPathItem& e) { return locations.insert(e.first).second; }); } @@ -267,9 +270,9 @@ static bool inferNotEqual(const std::list& values, MathLib::bi } std::vector infer(const ValuePtr& model, - const std::string& op, - std::list lhsValues, - std::list rhsValues) + const std::string& op, + std::list lhsValues, + std::list rhsValues) { std::vector result; auto notMatch = [&](const ValueFlow::Value& value) { @@ -342,17 +345,17 @@ std::vector infer(const ValuePtr& model, } std::vector infer(const ValuePtr& model, - const std::string& op, - MathLib::bigint lhs, - std::list rhsValues) + 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) + 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 index 33766cc5a74..b77e611cc1c 100644 --- a/lib/infer.h +++ b/lib/infer.h @@ -23,7 +23,8 @@ #include "valueflow.h" struct Interval; -template class ValuePtr; +template +class ValuePtr; struct InferModel { virtual bool match(const ValueFlow::Value& value) const = 0; @@ -32,19 +33,19 @@ struct InferModel { }; std::vector infer(const ValuePtr& model, - const std::string& op, - std::list lhsValues, - std::list rhsValues); + 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); + 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); + const std::string& op, + std::list lhsValues, + MathLib::bigint rhs); std::string toString(const Interval& i); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index fe8d51fbce1..8fc60108c0d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5229,8 +5229,7 @@ struct SimpleConditionHandler : ConditionHandler { }; struct IntegralInferModel : InferModel { - virtual bool match(const ValueFlow::Value& value) const OVERRIDE - { + virtual bool match(const ValueFlow::Value& value) const OVERRIDE { return value.isIntValue(); } virtual ValueFlow::Value yield(MathLib::bigint value) const OVERRIDE @@ -5285,8 +5284,9 @@ static void valueFlowInferCondition(TokenList* tokenlist, value.setKnown(); setTokenValue(tok, value, settings); } 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) { + std::vector result = + infer(IntegralInferModel{}, tok->str(), tok->astOperand1()->values(), tok->astOperand2()->values()); + for (const ValueFlow::Value& value : result) { setTokenValue(tok, value, settings); } } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 831e8b205e9..8ab8fe07ca8 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4004,10 +4004,10 @@ class TestCondition : public TestFixture { 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"); + " 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()); } From 8cddb0f9ae1334b57f0607c186c3e063492157a6 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 8 Oct 2021 10:25:19 -0500 Subject: [PATCH 13/13] Add missing header --- lib/infer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/infer.cpp b/lib/infer.cpp index 1b03b1639f1..a7e5a711293 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -1,6 +1,7 @@ #include "infer.h" #include "calculate.h" #include "valueptr.h" +#include #include template