walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix 11656: FP: containerOutOfBounds std::array (#5210)

This commit is contained in:
Paul Fultz II 2023-07-01 02:43:57 -05:00 committed by GitHub
parent 6d9fa6f10a
commit 59a8944e30
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 361 additions and 299 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

140
lib/programmemory.cpp
View File

@ -1185,7 +1185,14 @@ static BuiltinLibraryFunction getBuiltinLibraryFunction(const std::string& name)
return it->second; return it->second;
} }
static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const Settings* settings) struct Executor {
ProgramMemory* pm = nullptr;
const Settings* settings = nullptr;
int fdepth = 4;
explicit Executor(ProgramMemory* pm = nullptr, const Settings* settings = nullptr) : pm(pm), settings(settings) {}
ValueFlow::Value executeImpl(const Token* expr)
{ {
ValueFlow::Value unknown = ValueFlow::Value::unknown(); ValueFlow::Value unknown = ValueFlow::Value::unknown();
const ValueFlow::Value* value = nullptr; const ValueFlow::Value* value = nullptr;
@ -1214,14 +1221,14 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
const Token* containerTok = expr->tokAt(-2)->astOperand1(); const Token* containerTok = expr->tokAt(-2)->astOperand1();
const Library::Container::Yield yield = containerTok->valueType()->container->getYield(expr->strAt(-1)); const Library::Container::Yield yield = containerTok->valueType()->container->getYield(expr->strAt(-1));
if (yield == Library::Container::Yield::SIZE) { if (yield == Library::Container::Yield::SIZE) {
ValueFlow::Value v = execute(containerTok, pm); ValueFlow::Value v = execute(containerTok);
if (!v.isContainerSizeValue()) if (!v.isContainerSizeValue())
return unknown; return unknown;
v.valueType = ValueFlow::Value::ValueType::INT; v.valueType = ValueFlow::Value::ValueType::INT;
return v; return v;
} }
if (yield == Library::Container::Yield::EMPTY) { if (yield == Library::Container::Yield::EMPTY) {
ValueFlow::Value v = execute(containerTok, pm); ValueFlow::Value v = execute(containerTok);
if (!v.isContainerSizeValue()) if (!v.isContainerSizeValue())
return unknown; return unknown;
if (v.isImpossible() && v.intvalue == 0) if (v.isImpossible() && v.intvalue == 0)
@ -1229,50 +1236,52 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
if (!v.isImpossible()) if (!v.isImpossible())
return ValueFlow::Value{v.intvalue == 0}; return ValueFlow::Value{v.intvalue == 0};
} }
} else if (expr->isAssignmentOp() && expr->astOperand1() && expr->astOperand2() && expr->astOperand1()->exprId() > 0) { } else if (expr->isAssignmentOp() && expr->astOperand1() && expr->astOperand2() &&
ValueFlow::Value rhs = execute(expr->astOperand2(), pm); expr->astOperand1()->exprId() > 0) {
ValueFlow::Value rhs = execute(expr->astOperand2());
if (rhs.isUninitValue()) if (rhs.isUninitValue())
return unknown; return unknown;
if (expr->str() != "=") { if (expr->str() != "=") {
if (!pm.hasValue(expr->astOperand1()->exprId())) if (!pm->hasValue(expr->astOperand1()->exprId()))
return unknown; return unknown;
ValueFlow::Value& lhs = pm.at(expr->astOperand1()->exprId()); ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId());
rhs = evaluate(removeAssign(expr->str()), lhs, rhs); rhs = evaluate(removeAssign(expr->str()), lhs, rhs);
if (lhs.isIntValue()) if (lhs.isIntValue())
ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.intvalue), std::placeholders::_1)); ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.intvalue), std::placeholders::_1));
else if (lhs.isFloatValue()) else if (lhs.isFloatValue())
ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.floatValue), std::placeholders::_1)); ValueFlow::Value::visitValue(rhs,
std::bind(assign{}, std::ref(lhs.floatValue), std::placeholders::_1));
else else
return unknown; return unknown;
return lhs; return lhs;
} }
pm.setValue(expr->astOperand1(), rhs); pm->setValue(expr->astOperand1(), rhs);
return rhs; return rhs;
} else if (expr->str() == "&&" && expr->astOperand1() && expr->astOperand2()) { } else if (expr->str() == "&&" && expr->astOperand1() && expr->astOperand2()) {
ValueFlow::Value lhs = execute(expr->astOperand1(), pm); ValueFlow::Value lhs = execute(expr->astOperand1());
if (!lhs.isIntValue()) if (!lhs.isIntValue())
return unknown; return unknown;
if (isFalse(lhs)) if (isFalse(lhs))
return lhs; return lhs;
if (isTrue(lhs)) if (isTrue(lhs))
return execute(expr->astOperand2(), pm); return execute(expr->astOperand2());
return unknown; return unknown;
} else if (expr->str() == "||" && expr->astOperand1() && expr->astOperand2()) { } else if (expr->str() == "||" && expr->astOperand1() && expr->astOperand2()) {
ValueFlow::Value lhs = execute(expr->astOperand1(), pm); ValueFlow::Value lhs = execute(expr->astOperand1());
if (!lhs.isIntValue() || lhs.isImpossible()) if (!lhs.isIntValue() || lhs.isImpossible())
return unknown; return unknown;
if (isTrue(lhs)) if (isTrue(lhs))
return lhs; return lhs;
if (isFalse(lhs)) if (isFalse(lhs))
return execute(expr->astOperand2(), pm); return execute(expr->astOperand2());
return unknown; return unknown;
} else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) { } else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) {
execute(expr->astOperand1(), pm); execute(expr->astOperand1());
return execute(expr->astOperand2(), pm); return execute(expr->astOperand2());
} else if (expr->tokType() == Token::eIncDecOp && expr->astOperand1() && expr->astOperand1()->exprId() != 0) { } else if (expr->tokType() == Token::eIncDecOp && expr->astOperand1() && expr->astOperand1()->exprId() != 0) {
if (!pm.hasValue(expr->astOperand1()->exprId())) if (!pm->hasValue(expr->astOperand1()->exprId()))
return unknown; return unknown;
ValueFlow::Value& lhs = pm.at(expr->astOperand1()->exprId()); ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId());
if (!lhs.isIntValue()) if (!lhs.isIntValue())
return unknown; return unknown;
// overflow // overflow
@ -1286,7 +1295,7 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
return lhs; return lhs;
} else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) { } else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) {
const Token* tokvalue = nullptr; const Token* tokvalue = nullptr;
if (!pm.getTokValue(expr->astOperand1()->exprId(), &tokvalue)) { if (!pm->getTokValue(expr->astOperand1()->exprId(), &tokvalue)) {
auto tokvalue_it = std::find_if(expr->astOperand1()->values().cbegin(), auto tokvalue_it = std::find_if(expr->astOperand1()->values().cbegin(),
expr->astOperand1()->values().cend(), expr->astOperand1()->values().cend(),
std::mem_fn(&ValueFlow::Value::isTokValue)); std::mem_fn(&ValueFlow::Value::isTokValue));
@ -1299,7 +1308,7 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
return unknown; return unknown;
} }
const std::string strValue = tokvalue->strValue(); const std::string strValue = tokvalue->strValue();
ValueFlow::Value rhs = execute(expr->astOperand2(), pm); ValueFlow::Value rhs = execute(expr->astOperand2());
if (!rhs.isIntValue()) if (!rhs.isIntValue())
return unknown; return unknown;
const MathLib::bigint index = rhs.intvalue; const MathLib::bigint index = rhs.intvalue;
@ -1308,8 +1317,8 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
if (index == strValue.size()) if (index == strValue.size())
return ValueFlow::Value{}; return ValueFlow::Value{};
} else if (Token::Match(expr, "%cop%") && expr->astOperand1() && expr->astOperand2()) { } else if (Token::Match(expr, "%cop%") && expr->astOperand1() && expr->astOperand2()) {
ValueFlow::Value lhs = execute(expr->astOperand1(), pm); ValueFlow::Value lhs = execute(expr->astOperand1());
ValueFlow::Value rhs = execute(expr->astOperand2(), pm); ValueFlow::Value rhs = execute(expr->astOperand2());
ValueFlow::Value r = unknown; ValueFlow::Value r = unknown;
if (!lhs.isUninitValue() && !rhs.isUninitValue()) if (!lhs.isUninitValue() && !rhs.isUninitValue())
r = evaluate(expr->str(), lhs, rhs); r = evaluate(expr->str(), lhs, rhs);
@ -1332,7 +1341,7 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
} }
// Unary ops // Unary ops
else if (Token::Match(expr, "!|+|-") && expr->astOperand1() && !expr->astOperand2()) { else if (Token::Match(expr, "!|+|-") && expr->astOperand1() && !expr->astOperand2()) {
ValueFlow::Value lhs = execute(expr->astOperand1(), pm); ValueFlow::Value lhs = execute(expr->astOperand1());
if (!lhs.isIntValue()) if (!lhs.isIntValue())
return unknown; return unknown;
if (expr->str() == "!") { if (expr->str() == "!") {
@ -1350,23 +1359,23 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
lhs.intvalue = -lhs.intvalue; lhs.intvalue = -lhs.intvalue;
return lhs; return lhs;
} else if (expr->str() == "?" && expr->astOperand1() && expr->astOperand2()) { } else if (expr->str() == "?" && expr->astOperand1() && expr->astOperand2()) {
ValueFlow::Value cond = execute(expr->astOperand1(), pm); ValueFlow::Value cond = execute(expr->astOperand1());
if (!cond.isIntValue()) if (!cond.isIntValue())
return unknown; return unknown;
const Token* child = expr->astOperand2(); const Token* child = expr->astOperand2();
if (isFalse(cond)) if (isFalse(cond))
return execute(child->astOperand2(), pm); return execute(child->astOperand2());
if (isTrue(cond)) if (isTrue(cond))
return execute(child->astOperand1(), pm); return execute(child->astOperand1());
return unknown; return unknown;
} else if (expr->str() == "(" && expr->isCast()) { } else if (expr->str() == "(" && expr->isCast()) {
if (Token::simpleMatch(expr->previous(), ">") && expr->previous()->link()) if (Token::simpleMatch(expr->previous(), ">") && expr->previous()->link())
return execute(expr->astOperand2(), pm); return execute(expr->astOperand2());
return execute(expr->astOperand1(), pm); return execute(expr->astOperand1());
} }
if (expr->exprId() > 0 && pm.hasValue(expr->exprId())) { if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) {
ValueFlow::Value result = pm.at(expr->exprId()); ValueFlow::Value result = pm->at(expr->exprId());
if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr)) { if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr)) {
result.intvalue = !result.intvalue; result.intvalue = !result.intvalue;
result.setKnown(); result.setKnown();
@ -1377,13 +1386,31 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
if (Token::Match(expr->previous(), ">|%name% {|(")) { if (Token::Match(expr->previous(), ">|%name% {|(")) {
const Token* ftok = expr->previous(); const Token* ftok = expr->previous();
const Function* f = ftok->function(); const Function* f = ftok->function();
// TODO: Evaluate inline functions as well ValueFlow::Value result = unknown;
if (!f && settings && expr->str() == "(") { if (settings && expr->str() == "(") {
std::vector<const Token*> tokArgs = getArguments(expr); std::vector<const Token*> tokArgs = getArguments(expr);
std::vector<ValueFlow::Value> args(tokArgs.size()); std::vector<ValueFlow::Value> args(tokArgs.size());
std::transform(tokArgs.cbegin(), tokArgs.cend(), args.begin(), [&](const Token* tok) { std::transform(tokArgs.cbegin(), tokArgs.cend(), args.begin(), [&](const Token* tok) {
return execute(tok, pm, settings); return execute(tok);
}); });
if (f) {
if (fdepth >= 0) {
ProgramMemory functionState;
for (std::size_t i = 0; i < args.size(); ++i) {
const Variable* const arg = f->getArgumentVar(i);
if (!arg)
return unknown;
functionState.setValue(arg->nameToken(), args[i]);
}
Executor ex = *this;
ex.pm = &functionState;
ex.fdepth--;
auto r = ex.execute(f->functionScope);
if (!r.empty())
result = r.front();
// TODO: Track values changed by reference
}
} else {
BuiltinLibraryFunction lf = getBuiltinLibraryFunction(ftok->str()); BuiltinLibraryFunction lf = getBuiltinLibraryFunction(ftok->str());
if (lf) if (lf)
return lf(args); return lf(args);
@ -1391,18 +1418,19 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
if (!returnValue.empty()) { if (!returnValue.empty()) {
std::unordered_map<nonneg int, ValueFlow::Value> arg_map; std::unordered_map<nonneg int, ValueFlow::Value> arg_map;
int argn = 0; int argn = 0;
for (const ValueFlow::Value& result : args) { for (const ValueFlow::Value& v : args) {
if (!result.isUninitValue()) if (!v.isUninitValue())
arg_map[argn] = result; arg_map[argn] = v;
argn++; argn++;
} }
return evaluateLibraryFunction(arg_map, returnValue, settings); return evaluateLibraryFunction(arg_map, returnValue, settings);
} }
} }
}
// Check if function modifies argument // Check if function modifies argument
visitAstNodes(expr->astOperand2(), [&](const Token* child) { visitAstNodes(expr->astOperand2(), [&](const Token* child) {
if (child->exprId() > 0 && pm.hasValue(child->exprId())) { if (child->exprId() > 0 && pm->hasValue(child->exprId())) {
ValueFlow::Value& v = pm.at(child->exprId()); ValueFlow::Value& v = pm->at(child->exprId());
if (v.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) { if (v.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) {
if (ValueFlow::isContainerSizeChanged(child, v.indirect, settings)) if (ValueFlow::isContainerSizeChanged(child, v.indirect, settings))
v = unknown; v = unknown;
@ -1413,11 +1441,11 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
} }
return ChildrenToVisit::op1_and_op2; return ChildrenToVisit::op1_and_op2;
}); });
return result;
} }
return unknown; return unknown;
} }
static const ValueFlow::Value* getImpossibleValue(const Token* tok) static const ValueFlow::Value* getImpossibleValue(const Token* tok)
{ {
if (!tok) if (!tok)
@ -1430,7 +1458,8 @@ static const ValueFlow::Value* getImpossibleValue(const Token* tok)
values.push_back(std::addressof(v)); values.push_back(std::addressof(v));
} }
} }
auto it = std::max_element(values.begin(), values.end(), [](const ValueFlow::Value* x, const ValueFlow::Value* y) { auto it =
std::max_element(values.begin(), values.end(), [](const ValueFlow::Value* x, const ValueFlow::Value* y) {
return x->intvalue < y->intvalue; return x->intvalue < y->intvalue;
}); });
if (it == values.end()) if (it == values.end())
@ -1438,21 +1467,21 @@ static const ValueFlow::Value* getImpossibleValue(const Token* tok)
return *it; return *it;
} }
static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings* settings) ValueFlow::Value execute(const Token* expr)
{ {
ValueFlow::Value v = executeImpl(expr, pm, settings); ValueFlow::Value v = executeImpl(expr);
if (!v.isUninitValue()) if (!v.isUninitValue())
return v; return v;
if (!expr) if (!expr)
return v; return v;
if (expr->exprId() > 0 && pm.hasValue(expr->exprId())) if (expr->exprId() > 0 && pm->hasValue(expr->exprId()))
return pm.at(expr->exprId()); return pm->at(expr->exprId());
if (const ValueFlow::Value* value = getImpossibleValue(expr)) if (const ValueFlow::Value* value = getImpossibleValue(expr))
return *value; return *value;
return v; return v;
} }
std::vector<ValueFlow::Value> execute(const Scope* scope, ProgramMemory& pm, const Settings* settings) std::vector<ValueFlow::Value> execute(const Scope* scope)
{ {
static const std::vector<ValueFlow::Value> unknown = {ValueFlow::Value::unknown()}; static const std::vector<ValueFlow::Value> unknown = {ValueFlow::Value::unknown()};
if (!scope) if (!scope)
@ -1465,10 +1494,10 @@ std::vector<ValueFlow::Value> execute(const Scope* scope, ProgramMemory& pm, con
return unknown; return unknown;
if (Token::simpleMatch(top, "return") && top->astOperand1()) if (Token::simpleMatch(top, "return") && top->astOperand1())
return {execute(top->astOperand1(), pm, settings)}; return {execute(top->astOperand1())};
if (Token::Match(top, "%op%")) { if (Token::Match(top, "%op%")) {
if (execute(top, pm, settings).isUninitValue()) if (execute(top).isUninitValue())
return unknown; return unknown;
const Token* next = nextAfterAstRightmostLeaf(top); const Token* next = nextAfterAstRightmostLeaf(top);
if (!next) if (!next)
@ -1476,7 +1505,7 @@ std::vector<ValueFlow::Value> execute(const Scope* scope, ProgramMemory& pm, con
tok = next; tok = next;
} else if (Token::simpleMatch(top->previous(), "if (")) { } else if (Token::simpleMatch(top->previous(), "if (")) {
const Token* condTok = top->astOperand2(); const Token* condTok = top->astOperand2();
ValueFlow::Value v = execute(condTok, pm, settings); ValueFlow::Value v = execute(condTok);
if (!v.isIntValue()) if (!v.isIntValue())
return unknown; return unknown;
const Token* thenStart = top->link()->next(); const Token* thenStart = top->link()->next();
@ -1488,10 +1517,10 @@ std::vector<ValueFlow::Value> execute(const Scope* scope, ProgramMemory& pm, con
} }
std::vector<ValueFlow::Value> result; std::vector<ValueFlow::Value> result;
if (isTrue(v)) { if (isTrue(v)) {
result = execute(thenStart->scope(), pm, settings); result = execute(thenStart->scope());
} else if (isFalse(v)) { } else if (isFalse(v)) {
if (elseStart) if (elseStart)
result = execute(elseStart->scope(), pm, settings); result = execute(elseStart->scope());
} else { } else {
return unknown; return unknown;
} }
@ -1504,6 +1533,19 @@ std::vector<ValueFlow::Value> execute(const Scope* scope, ProgramMemory& pm, con
} }
return {}; return {};
} }
};
static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings* settings)
{
Executor ex{&pm, settings};
return ex.execute(expr);
}
std::vector<ValueFlow::Value> execute(const Scope* scope, ProgramMemory& pm, const Settings* settings)
{
Executor ex{&pm, settings};
return ex.execute(scope);
}
ValueFlow::Value evaluateLibraryFunction(const std::unordered_map<nonneg int, ValueFlow::Value>& args, ValueFlow::Value evaluateLibraryFunction(const std::unordered_map<nonneg int, ValueFlow::Value>& args,
const std::string& returnValue, const std::string& returnValue,

20
test/teststl.cpp
View File

@ -888,6 +888,26 @@ private:
" auto x = v->back();\n" " auto x = v->back();\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
checkNormal("template <typename T, uint8_t count>\n"
"struct Foo {\n"
" std::array<T, count> items = {0};\n"
" T maxCount = count;\n"
" explicit Foo(const T& maxValue = (std::numeric_limits<T>::max)()) : maxCount(maxValue) {}\n"
" bool Set(const uint8_t idx) {\n"
" if (CheckBounds(idx) && items[idx] < maxCount) {\n"
" items[idx] += 1;\n"
" return true;\n"
" }\n"
" return false;\n"
" }\n"
" static bool CheckBounds(const uint8_t idx) { return idx < count; }\n"
"};\n"
"void f() {\n"
" Foo<uint8_t, 42U> x;\n"
" if (x.Set(42U)) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
} }
void outOfBoundsSymbolic() void outOfBoundsSymbolic()