Fix issue 10378: FP derefInvalidIteratorRedundantCheck (#3353)

This commit is contained in:
Paul Fultz II 2021-07-24 15:44:18 -05:00 committed by GitHub
parent e08ee3bac7
commit f9516cf1c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 111 additions and 22 deletions

View File

@ -7,6 +7,7 @@
#include "valueflow.h" #include "valueflow.h"
#include <algorithm> #include <algorithm>
#include <cassert> #include <cassert>
#include <functional>
#include <limits> #include <limits>
#include <memory> #include <memory>
@ -458,14 +459,53 @@ ProgramMemory getProgramMemory(const Token* tok, nonneg int exprid, const ValueF
return programMemory; return programMemory;
} }
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<ValueFlow::Value::ValueType> findIteratorTypes(const ProgramMemory& pm)
{
std::set<ValueFlow::Value::ValueType> 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, void execute(const Token* expr,
ProgramMemory* const programMemory, ProgramMemory* const programMemory,
MathLib::bigint* result, MathLib::bigint* result,
bool *error) bool* error,
const PMEvaluateFunction& f)
{ {
if (!expr) if (!expr)
*error = true; *error = true;
else if (f && f(expr, programMemory, result))
*error = false;
else if (expr->hasKnownIntValue() && !expr->isAssignmentOp()) { else if (expr->hasKnownIntValue() && !expr->isAssignmentOp()) {
*result = expr->values().front().intvalue; *result = expr->values().front().intvalue;
} }
@ -504,8 +544,16 @@ void execute(const Token *expr,
MathLib::bigint result1(0), result2(0); MathLib::bigint result1(0), result2(0);
bool error1 = false; bool error1 = false;
bool error2 = false; bool error2 = false;
execute(expr->astOperand1(), programMemory, &result1, &error1); execute(expr->astOperand1(), programMemory, &result1, &error1, f);
execute(expr->astOperand2(), programMemory, &result2, &error2); 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) { if (error1 && error2) {
*error = true; *error = true;
} else if (error1 && !error2) { } else if (error1 && !error2) {
@ -533,7 +581,7 @@ void execute(const Token *expr,
} }
else if (expr->isAssignmentOp()) { else if (expr->isAssignmentOp()) {
execute(expr->astOperand2(), programMemory, result, error); execute(expr->astOperand2(), programMemory, result, error, f);
if (!expr->astOperand1() || !expr->astOperand1()->exprId()) if (!expr->astOperand1() || !expr->astOperand1()->exprId())
*error = true; *error = true;
if (*error) if (*error)
@ -588,8 +636,8 @@ void execute(const Token *expr,
else if (expr->isArithmeticalOp() && expr->astOperand1() && expr->astOperand2()) { else if (expr->isArithmeticalOp() && expr->astOperand1() && expr->astOperand2()) {
MathLib::bigint result1(0), result2(0); MathLib::bigint result1(0), result2(0);
execute(expr->astOperand1(), programMemory, &result1, error); execute(expr->astOperand1(), programMemory, &result1, error, f);
execute(expr->astOperand2(), programMemory, &result2, error); execute(expr->astOperand2(), programMemory, &result2, error, f);
if (expr->str() == "+") if (expr->str() == "+")
*result = result1 + result2; *result = result1 + result2;
else if (expr->str() == "-") else if (expr->str() == "-")
@ -622,33 +670,33 @@ void execute(const Token *expr,
else if (expr->str() == "&&") { else if (expr->str() == "&&") {
bool error1 = false; bool error1 = false;
execute(expr->astOperand1(), programMemory, result, &error1); execute(expr->astOperand1(), programMemory, result, &error1, f);
if (!error1 && *result == 0) if (!error1 && *result == 0)
*result = 0; *result = 0;
else { else {
bool error2 = false; bool error2 = false;
execute(expr->astOperand2(), programMemory, result, &error2); execute(expr->astOperand2(), programMemory, result, &error2, f);
if (error1 || error2) if (error1 || error2)
*error = true; *error = true;
} }
} }
else if (expr->str() == "||") { else if (expr->str() == "||") {
execute(expr->astOperand1(), programMemory, result, error); execute(expr->astOperand1(), programMemory, result, error, f);
if (*result == 1 && *error == false) if (*result == 1 && *error == false)
*result = 1; *result = 1;
else if (*result == 0 && *error == false) else if (*result == 0 && *error == false)
execute(expr->astOperand2(), programMemory, result, error); execute(expr->astOperand2(), programMemory, result, error, f);
} }
else if (expr->str() == "!") { else if (expr->str() == "!") {
execute(expr->astOperand1(), programMemory, result, error); execute(expr->astOperand1(), programMemory, result, error, f);
*result = !(*result); *result = !(*result);
} }
else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) { else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) {
execute(expr->astOperand1(), programMemory, result, error); execute(expr->astOperand1(), programMemory, result, error, f);
execute(expr->astOperand2(), programMemory, result, error); execute(expr->astOperand2(), programMemory, result, error, f);
} }
else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) { else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) {
@ -669,7 +717,7 @@ void execute(const Token *expr,
} }
const std::string strValue = tokvalue->strValue(); const std::string strValue = tokvalue->strValue();
MathLib::bigint index = 0; MathLib::bigint index = 0;
execute(expr->astOperand2(), programMemory, &index, error); execute(expr->astOperand2(), programMemory, &index, error, f);
if (index >= 0 && index < strValue.size()) if (index >= 0 && index < strValue.size())
*result = strValue[index]; *result = strValue[index];
else if (index == strValue.size()) else if (index == strValue.size())

View File

@ -1,9 +1,10 @@
#ifndef GUARD_PROGRAMMEMORY_H #ifndef GUARD_PROGRAMMEMORY_H
#define GUARD_PROGRAMMEMORY_H #define GUARD_PROGRAMMEMORY_H
#include "mathlib.h"
#include "utils.h" #include "utils.h"
#include "valueflow.h" // needed for alias #include "valueflow.h" // needed for alias
#include "mathlib.h" #include <functional>
#include <map> #include <map>
#include <unordered_map> #include <unordered_map>
@ -57,10 +58,14 @@ struct ProgramMemoryState {
ProgramMemory get(const Token* tok, const Token* ctx, const ProgramMemory::Map& vars) const; ProgramMemory get(const Token* tok, const Token* ctx, const ProgramMemory::Map& vars) const;
}; };
using PMEvaluateFunction =
std::function<bool(const Token* expr, ProgramMemory* const programMemory, MathLib::bigint* result)>;
void execute(const Token* expr, void execute(const Token* expr,
ProgramMemory* const programMemory, ProgramMemory* const programMemory,
MathLib::bigint* result, MathLib::bigint* result,
bool *error); bool* error,
const PMEvaluateFunction& f = nullptr);
/** /**
* Is condition always false when variable has given value? * Is condition always false when variable has given value?

View File

@ -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)); 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 bool Token::isImpossibleIntValue(const MathLib::bigint val) const
{ {
if (!mImpl->mValues) if (!mImpl->mValues)

View File

@ -1133,7 +1133,9 @@ public:
bool hasKnownIntValue() const; bool hasKnownIntValue() const;
bool hasKnownValue() const; bool hasKnownValue() const;
bool hasKnownValue(ValueFlow::Value::ValueType t) const;
const ValueFlow::Value* getKnownValue(ValueFlow::Value::ValueType t) const;
MathLib::bigint getKnownIntValue() const { MathLib::bigint getKnownIntValue() const {
return mImpl->mValues->front().intvalue; return mImpl->mValues->front().intvalue;
} }

View File

@ -70,6 +70,7 @@ private:
TEST_CASE(iterator24); TEST_CASE(iterator24);
TEST_CASE(iterator25); // #9742 TEST_CASE(iterator25); // #9742
TEST_CASE(iterator26); // #9176 TEST_CASE(iterator26); // #9176
TEST_CASE(iterator27); // #10378
TEST_CASE(iteratorExpression); TEST_CASE(iteratorExpression);
TEST_CASE(iteratorSameExpression); TEST_CASE(iteratorSameExpression);
TEST_CASE(mismatchingContainerIterator); TEST_CASE(mismatchingContainerIterator);
@ -1392,6 +1393,21 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void iterator27()
{ // #10378
check("struct A {\n"
" int a;\n"
" int b;\n"
"};\n"
"int f(std::map<int, A> 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() { void iteratorExpression() {
check("std::vector<int>& f();\n" check("std::vector<int>& f();\n"
"std::vector<int>& g();\n" "std::vector<int>& g();\n"