Evaluate container sizes in forward analysis (#3338)

This commit is contained in:
Paul Fultz II 2021-07-16 11:49:07 -05:00 committed by GitHub
parent f5fac96670
commit 942202aede
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 138 additions and 29 deletions

View File

@ -128,15 +128,22 @@ struct Analyzer {
None = 0, None = 0,
Quiet = (1 << 0), Quiet = (1 << 0),
Absolute = (1 << 1), Absolute = (1 << 1),
ContainerEmpty = (1 << 2),
}; };
}; };
enum class Evaluate { Integral, ContainerEmpty };
/// Analyze a token /// Analyze a token
virtual Action analyze(const Token* tok, Direction d) const = 0; virtual Action analyze(const Token* tok, Direction d) const = 0;
/// Update the state of the value /// Update the state of the value
virtual void update(Token* tok, Action a, Direction d) = 0; virtual void update(Token* tok, Action a, Direction d) = 0;
/// Try to evaluate the value of a token(most likely a condition) /// Try to evaluate the value of a token(most likely a condition)
virtual std::vector<int> evaluate(const Token* tok, const Token* ctx = nullptr) const = 0; virtual std::vector<int> evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const = 0;
std::vector<int> evaluate(const Token* tok, const Token* ctx = nullptr) const
{
return evaluate(Evaluate::Integral, tok, ctx);
}
/// Lower any values to possible /// Lower any values to possible
virtual bool lowerToPossible() = 0; virtual bool lowerToPossible() = 0;
/// Lower any values to inconclusive /// Lower any values to inconclusive

View File

@ -565,7 +565,13 @@ struct ForwardTraversal {
Token* conTok = condTok->astOperand2(); Token* conTok = condTok->astOperand2();
if (conTok && updateRecursive(conTok) == Progress::Break) if (conTok && updateRecursive(conTok) == Progress::Break)
return Break(); return Break();
if (updateLoop(end, endBlock, condTok) == Progress::Break) bool isEmpty = false;
std::vector<int> result = analyzer->evaluate(Analyzer::Evaluate::ContainerEmpty, conTok);
if (result.empty())
analyzer->assume(conTok, false, Analyzer::Assume::ContainerEmpty);
else
isEmpty = result.front() != 0;
if (!isEmpty && updateLoop(end, endBlock, condTok) == Progress::Break)
return Break(); return Break();
} else { } else {
Token* stepTok = getStepTok(tok); Token* stepTok = getStepTok(tok);

View File

@ -74,6 +74,15 @@ bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint* r
return false; return false;
} }
void ProgramMemory::setContainerSizeValue(nonneg int exprid, MathLib::bigint value, bool isEqual)
{
ValueFlow::Value v(value);
v.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
if (!isEqual)
v.valueKind = ValueFlow::Value::ValueKind::Impossible;
values[exprid] = v;
}
void ProgramMemory::setUnknown(nonneg int exprid) void ProgramMemory::setUnknown(nonneg int exprid)
{ {
values[exprid].valueType = ValueFlow::Value::ValueType::UNINIT; values[exprid].valueType = ValueFlow::Value::ValueType::UNINIT;
@ -142,6 +151,36 @@ bool conditionIsTrue(const Token *condition, const ProgramMemory &programMemory)
return !error && result == 1; return !error && result == 1;
} }
static const Token* getContainerFromEmpty(const Token* tok)
{
if (!Token::Match(tok->tokAt(-2), ". %name% ("))
return nullptr;
const Token* containerTok = tok->tokAt(-2)->astOperand1();
if (!astIsContainer(containerTok))
return nullptr;
if (containerTok->valueType()->container &&
containerTok->valueType()->container->getYield(tok->strAt(-1)) == Library::Container::Yield::EMPTY)
return containerTok;
if (Token::simpleMatch(tok->tokAt(-1), "empty ( )"))
return containerTok;
return nullptr;
}
static const Token* getContainerFromSize(const Token* tok)
{
if (!Token::Match(tok->tokAt(-2), ". %name% ("))
return nullptr;
const Token* containerTok = tok->tokAt(-2)->astOperand1();
if (!astIsContainer(containerTok))
return nullptr;
if (containerTok->valueType()->container &&
containerTok->valueType()->container->getYield(tok->strAt(-1)) == Library::Container::Yield::SIZE)
return containerTok;
if (Token::Match(tok->tokAt(-1), "size|length ( )"))
return containerTok;
return nullptr;
}
void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings* settings, bool then) void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings* settings, bool then)
{ {
if (Token::Match(tok, "==|>=|<=|<|>|!=")) { if (Token::Match(tok, "==|>=|<=|<|>|!=")) {
@ -169,7 +208,12 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke
return; return;
if (endTok && isExpressionChanged(vartok, tok->next(), endTok, settings, true)) if (endTok && isExpressionChanged(vartok, tok->next(), endTok, settings, true))
return; return;
pm.setIntValue(vartok->exprId(), then ? truevalue.intvalue : falsevalue.intvalue); bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then);
if (!impossible)
pm.setIntValue(vartok->exprId(), then ? truevalue.intvalue : falsevalue.intvalue);
const Token* containerTok = getContainerFromSize(vartok);
if (containerTok)
pm.setContainerSizeValue(containerTok->exprId(), then ? truevalue.intvalue : falsevalue.intvalue, !impossible);
} else if (Token::simpleMatch(tok, "!")) { } else if (Token::simpleMatch(tok, "!")) {
programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then); programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then);
} else if (then && Token::simpleMatch(tok, "&&")) { } else if (then && Token::simpleMatch(tok, "&&")) {
@ -184,6 +228,9 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke
if (endTok && isExpressionChanged(tok, tok->next(), endTok, settings, true)) if (endTok && isExpressionChanged(tok, tok->next(), endTok, settings, true))
return; return;
pm.setIntValue(tok->exprId(), then); pm.setIntValue(tok->exprId(), then);
const Token* containerTok = getContainerFromEmpty(tok);
if (containerTok)
pm.setContainerSizeValue(containerTok->exprId(), 0, then);
} }
} }
@ -331,10 +378,13 @@ void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& va
replace(pm, tok); replace(pm, tok);
} }
void ProgramMemoryState::assume(const Token* tok, bool b) void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty)
{ {
ProgramMemory pm = state; ProgramMemory pm = state;
programMemoryParseCondition(pm, tok, nullptr, nullptr, b); if (isEmpty)
pm.setContainerSizeValue(tok->exprId(), 0, b);
else
programMemoryParseCondition(pm, tok, nullptr, nullptr, b);
const Token* origin = tok; const Token* origin = tok;
const Token* top = tok->astTop(); const Token* top = tok->astTop();
if (top && Token::Match(top->previous(), "for|while (")) if (top && Token::Match(top->previous(), "for|while ("))

View File

@ -21,6 +21,7 @@ struct ProgramMemory {
bool getContainerSizeValue(nonneg int exprid, MathLib::bigint* result) const; bool getContainerSizeValue(nonneg int exprid, MathLib::bigint* result) const;
bool getContainerEmptyValue(nonneg int exprid, MathLib::bigint* result) const; bool getContainerEmptyValue(nonneg int exprid, MathLib::bigint* result) const;
void setContainerSizeValue(nonneg int exprid, MathLib::bigint value, bool isEqual = true);
void setUnknown(nonneg int exprid); void setUnknown(nonneg int exprid);
@ -49,7 +50,7 @@ struct ProgramMemoryState {
void addState(const Token* tok, const ProgramMemory::Map& vars); void addState(const Token* tok, const ProgramMemory::Map& vars);
void assume(const Token* tok, bool b); void assume(const Token* tok, bool b, bool isEmpty = false);
void removeModifiedVars(const Token* tok); void removeModifiedVars(const Token* tok);

View File

@ -1455,7 +1455,7 @@ void SymbolDatabase::createSymbolDatabaseExprIds()
continue; continue;
if (tok1->exprId() == tok2->exprId()) if (tok1->exprId() == tok2->exprId())
continue; continue;
if (!isSameExpression(isCPP(), true, tok1, tok2, mSettings->library, true, false)) if (!isSameExpression(isCPP(), true, tok1, tok2, mSettings->library, false, false))
continue; continue;
nonneg int cid = std::min(tok1->exprId(), tok2->exprId()); nonneg int cid = std::min(tok1->exprId(), tok2->exprId());
tok1->exprId(cid); tok1->exprId(cid);

View File

@ -2000,7 +2000,7 @@ struct ValueFlowAnalyzer : Analyzer {
// Check if its assigned to the same value // Check if its assigned to the same value
if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) && if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) &&
astIsIntegral(tok->astParent()->astOperand2(), false)) { astIsIntegral(tok->astParent()->astOperand2(), false)) {
std::vector<int> result = evaluate(tok->astParent()->astOperand2()); std::vector<int> result = evaluate(Evaluate::Integral, tok->astParent()->astOperand2());
if (!result.empty() && value->equalTo(result.front())) if (!result.empty() && value->equalTo(result.front()))
return Action::Idempotent; return Action::Idempotent;
} }
@ -2177,32 +2177,48 @@ struct ValueFlowAnalyzer : Analyzer {
return Action::None; return Action::None;
} }
virtual std::vector<int> evaluate(const Token* tok, const Token* ctx = nullptr) const OVERRIDE { virtual std::vector<int> evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const OVERRIDE
if (tok->hasKnownIntValue()) {
return {static_cast<int>(tok->values().front().intvalue)}; if (e == Evaluate::Integral) {
std::vector<int> result; if (tok->hasKnownIntValue())
ProgramMemory pm = pms.get(tok, ctx, getProgramState()); return {static_cast<int>(tok->values().front().intvalue)};
if (Token::Match(tok, "&&|%oror%")) { std::vector<int> result;
if (conditionIsTrue(tok, pm)) ProgramMemory pm = pms.get(tok, ctx, getProgramState());
result.push_back(1); if (Token::Match(tok, "&&|%oror%")) {
if (conditionIsFalse(tok, pm)) if (conditionIsTrue(tok, pm))
result.push_back(0); result.push_back(1);
} else { if (conditionIsFalse(tok, pm))
MathLib::bigint out = 0; result.push_back(0);
bool error = false; } else {
execute(tok, &pm, &out, &error); MathLib::bigint out = 0;
if (!error) bool error = false;
result.push_back(out); execute(tok, &pm, &out, &error);
} if (!error)
result.push_back(out);
}
return result; return result;
} else if (e == Evaluate::ContainerEmpty) {
const ValueFlow::Value* value = ValueFlow::findValue(tok->values(), nullptr, [](const ValueFlow::Value& v) {
return v.isKnown() && v.isContainerSizeValue();
});
if (value)
return {value->intvalue == 0};
ProgramMemory pm = pms.get(tok, ctx, getProgramState());
MathLib::bigint out = 0;
if (pm.getContainerEmptyValue(tok->exprId(), &out))
return {static_cast<int>(out)};
return {};
} else {
return {};
}
} }
virtual void assume(const Token* tok, bool state, unsigned int flags) OVERRIDE { virtual void assume(const Token* tok, bool state, unsigned int flags) OVERRIDE {
// Update program state // Update program state
pms.removeModifiedVars(tok); pms.removeModifiedVars(tok);
pms.addState(tok, getProgramState()); pms.addState(tok, getProgramState());
pms.assume(tok, state); pms.assume(tok, state, flags & Assume::ContainerEmpty);
bool isCondBlock = false; bool isCondBlock = false;
const Token* parent = tok->astParent(); const Token* parent = tok->astParent();
@ -2221,8 +2237,13 @@ struct ValueFlowAnalyzer : Analyzer {
} }
if (!(flags & Assume::Quiet)) { if (!(flags & Assume::Quiet)) {
std::string s = state ? "true" : "false"; if (flags & Assume::ContainerEmpty) {
addErrorPath(tok, "Assuming condition is " + s); std::string s = state ? "empty" : "not empty";
addErrorPath(tok, "Assuming container is " + s);
} else {
std::string s = state ? "true" : "false";
addErrorPath(tok, "Assuming condition is " + s);
}
} }
if (!(flags & Assume::Absolute)) if (!(flags & Assume::Absolute))
makeConditional(); makeConditional();

View File

@ -538,6 +538,30 @@ private:
" return Name;\n" " return Name;\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
checkNormal("bool f(bool b) {\n"
" std::vector<int> v;\n"
" if (b)\n"
" v.push_back(0);\n"
" for(auto i:v)\n"
" if (v[i] > 0)\n"
" return true;\n"
" return false;\n"
"}\n");
ASSERT_EQUALS("test.cpp:6:style:Consider using std::any_of algorithm instead of a raw loop.\n", errout.str());
checkNormal("std::vector<int> range(int n);\n"
"bool f(bool b) {\n"
" std::vector<int> v;\n"
" if (b)\n"
" v.push_back(1);\n"
" assert(range(v.size()).size() == v.size());\n"
" for(auto i:range(v.size()))\n"
" if (v[i] > 0)\n"
" return true;\n"
" return false;\n"
"}\n");
ASSERT_EQUALS("test.cpp:8:style:Consider using std::any_of algorithm instead of a raw loop.\n", errout.str());
} }
void outOfBoundsIndexExpression() { void outOfBoundsIndexExpression() {