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

Clarify STL out of bounds warning message

This commit is contained in:
Daniel Marjamäki 2019-03-29 11:13:25 +01:00
parent 682069d512
commit 3c30d274a0
3 changed files with 32 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
lib/checkstl.cpp
View File

@ -70,7 +70,7 @@ void CheckStl::outOfBounds()
if (!value.errorSeverity() && !mSettings->isEnabled(Settings::WARNING)) if (!value.errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
continue; continue;
if (value.intvalue == 0 && Token::Match(tok, "%name% . %name% (") && container->getYield(tok->strAt(2)) == Library::Container::Yield::ITEM) { if (value.intvalue == 0 && Token::Match(tok, "%name% . %name% (") && container->getYield(tok->strAt(2)) == Library::Container::Yield::ITEM) {
outOfBoundsError(tok, &value, nullptr); outOfBoundsError(tok->tokAt(3), tok->str(), &value, tok->strAt(2), nullptr);
continue; continue;
} }
if (Token::Match(tok, "%name% . %name% (") && container->getYield(tok->strAt(2)) == Library::Container::Yield::START_ITERATOR) { if (Token::Match(tok, "%name% . %name% (") && container->getYield(tok->strAt(2)) == Library::Container::Yield::START_ITERATOR) {
@ -81,26 +81,26 @@ void CheckStl::outOfBounds()
else if (Token::simpleMatch(parent, "+") && parent->astOperand2() && parent->astOperand2()->hasKnownIntValue()) else if (Token::simpleMatch(parent, "+") && parent->astOperand2() && parent->astOperand2()->hasKnownIntValue())
other = parent->astOperand2(); other = parent->astOperand2();
if (other && other->getKnownIntValue() > value.intvalue) { if (other && other->getKnownIntValue() > value.intvalue) {
outOfBoundsError(tok, &value, &other->values().back()); outOfBoundsError(parent, tok->str(), &value, other->expressionString(), &other->values().back());
continue; continue;
} }
} }
if (!container->arrayLike_indexOp && !container->stdStringLike) if (!container->arrayLike_indexOp && !container->stdStringLike)
continue; continue;
if (value.intvalue == 0 && Token::Match(tok, "%name% [")) { if (value.intvalue == 0 && Token::Match(tok, "%name% [")) {
outOfBoundsError(tok, &value, nullptr); outOfBoundsError(tok->next(), tok->str(), &value, "", nullptr);
continue; continue;
} }
if (container->arrayLike_indexOp && Token::Match(tok, "%name% [")) { if (container->arrayLike_indexOp && Token::Match(tok, "%name% [")) {
const ValueFlow::Value *indexValue = tok->next()->astOperand2() ? tok->next()->astOperand2()->getMaxValue(false) : nullptr; const ValueFlow::Value *indexValue = tok->next()->astOperand2() ? tok->next()->astOperand2()->getMaxValue(false) : nullptr;
if (indexValue && indexValue->intvalue >= value.intvalue) { if (indexValue && indexValue->intvalue >= value.intvalue) {
outOfBoundsError(tok, &value, indexValue); outOfBoundsError(tok->next(), tok->str(), &value, tok->next()->astOperand2()->expressionString(), indexValue);
continue; continue;
} }
if (mSettings->isEnabled(Settings::WARNING)) { if (mSettings->isEnabled(Settings::WARNING)) {
indexValue = tok->next()->astOperand2() ? tok->next()->astOperand2()->getMaxValue(true) : nullptr; indexValue = tok->next()->astOperand2() ? tok->next()->astOperand2()->getMaxValue(true) : nullptr;
if (indexValue && indexValue->intvalue >= value.intvalue) { if (indexValue && indexValue->intvalue >= value.intvalue) {
outOfBoundsError(tok, &value, indexValue); outOfBoundsError(tok->next(), tok->str(), &value, tok->next()->astOperand2()->expressionString(), indexValue);
continue; continue;
} }
} }
@ -110,39 +110,40 @@ void CheckStl::outOfBounds()
} }
} }
void CheckStl::outOfBoundsError(const Token *tok, const ValueFlow::Value *containerSize, const ValueFlow::Value *index) void CheckStl::outOfBoundsError(const Token *tok, const std::string &containerName, const ValueFlow::Value *containerSize, const std::string &index, const ValueFlow::Value *indexValue)
{ {
// Do not warn if both the container size and index are possible // Do not warn if both the container size and index value are possible
if (containerSize && index && containerSize->isPossible() && index->isPossible()) if (containerSize && indexValue && containerSize->isPossible() && indexValue->isPossible())
return; return;
const std::string varname = tok ? tok->str() : std::string("var"); const std::string expression = tok ? tok->expressionString() : (containerName+"[x]");
std::string errmsg; std::string errmsg;
if (!containerSize) if (!containerSize)
errmsg = "Out of bounds access of item in container '$symbol'"; errmsg = "Out of bounds access in expression '" + expression + "'";
else if (containerSize->intvalue == 0) { else if (containerSize->intvalue == 0) {
if (containerSize->condition) if (containerSize->condition)
errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or $symbol is accessed out of bounds when $symbol is empty."; errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or expression '" + expression + "' cause access out of bounds.";
else else
errmsg = "Out of bounds access in $symbol because $symbol is empty."; errmsg = "Out of bounds access in expression '" + expression + "' because '$symbol' is empty.";
} else if (index) { } else if (indexValue) {
errmsg = "Accessing $symbol[" + MathLib::toString(index->intvalue) + "] is out of bounds when $symbol size is " + MathLib::toString(containerSize->intvalue) + ".";
if (containerSize->condition) if (containerSize->condition)
errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or $symbol size can be " + MathLib::toString(containerSize->intvalue) + ". " + errmsg; errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or $symbol size can be " + MathLib::toString(containerSize->intvalue) + ". Expression '" + expression + "' cause access out of bounds.";
else if (index->condition) else if (indexValue->condition)
errmsg = ValueFlow::eitherTheConditionIsRedundant(index->condition) + " or $symbol item " + MathLib::toString(index->intvalue) + " can be accessed. " + errmsg; errmsg = ValueFlow::eitherTheConditionIsRedundant(indexValue->condition) + " or '" + index + "' can have the value " + MathLib::toString(indexValue->intvalue) + ". Expression '" + expression + "' cause access out of bounds.";
else
errmsg = "Out of bounds access in '" + expression + "', if '$symbol' size is " + MathLib::toString(containerSize->intvalue) + " and '" + index + "' is " + MathLib::toString(indexValue->intvalue);
} else { } else {
// should not happen // should not happen
return; return;
} }
ErrorPath errorPath; ErrorPath errorPath;
if (!index) if (!indexValue)
errorPath = getErrorPath(tok, containerSize, "Access out of bounds"); errorPath = getErrorPath(tok, containerSize, "Access out of bounds");
else { else {
ErrorPath errorPath1 = getErrorPath(tok, containerSize, "Access out of bounds"); ErrorPath errorPath1 = getErrorPath(tok, containerSize, "Access out of bounds");
ErrorPath errorPath2 = getErrorPath(tok, index, "Access out of bounds"); ErrorPath errorPath2 = getErrorPath(tok, indexValue, "Access out of bounds");
if (errorPath1.size() <= 1) if (errorPath1.size() <= 1)
errorPath = errorPath2; errorPath = errorPath2;
else if (errorPath2.size() <= 1) else if (errorPath2.size() <= 1)
@ -154,11 +155,11 @@ void CheckStl::outOfBoundsError(const Token *tok, const ValueFlow::Value *contai
} }
reportError(errorPath, reportError(errorPath,
(containerSize && !containerSize->errorSeverity()) || (index && !index->errorSeverity()) ? Severity::warning : Severity::error, (containerSize && !containerSize->errorSeverity()) || (indexValue && !indexValue->errorSeverity()) ? Severity::warning : Severity::error,
"containerOutOfBounds", "containerOutOfBounds",
"$symbol:" + varname +"\n" + errmsg, "$symbol:" + containerName +"\n" + errmsg,
CWE398, CWE398,
(containerSize && containerSize->isInconclusive()) || (index && index->isInconclusive())); (containerSize && containerSize->isInconclusive()) || (indexValue && indexValue->isInconclusive()));
} }
bool CheckStl::isContainerSize(const Token *containerToken, const Token *expr) const bool CheckStl::isContainerSize(const Token *containerToken, const Token *expr) const

4
lib/checkstl.h
View File

@ -185,7 +185,7 @@ private:
void string_c_strReturn(const Token* tok); void string_c_strReturn(const Token* tok);
void string_c_strParam(const Token* tok, unsigned int number); void string_c_strParam(const Token* tok, unsigned int number);
void outOfBoundsError(const Token *tok, const ValueFlow::Value *containerSize, const ValueFlow::Value *index); void outOfBoundsError(const Token *tok, const std::string &containerName, const ValueFlow::Value *containerSize, const std::string &index, const ValueFlow::Value *indexValue);
void outOfBoundsIndexExpressionError(const Token *tok, const Token *index); void outOfBoundsIndexExpressionError(const Token *tok, const Token *index);
void stlOutOfBoundsError(const Token* tok, const std::string& num, const std::string& var, bool at); void stlOutOfBoundsError(const Token* tok, const std::string& num, const std::string& var, bool at);
void negativeIndexError(const Token* tok, const ValueFlow::Value& index); void negativeIndexError(const Token* tok, const ValueFlow::Value& index);
@ -221,7 +221,7 @@ private:
void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const OVERRIDE { void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const OVERRIDE {
CheckStl c(nullptr, settings, errorLogger); CheckStl c(nullptr, settings, errorLogger);
c.outOfBoundsError(nullptr, nullptr, nullptr); c.outOfBoundsError(nullptr, "container", nullptr, "x", nullptr);
c.invalidIteratorError(nullptr, "iterator"); c.invalidIteratorError(nullptr, "iterator");
c.iteratorsError(nullptr, "container1", "container2"); c.iteratorsError(nullptr, "container1", "container2");
c.iteratorsError(nullptr, nullptr, "container0", "container1"); c.iteratorsError(nullptr, nullptr, "container0", "container1");

14
test/teststl.cpp
View File

@ -201,19 +201,19 @@ private:
" std::string s;\n" " std::string s;\n"
" s[10] = 1;\n" " s[10] = 1;\n"
"}"); "}");
ASSERT_EQUALS("test.cpp:3:error:Out of bounds access in s because s is empty.\n", errout.str()); ASSERT_EQUALS("test.cpp:3:error:Out of bounds access in expression 's[10]' because 's' is empty.\n", errout.str());
checkNormal("void f() {\n" checkNormal("void f() {\n"
" std::string s = \"abcd\";\n" " std::string s = \"abcd\";\n"
" s[10] = 1;\n" " s[10] = 1;\n"
"}"); "}");
ASSERT_EQUALS("test.cpp:3:error:Accessing s[10] is out of bounds when s size is 4.\n", errout.str()); ASSERT_EQUALS("test.cpp:3:error:Out of bounds access in 's[10]', if 's' size is 4 and '10' is 10\n", errout.str());
checkNormal("void f(std::vector<int> v) {\n" checkNormal("void f(std::vector<int> v) {\n"
" v.front();\n" " v.front();\n"
" if (v.empty()) {}\n" " if (v.empty()) {}\n"
"}\n"); "}\n");
ASSERT_EQUALS("test.cpp:2:warning:Either the condition 'v.empty()' is redundant or v is accessed out of bounds when v is empty.\n" ASSERT_EQUALS("test.cpp:2:warning:Either the condition 'v.empty()' is redundant or expression 'v.front()' cause access out of bounds.\n"
"test.cpp:3:note:condition 'v.empty()'\n" "test.cpp:3:note:condition 'v.empty()'\n"
"test.cpp:2:note:Access out of bounds\n", errout.str()); "test.cpp:2:note:Access out of bounds\n", errout.str());
@ -221,7 +221,7 @@ private:
" if (v.size() == 3) {}\n" " if (v.size() == 3) {}\n"
" v[16] = 0;\n" " v[16] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("test.cpp:3:warning:Either the condition 'v.size()==3' is redundant or v size can be 3. Accessing v[16] is out of bounds when v size is 3.\n" ASSERT_EQUALS("test.cpp:3:warning:Either the condition 'v.size()==3' is redundant or v size can be 3. Expression 'v[16]' cause access out of bounds.\n"
"test.cpp:2:note:condition 'v.size()==3'\n" "test.cpp:2:note:condition 'v.size()==3'\n"
"test.cpp:3:note:Access out of bounds\n", errout.str()); "test.cpp:3:note:Access out of bounds\n", errout.str());
@ -231,7 +231,7 @@ private:
" v[i] = 0;\n" " v[i] = 0;\n"
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("test.cpp:4:warning:Either the condition 'v.size()==3' is redundant or v size can be 3. Accessing v[16] is out of bounds when v size is 3.\n" ASSERT_EQUALS("test.cpp:4:warning:Either the condition 'v.size()==3' is redundant or v size can be 3. Expression 'v[i]' cause access out of bounds.\n"
"test.cpp:3:note:condition 'v.size()==3'\n" "test.cpp:3:note:condition 'v.size()==3'\n"
"test.cpp:4:note:Access out of bounds\n", errout.str()); "test.cpp:4:note:Access out of bounds\n", errout.str());
@ -251,7 +251,7 @@ private:
" s[2] = 0;\n" " s[2] = 0;\n"
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("test.cpp:3:warning:Either the condition 's.size()==1' is redundant or s size can be 1. Accessing s[2] is out of bounds when s size is 1.\n" ASSERT_EQUALS("test.cpp:3:warning:Either the condition 's.size()==1' is redundant or s size can be 1. Expression 's[2]' cause access out of bounds.\n"
"test.cpp:2:note:condition 's.size()==1'\n" "test.cpp:2:note:condition 's.size()==1'\n"
"test.cpp:3:note:Access out of bounds\n", errout.str()); "test.cpp:3:note:Access out of bounds\n", errout.str());
@ -279,7 +279,7 @@ private:
" std::string s;\n" " std::string s;\n"
" x = s.begin() + 1;\n" " x = s.begin() + 1;\n"
"}\n"); "}\n");
ASSERT_EQUALS("test.cpp:3:error:Out of bounds access in s because s is empty.\n", errout.str()); ASSERT_EQUALS("test.cpp:3:error:Out of bounds access in expression 's.begin()+1' because 's' is empty.\n", errout.str());
} }
void outOfBoundsIndexExpression() { void outOfBoundsIndexExpression() {