From f9516cf1c6c5349a4a5427468f1db0d8ddaa9450 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 24 Jul 2021 15:44:18 -0500 Subject: [PATCH] Fix issue 10378: FP derefInvalidIteratorRedundantCheck (#3353) --- lib/programmemory.cpp | 82 ++++++++++++++++++++++++++++++++++--------- lib/programmemory.h | 15 +++++--- lib/token.cpp | 18 ++++++++++ lib/token.h | 2 ++ test/teststl.cpp | 16 +++++++++ 5 files changed, 111 insertions(+), 22 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index e5990c5c3..fbe92a27e 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -7,6 +7,7 @@ #include "valueflow.h" #include #include +#include #include #include @@ -458,14 +459,53 @@ ProgramMemory getProgramMemory(const Token* tok, nonneg int exprid, const ValueF return programMemory; } -void execute(const Token *expr, - ProgramMemory * const programMemory, - MathLib::bigint *result, - bool *error) +static PMEvaluateFunction evaluateAsInt(PMEvaluateFunction f, ValueFlow::Value::ValueType t) +{ + return [=](const Token* expr, ProgramMemory* const programMemory, MathLib::bigint* result) -> bool { + const ValueFlow::Value* value = expr->getKnownValue(t); + if (!value) + value = programMemory->getValue(expr->exprId()); + if (value && value->valueType == t) { + *result = value->intvalue; + return true; + } + return f && f(expr, programMemory, result); + }; +} + +static std::set findIteratorTypes(const ProgramMemory& pm) +{ + std::set result; + for (auto&& p : pm.values) { + if (p.second.isIteratorValue()) + result.insert(p.second.valueType); + if (result.size() == 2) + break; + } + return result; +} + +static bool isIterator(const Token* expr) +{ + if (!expr) + return false; + if (astIsIterator(expr)) + return true; + return std::any_of(expr->values().begin(), expr->values().end(), std::mem_fn(&ValueFlow::Value::isIteratorValue)); +} + +void execute(const Token* expr, + ProgramMemory* const programMemory, + MathLib::bigint* result, + bool* error, + const PMEvaluateFunction& f) { if (!expr) *error = true; + else if (f && f(expr, programMemory, result)) + *error = false; + else if (expr->hasKnownIntValue() && !expr->isAssignmentOp()) { *result = expr->values().front().intvalue; } @@ -504,8 +544,16 @@ void execute(const Token *expr, MathLib::bigint result1(0), result2(0); bool error1 = false; bool error2 = false; - execute(expr->astOperand1(), programMemory, &result1, &error1); - execute(expr->astOperand2(), programMemory, &result2, &error2); + execute(expr->astOperand1(), programMemory, &result1, &error1, f); + execute(expr->astOperand2(), programMemory, &result2, &error2, f); + if (error1 && error2 && (isIterator(expr->astOperand1()) || isIterator(expr->astOperand2()))) { + for (ValueFlow::Value::ValueType t : findIteratorTypes(*programMemory)) { + execute(expr->astOperand1(), programMemory, &result1, &error1, evaluateAsInt(f, t)); + execute(expr->astOperand2(), programMemory, &result2, &error2, evaluateAsInt(f, t)); + if (!error1 && !error2) + break; + } + } if (error1 && error2) { *error = true; } else if (error1 && !error2) { @@ -533,7 +581,7 @@ void execute(const Token *expr, } else if (expr->isAssignmentOp()) { - execute(expr->astOperand2(), programMemory, result, error); + execute(expr->astOperand2(), programMemory, result, error, f); if (!expr->astOperand1() || !expr->astOperand1()->exprId()) *error = true; if (*error) @@ -588,8 +636,8 @@ void execute(const Token *expr, else if (expr->isArithmeticalOp() && expr->astOperand1() && expr->astOperand2()) { MathLib::bigint result1(0), result2(0); - execute(expr->astOperand1(), programMemory, &result1, error); - execute(expr->astOperand2(), programMemory, &result2, error); + execute(expr->astOperand1(), programMemory, &result1, error, f); + execute(expr->astOperand2(), programMemory, &result2, error, f); if (expr->str() == "+") *result = result1 + result2; else if (expr->str() == "-") @@ -622,33 +670,33 @@ void execute(const Token *expr, else if (expr->str() == "&&") { bool error1 = false; - execute(expr->astOperand1(), programMemory, result, &error1); + execute(expr->astOperand1(), programMemory, result, &error1, f); if (!error1 && *result == 0) *result = 0; else { bool error2 = false; - execute(expr->astOperand2(), programMemory, result, &error2); + execute(expr->astOperand2(), programMemory, result, &error2, f); if (error1 || error2) *error = true; } } else if (expr->str() == "||") { - execute(expr->astOperand1(), programMemory, result, error); + execute(expr->astOperand1(), programMemory, result, error, f); if (*result == 1 && *error == false) *result = 1; else if (*result == 0 && *error == false) - execute(expr->astOperand2(), programMemory, result, error); + execute(expr->astOperand2(), programMemory, result, error, f); } else if (expr->str() == "!") { - execute(expr->astOperand1(), programMemory, result, error); + execute(expr->astOperand1(), programMemory, result, error, f); *result = !(*result); } else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) { - execute(expr->astOperand1(), programMemory, result, error); - execute(expr->astOperand2(), programMemory, result, error); + execute(expr->astOperand1(), programMemory, result, error, f); + execute(expr->astOperand2(), programMemory, result, error, f); } else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) { @@ -669,7 +717,7 @@ void execute(const Token *expr, } const std::string strValue = tokvalue->strValue(); MathLib::bigint index = 0; - execute(expr->astOperand2(), programMemory, &index, error); + execute(expr->astOperand2(), programMemory, &index, error, f); if (index >= 0 && index < strValue.size()) *result = strValue[index]; else if (index == strValue.size()) diff --git a/lib/programmemory.h b/lib/programmemory.h index ba8a748d7..5d18c59f1 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -1,9 +1,10 @@ #ifndef GUARD_PROGRAMMEMORY_H #define GUARD_PROGRAMMEMORY_H +#include "mathlib.h" #include "utils.h" #include "valueflow.h" // needed for alias -#include "mathlib.h" +#include #include #include @@ -57,10 +58,14 @@ struct ProgramMemoryState { ProgramMemory get(const Token* tok, const Token* ctx, const ProgramMemory::Map& vars) const; }; -void execute(const Token *expr, - ProgramMemory * const programMemory, - MathLib::bigint *result, - bool *error); +using PMEvaluateFunction = + std::function; + +void execute(const Token* expr, + ProgramMemory* const programMemory, + MathLib::bigint* result, + bool* error, + const PMEvaluateFunction& f = nullptr); /** * Is condition always false when variable has given value? diff --git a/lib/token.cpp b/lib/token.cpp index 114af190e..ed81bc233 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -2385,6 +2385,24 @@ bool Token::hasKnownValue() const return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown)); } +bool Token::hasKnownValue(ValueFlow::Value::ValueType t) const +{ + return mImpl->mValues && + std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), [&](const ValueFlow::Value& value) { + return value.isKnown() && value.valueType == t; + }); +} + +const ValueFlow::Value* Token::getKnownValue(ValueFlow::Value::ValueType t) const +{ + if (!mImpl->mValues) + return nullptr; + auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [&](const ValueFlow::Value& value) { + return value.isKnown() && value.valueType == t; + }); + return it == mImpl->mValues->end() ? nullptr : &*it; +} + bool Token::isImpossibleIntValue(const MathLib::bigint val) const { if (!mImpl->mValues) diff --git a/lib/token.h b/lib/token.h index 6667cb133..a160c2688 100644 --- a/lib/token.h +++ b/lib/token.h @@ -1133,7 +1133,9 @@ public: bool hasKnownIntValue() const; bool hasKnownValue() const; + bool hasKnownValue(ValueFlow::Value::ValueType t) const; + const ValueFlow::Value* getKnownValue(ValueFlow::Value::ValueType t) const; MathLib::bigint getKnownIntValue() const { return mImpl->mValues->front().intvalue; } diff --git a/test/teststl.cpp b/test/teststl.cpp index 5b27b6764..ee5983cca 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -70,6 +70,7 @@ private: TEST_CASE(iterator24); TEST_CASE(iterator25); // #9742 TEST_CASE(iterator26); // #9176 + TEST_CASE(iterator27); // #10378 TEST_CASE(iteratorExpression); TEST_CASE(iteratorSameExpression); TEST_CASE(mismatchingContainerIterator); @@ -1392,6 +1393,21 @@ private: ASSERT_EQUALS("", errout.str()); } + void iterator27() + { // #10378 + check("struct A {\n" + " int a;\n" + " int b;\n" + "};\n" + "int f(std::map m) {\n" + " auto it = m.find( 1 );\n" + " const int a( it == m.cend() ? 0 : it->second.a );\n" + " const int b( it == m.cend() ? 0 : it->second.b );\n" + " return a + b;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void iteratorExpression() { check("std::vector& f();\n" "std::vector& g();\n"