diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 8fe75b2e7..87f1823f2 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -70,7 +70,7 @@ void CheckStl::outOfBounds() if (!value.errorSeverity() && !mSettings->isEnabled(Settings::WARNING)) continue; 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; } 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()) other = parent->astOperand2(); if (other && other->getKnownIntValue() > value.intvalue) { - outOfBoundsError(tok, &value, &other->values().back()); + outOfBoundsError(parent, tok->str(), &value, other->expressionString(), &other->values().back()); continue; } } if (!container->arrayLike_indexOp && !container->stdStringLike) continue; if (value.intvalue == 0 && Token::Match(tok, "%name% [")) { - outOfBoundsError(tok, &value, nullptr); + outOfBoundsError(tok->next(), tok->str(), &value, "", nullptr); continue; } if (container->arrayLike_indexOp && Token::Match(tok, "%name% [")) { const ValueFlow::Value *indexValue = tok->next()->astOperand2() ? tok->next()->astOperand2()->getMaxValue(false) : nullptr; if (indexValue && indexValue->intvalue >= value.intvalue) { - outOfBoundsError(tok, &value, indexValue); + outOfBoundsError(tok->next(), tok->str(), &value, tok->next()->astOperand2()->expressionString(), indexValue); continue; } if (mSettings->isEnabled(Settings::WARNING)) { indexValue = tok->next()->astOperand2() ? tok->next()->astOperand2()->getMaxValue(true) : nullptr; if (indexValue && indexValue->intvalue >= value.intvalue) { - outOfBoundsError(tok, &value, indexValue); + outOfBoundsError(tok->next(), tok->str(), &value, tok->next()->astOperand2()->expressionString(), indexValue); 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 - if (containerSize && index && containerSize->isPossible() && index->isPossible()) + // Do not warn if both the container size and index value are possible + if (containerSize && indexValue && containerSize->isPossible() && indexValue->isPossible()) return; - const std::string varname = tok ? tok->str() : std::string("var"); + const std::string expression = tok ? tok->expressionString() : (containerName+"[x]"); std::string errmsg; 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) { 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 - errmsg = "Out of bounds access in $symbol because $symbol is empty."; - } else if (index) { - errmsg = "Accessing $symbol[" + MathLib::toString(index->intvalue) + "] is out of bounds when $symbol size is " + MathLib::toString(containerSize->intvalue) + "."; + errmsg = "Out of bounds access in expression '" + expression + "' because '$symbol' is empty."; + } else if (indexValue) { if (containerSize->condition) - errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or $symbol size can be " + MathLib::toString(containerSize->intvalue) + ". " + errmsg; - else if (index->condition) - errmsg = ValueFlow::eitherTheConditionIsRedundant(index->condition) + " or $symbol item " + MathLib::toString(index->intvalue) + " can be accessed. " + errmsg; + errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or $symbol size can be " + MathLib::toString(containerSize->intvalue) + ". Expression '" + expression + "' cause access out of bounds."; + else if (indexValue->condition) + 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 { // should not happen return; } ErrorPath errorPath; - if (!index) + if (!indexValue) errorPath = getErrorPath(tok, containerSize, "Access out of bounds"); else { 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) errorPath = errorPath2; else if (errorPath2.size() <= 1) @@ -154,11 +155,11 @@ void CheckStl::outOfBoundsError(const Token *tok, const ValueFlow::Value *contai } reportError(errorPath, - (containerSize && !containerSize->errorSeverity()) || (index && !index->errorSeverity()) ? Severity::warning : Severity::error, + (containerSize && !containerSize->errorSeverity()) || (indexValue && !indexValue->errorSeverity()) ? Severity::warning : Severity::error, "containerOutOfBounds", - "$symbol:" + varname +"\n" + errmsg, + "$symbol:" + containerName +"\n" + errmsg, CWE398, - (containerSize && containerSize->isInconclusive()) || (index && index->isInconclusive())); + (containerSize && containerSize->isInconclusive()) || (indexValue && indexValue->isInconclusive())); } bool CheckStl::isContainerSize(const Token *containerToken, const Token *expr) const diff --git a/lib/checkstl.h b/lib/checkstl.h index 7c558e342..a3315341b 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -185,7 +185,7 @@ private: void string_c_strReturn(const Token* tok); 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 stlOutOfBoundsError(const Token* tok, const std::string& num, const std::string& var, bool at); void negativeIndexError(const Token* tok, const ValueFlow::Value& index); @@ -221,7 +221,7 @@ private: void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const OVERRIDE { CheckStl c(nullptr, settings, errorLogger); - c.outOfBoundsError(nullptr, nullptr, nullptr); + c.outOfBoundsError(nullptr, "container", nullptr, "x", nullptr); c.invalidIteratorError(nullptr, "iterator"); c.iteratorsError(nullptr, "container1", "container2"); c.iteratorsError(nullptr, nullptr, "container0", "container1"); diff --git a/test/teststl.cpp b/test/teststl.cpp index 1a54a88c4..e9e359fab 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -201,19 +201,19 @@ private: " std::string s;\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" " std::string s = \"abcd\";\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 v) {\n" " v.front();\n" " if (v.empty()) {}\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:2:note:Access out of bounds\n", errout.str()); @@ -221,7 +221,7 @@ private: " if (v.size() == 3) {}\n" " v[16] = 0;\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:3:note:Access out of bounds\n", errout.str()); @@ -231,7 +231,7 @@ private: " v[i] = 0;\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:4:note:Access out of bounds\n", errout.str()); @@ -251,7 +251,7 @@ private: " s[2] = 0;\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:3:note:Access out of bounds\n", errout.str()); @@ -279,7 +279,7 @@ private: " std::string s;\n" " x = s.begin() + 1;\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() {