Skip to content

Commit

Permalink
Improve mismatchingContainerIterator message (danmar#5897)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chrchr-github
chrchr-github committed Jan 23, 2024
1 parent 915824a commit ab93362
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 30 deletions.
19 changes: 10 additions & 9 deletions lib/checkstl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,21 +231,21 @@ void CheckStl::outOfBoundsError(const Token *tok, const std::string &containerNa
if (indexValue && indexValue->condition)
errmsg = ValueFlow::eitherTheConditionIsRedundant(indexValue->condition) + " or '" + index +
"' can have the value " + indexValueString(*indexValue, containerName) + ". Expression '" +
expression + "' cause access out of bounds.";
expression + "' causes access out of bounds.";
else
errmsg = "Out of bounds access in expression '" + expression + "'";
} else if (containerSize->intvalue == 0) {
if (containerSize->condition)
errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or expression '" + expression + "' cause access out of bounds.";
errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or expression '" + expression + "' causes access out of bounds.";
else if (indexValue == nullptr && !index.empty() && tok->valueType() && tok->valueType()->type == ValueType::ITERATOR)
errmsg = "Out of bounds access in expression '" + expression + "' because '$symbol' is empty and '" + index + "' may be non-zero.";
else
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 " + std::to_string(containerSize->intvalue) + ". Expression '" + expression + "' cause access out of bounds.";
errmsg = ValueFlow::eitherTheConditionIsRedundant(containerSize->condition) + " or size of '$symbol' can be " + std::to_string(containerSize->intvalue) + ". Expression '" + expression + "' causes access out of bounds.";
else if (indexValue->condition)
errmsg = ValueFlow::eitherTheConditionIsRedundant(indexValue->condition) + " or '" + index + "' can have the value " + indexValueString(*indexValue) + ". Expression '" + expression + "' cause access out of bounds.";
errmsg = ValueFlow::eitherTheConditionIsRedundant(indexValue->condition) + " or '" + index + "' can have the value " + indexValueString(*indexValue) + ". Expression '" + expression + "' causes access out of bounds.";
else
errmsg = "Out of bounds access in '" + expression + "', if '$symbol' size is " + std::to_string(containerSize->intvalue) + " and '" + index + "' is " + indexValueString(*indexValue);
} else {
Expand Down Expand Up @@ -646,14 +646,15 @@ void CheckStl::iterators()
}
}

void CheckStl::mismatchingContainerIteratorError(const Token* tok, const Token* iterTok)
void CheckStl::mismatchingContainerIteratorError(const Token* containerTok, const Token* iterTok, const Token* containerTok2)
{
const std::string container(tok ? tok->expressionString() : std::string("v1"));
const std::string container(containerTok ? containerTok->expressionString() : std::string("v1"));
const std::string container2(containerTok2 ? containerTok2->expressionString() : std::string("v2"));
const std::string iter(iterTok ? iterTok->expressionString() : std::string("it"));
reportError(tok,
reportError(containerTok,
Severity::error,
"mismatchingContainerIterator",
"Iterator '" + iter + "' from different container '" + container + "' are used together.",
"Iterator '" + iter + "' referring to container '" + container2 + "' is used with container '" + container + "'.",
CWE664,
Certainty::normal);
}
Expand Down Expand Up @@ -869,7 +870,7 @@ void CheckStl::mismatchingContainerIterator()
continue;
if (isSameIteratorContainerExpression(tok, val.tokvalue, mSettings->library))
continue;
mismatchingContainerIteratorError(tok, iterTok);
mismatchingContainerIteratorError(tok, iterTok, val.tokvalue);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/checkstl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ class CPPCHECKLIB CheckStl : public Check {
void iteratorsError(const Token* tok, const std::string& containerName1, const std::string& containerName2);
void iteratorsError(const Token* tok, const Token* containerTok, const std::string& containerName1, const std::string& containerName2);
void iteratorsError(const Token* tok, const Token* containerTok, const std::string& containerName);
void mismatchingContainerIteratorError(const Token* tok, const Token* iterTok);
void mismatchingContainerIteratorError(const Token* containerTok, const Token* iterTok, const Token* containerTok2);
void mismatchingContainersError(const Token* tok1, const Token* tok2);
void mismatchingContainerExpressionError(const Token *tok1, const Token *tok2);
void sameIteratorExpressionError(const Token *tok);
Expand Down Expand Up @@ -247,7 +247,7 @@ class CPPCHECKLIB CheckStl : public Check {
c.iteratorsError(nullptr, nullptr, "container");
c.invalidContainerLoopError(nullptr, nullptr, errorPath);
c.invalidContainerError(nullptr, nullptr, nullptr, errorPath);
c.mismatchingContainerIteratorError(nullptr, nullptr);
c.mismatchingContainerIteratorError(nullptr, nullptr, nullptr);
c.mismatchingContainersError(nullptr, nullptr);
c.mismatchingContainerExpressionError(nullptr, nullptr);
c.sameIteratorExpressionError(nullptr);
Expand Down
38 changes: 19 additions & 19 deletions test/teststl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,10 @@ class TestStl : public TestFixture {
" bar(v[2], v[3]) )\n" // v[3] is accessed
" {;}\n"
"}\n");
ASSERT_EQUALS("test.cpp:9:warning:Either the condition 'v.size()>=2' is redundant or v size can be 2. Expression 'v[2]' cause access out of bounds.\n"
ASSERT_EQUALS("test.cpp:9:warning:Either the condition 'v.size()>=2' is redundant or size of 'v' can be 2. Expression 'v[2]' causes access out of bounds.\n"
"test.cpp:8:note:condition 'v.size()>=2'\n"
"test.cpp:9:note:Access out of bounds\n"
"test.cpp:9:warning:Either the condition 'v.size()>=2' is redundant or v size can be 2. Expression 'v[3]' cause access out of bounds.\n"
"test.cpp:9:warning:Either the condition 'v.size()>=2' is redundant or size of 'v' can be 2. Expression 'v[3]' causes access out of bounds.\n"
"test.cpp:8:note:condition 'v.size()>=2'\n"
"test.cpp:9:note:Access out of bounds\n", errout.str());

Expand All @@ -247,15 +247,15 @@ class TestStl : public TestFixture {
" v.front();\n"
" if (v.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"
ASSERT_EQUALS("test.cpp:2:warning:Either the condition 'v.empty()' is redundant or expression 'v.front()' causes access out of bounds.\n"
"test.cpp:3:note:condition 'v.empty()'\n"
"test.cpp:2:note:Access out of bounds\n", errout.str());

checkNormal("void f(std::vector<int> v) {\n"
" if (v.size() == 3) {}\n"
" v[16] = 0;\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"
ASSERT_EQUALS("test.cpp:3:warning:Either the condition 'v.size()==3' is redundant or size of 'v' can be 3. Expression 'v[16]' causes 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());

Expand All @@ -265,7 +265,7 @@ class TestStl : public TestFixture {
" v[i] = 0;\n"
" }\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"
ASSERT_EQUALS("test.cpp:4:warning:Either the condition 'v.size()==3' is redundant or size of 'v' can be 3. Expression 'v[i]' causes 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());

Expand All @@ -285,7 +285,7 @@ class TestStl : public TestFixture {
" s[2] = 0;\n"
" }\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"
ASSERT_EQUALS("test.cpp:3:warning:Either the condition 's.size()==1' is redundant or size of 's' can be 1. Expression 's[2]' causes 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());

Expand Down Expand Up @@ -385,7 +385,7 @@ class TestStl : public TestFixture {
" if(v.at(b?42:0)) {}\n"
"}\n");
ASSERT_EQUALS(
"test.cpp:3:warning:Either the condition 'v.size()==1' is redundant or v size can be 1. Expression 'v.at(b?42:0)' cause access out of bounds.\n"
"test.cpp:3:warning:Either the condition 'v.size()==1' is redundant or size of 'v' can be 1. Expression 'v.at(b?42:0)' causes access out of bounds.\n"
"test.cpp:2:note:condition 'v.size()==1'\n"
"test.cpp:3:note:Access out of bounds\n",
errout.str());
Expand Down Expand Up @@ -628,7 +628,7 @@ class TestStl : public TestFixture {
checkNormal("void foo(const std::vector<int> &v) {\n"
" if(v.size() >=1 && v[0] == 4 && v[1] == 2){}\n"
"}\n");
ASSERT_EQUALS("test.cpp:2:warning:Either the condition 'v.size()>=1' is redundant or v size can be 1. Expression 'v[1]' cause access out of bounds.\n"
ASSERT_EQUALS("test.cpp:2:warning:Either the condition 'v.size()>=1' is redundant or size of 'v' can be 1. Expression 'v[1]' causes access out of bounds.\n"
"test.cpp:2:note:condition 'v.size()>=1'\n"
"test.cpp:2:note:Access out of bounds\n", errout.str());

Expand All @@ -641,7 +641,7 @@ class TestStl : public TestFixture {
" return y;\n"
"}\n");
ASSERT_EQUALS(
"test.cpp:6:warning:Either the condition 'x<2' is redundant or 'x' can have the value greater or equal to 3. Expression 'a[x]' cause access out of bounds.\n"
"test.cpp:6:warning:Either the condition 'x<2' is redundant or 'x' can have the value greater or equal to 3. Expression 'a[x]' causes access out of bounds.\n"
"test.cpp:3:note:condition 'x<2'\n"
"test.cpp:6:note:Access out of bounds\n",
errout.str());
Expand Down Expand Up @@ -692,10 +692,10 @@ class TestStl : public TestFixture {
" if (v[i]) {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("test.cpp:3:warning:Either the condition 'i<=(int)v.size()' is redundant or 'i' can have the value v.size(). Expression 'v[i]' cause access out of bounds.\n"
ASSERT_EQUALS("test.cpp:3:warning:Either the condition 'i<=(int)v.size()' is redundant or 'i' can have the value v.size(). Expression 'v[i]' causes access out of bounds.\n"
"test.cpp:2:note:condition 'i<=(int)v.size()'\n"
"test.cpp:3:note:Access out of bounds\n"
"test.cpp:6:warning:Either the condition 'i<=static_cast<int>(v.size())' is redundant or 'i' can have the value v.size(). Expression 'v[i]' cause access out of bounds.\n"
"test.cpp:6:warning:Either the condition 'i<=static_cast<int>(v.size())' is redundant or 'i' can have the value v.size(). Expression 'v[i]' causes access out of bounds.\n"
"test.cpp:5:note:condition 'i<=static_cast<int>(v.size())'\n"
"test.cpp:6:note:Access out of bounds\n",
errout.str());
Expand Down Expand Up @@ -730,7 +730,7 @@ class TestStl : public TestFixture {
"}\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:warning:Either the condition 'v.empty()' is redundant or expression 'v.back()' causes access out of bounds.\n"
"test.cpp:2:note:condition 'v.empty()'\n"
"test.cpp:2:note:Access out of bounds\n",
errout.str());
Expand Down Expand Up @@ -936,7 +936,7 @@ class TestStl : public TestFixture {
" int x = textline[col];\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:2] -> [test.cpp:4]: (warning) Either the condition 'col>textline.size()' is redundant or 'col' can have the value textline.size(). Expression 'textline[col]' cause access out of bounds.\n",
"[test.cpp:2] -> [test.cpp:4]: (warning) Either the condition 'col>textline.size()' is redundant or 'col' can have the value textline.size(). Expression 'textline[col]' causes access out of bounds.\n",
errout.str());
}

Expand Down Expand Up @@ -1108,7 +1108,7 @@ class TestStl : public TestFixture {
" l2.insert(it, 0);\n"
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n"
"[test.cpp:6]: (error) Iterator 'it' from different container 'l2' are used together.\n",
"[test.cpp:6]: (error) Iterator 'it' referring to container 'l1' is used with container 'l2'.\n",
errout.str());

check("void foo() {\n" // #5803
Expand Down Expand Up @@ -1857,7 +1857,7 @@ class TestStl : public TestFixture {
" while (it != g().end())\n"
" it = v.erase(it);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6]: (error) Iterator 'it' from different container 'v' are used together.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6]: (error) Iterator 'it' referring to container 'g()' is used with container 'v'.\n", errout.str());

check("std::vector<int>& g(int);\n"
"void f(int i, int j) {\n"
Expand All @@ -1866,7 +1866,7 @@ class TestStl : public TestFixture {
" while (it != g(j).end())\n"
" it = r.erase(it);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6]: (error) Iterator 'it' from different container 'r' are used together.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6]: (error) Iterator 'it' referring to container 'g(j)' is used with container 'r'.\n", errout.str());

check("std::vector<int>& g();\n"
"void f() {\n"
Expand Down Expand Up @@ -2085,13 +2085,13 @@ class TestStl : public TestFixture {
" a.insert(b.end(), value);\n"
" return a;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Iterator 'b.end()' from different container 'a' are used together.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Iterator 'b.end()' referring to container 'b' is used with container 'a'.\n", errout.str());

check("std::vector<int> f(std::vector<int> a, std::vector<int> b) {\n"
" a.erase(b.begin());\n"
" return a;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (error) Iterator 'b.begin()' from different container 'a' are used together.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (error) Iterator 'b.begin()' referring to container 'b' is used with container 'a'.\n", errout.str());

// #9973
check("void f() {\n"
Expand Down Expand Up @@ -3206,7 +3206,7 @@ class TestStl : public TestFixture {
" if (v.empty()) {}\n"
" v.pop_back();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Either the condition 'v.empty()' is redundant or expression 'v.pop_back()' cause access out of bounds.\n",
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Either the condition 'v.empty()' is redundant or expression 'v.pop_back()' causes access out of bounds.\n",
errout.str());

check("void f(std::vector<int>& v) {\n"
Expand Down

0 comments on commit ab93362

Please sign in to comment.