Fix 10821: FN: containerOutOfBounds (#3856)

* Fix 10821: FN: containerOutOfBounds

* Format

* Fix cppcheck warning

* Add valueflow tests

* Format

* Fix some bugs

* Format
This commit is contained in:
Paul Fultz II 2022-02-24 22:53:51 -06:00 committed by GitHub
parent 03deb4d31e
commit dbc80787e1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 88 additions and 17 deletions

View File

@ -5298,7 +5298,7 @@ struct ConditionHandler {
if (!top) if (!top)
continue; continue;
if (!Token::Match(top->previous(), "if|while|for (") && !Token::Match(tok->astParent(), "&&|%oror%|?")) if (!Token::Match(top->previous(), "if|while|for (") && !Token::Match(tok->astParent(), "&&|%oror%|?|!"))
continue; continue;
for (const Condition& cond : parse(tok, tokenlist->getSettings())) { for (const Condition& cond : parse(tok, tokenlist->getSettings())) {
if (!cond.vartok) if (!cond.vartok)
@ -5430,8 +5430,6 @@ struct ConditionHandler {
ErrorLogger* errorLogger, ErrorLogger* errorLogger,
const Settings* settings) const { const Settings* settings) const {
traverseCondition(tokenlist, symboldatabase, [&](const Condition& cond, Token* tok, const Scope* scope) { traverseCondition(tokenlist, symboldatabase, [&](const Condition& cond, Token* tok, const Scope* scope) {
if (Token::simpleMatch(tok->astParent(), "?"))
return;
const Token* top = tok->astTop(); const Token* top = tok->astTop();
std::list<ValueFlow::Value> thenValues; std::list<ValueFlow::Value> thenValues;
@ -5526,6 +5524,31 @@ struct ConditionHandler {
} }
} }
// if astParent is "!" we need to invert codeblock
{
const Token* tok2 = tok;
while (tok2 && tok2->astParent() && tok2->astParent()->str() != "?") {
const Token* parent = tok2->astParent();
while (parent && parent->str() == "&&")
parent = parent->astParent();
if (parent && (parent->str() == "!" || Token::simpleMatch(parent, "== false")))
std::swap(thenValues, elseValues);
tok2 = parent;
}
}
Token* condTop = tok;
while (Token::Match(condTop->astParent(), "%oror%|&&|!"))
condTop = condTop->astParent();
if (Token::simpleMatch(condTop->astParent(), "?")) {
Token* colon = condTop->astParent()->astOperand2();
forward(colon->astOperand1(), cond.vartok, thenValues, tokenlist, settings);
forward(colon->astOperand2(), cond.vartok, elseValues, tokenlist, settings);
// TODO: Handle after condition
return;
}
if (!Token::Match(top->previous(), "if|while|for (")) if (!Token::Match(top->previous(), "if|while|for ("))
return; return;
@ -5583,19 +5606,6 @@ struct ConditionHandler {
} }
} }
// if astParent is "!" we need to invert codeblock
{
const Token* tok2 = tok;
while (tok2->astParent()) {
const Token* parent = tok2->astParent();
while (parent && parent->str() == "&&")
parent = parent->astParent();
if (parent && (parent->str() == "!" || Token::simpleMatch(parent, "== false")))
std::swap(thenValues, elseValues);
tok2 = parent;
}
}
bool deadBranch[] = {false, false}; bool deadBranch[] = {false, false};
// start token of conditional code // start token of conditional code
Token* startTokens[] = {nullptr, nullptr}; Token* startTokens[] = {nullptr, nullptr};

View File

@ -1466,7 +1466,9 @@ private:
" : s->value - 1; // <-- null ptr dereference\n" " : s->value - 1; // <-- null ptr dereference\n"
" return i;\n" " return i;\n"
"}"); "}");
TODO_ASSERT_EQUALS("[test.cpp:4]: (warning) Possible null pointer dereference: s\n", "", errout.str()); ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition 's' is redundant or there is possible null pointer dereference: s.\n",
errout.str());
} }
void nullpointer30() { // #6392 void nullpointer30() { // #6392

View File

@ -719,6 +719,16 @@ private:
"}\n", "}\n",
true); true);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("int f(const std::vector<int> &v) {\n"
" return !v.empty() ? 42 : v.back();\n"
"}\n",
true);
ASSERT_EQUALS(
"test.cpp:2:warning:Either the condition 'v.empty()' is redundant or expression 'v.back()' cause access out of bounds.\n"
"test.cpp:2:note:condition 'v.empty()'\n"
"test.cpp:2:note:Access out of bounds\n",
errout.str());
} }
void outOfBoundsSymbolic() void outOfBoundsSymbolic()

View File

@ -92,6 +92,7 @@ private:
TEST_CASE(valueFlowAfterAssign); TEST_CASE(valueFlowAfterAssign);
TEST_CASE(valueFlowAfterSwap); TEST_CASE(valueFlowAfterSwap);
TEST_CASE(valueFlowAfterCondition); TEST_CASE(valueFlowAfterCondition);
TEST_CASE(valueFlowAfterConditionTernary);
TEST_CASE(valueFlowAfterConditionExpr); TEST_CASE(valueFlowAfterConditionExpr);
TEST_CASE(valueFlowAfterConditionSeveralNot); TEST_CASE(valueFlowAfterConditionSeveralNot);
TEST_CASE(valueFlowForwardCompoundAssign); TEST_CASE(valueFlowForwardCompoundAssign);
@ -3160,6 +3161,54 @@ private:
ASSERT_EQUALS(true, testValueOfX(code, 9U, 0)); ASSERT_EQUALS(true, testValueOfX(code, 9U, 0));
} }
void valueFlowAfterConditionTernary()
{
const char* code;
code = "auto f(int x) {\n"
" return x == 3 ?\n"
" x :\n"
" 0;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 3U, 3));
code = "auto f(int x) {\n"
" return x != 3 ?\n"
" 0 :\n"
" x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 4U, 3));
code = "auto f(int x) {\n"
" return !(x == 3) ?\n"
" 0 :\n"
" x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 4U, 3));
code = "auto f(int* x) {\n"
" return x ?\n"
" x :\n"
" 0;\n"
"}\n";
ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
code = "auto f(int* x) {\n"
" return x ?\n"
" 0 :\n"
" x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 4U, 0));
code = "bool g(int);\n"
"auto f(int* x) {\n"
" if (!g(x ?\n"
" *x :\n"
" 0)) {}\n"
"}\n";
ASSERT_EQUALS(false, testValueOfX(code, 4U, 0));
}
void valueFlowAfterConditionExpr() { void valueFlowAfterConditionExpr() {
const char* code; const char* code;