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

Revert "CheckBufferOverrun: Handle multidimensional arrays" 

This reverts commit e98a4a6f14.
This commit is contained in:
Daniel Marjamäki 2019-03-19 13:13:29 +01:00
parent e98a4a6f14
commit 9d1755f449
3 changed files with 61 additions and 137 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

129
lib/checkbufferoverrun.cpp
View File

@ -201,15 +201,8 @@ void CheckBufferOverrun::arrayIndex()
continue; continue;
} }
std::vector<const Token *> indexTokens; const Token *indexToken = tok->astOperand2();
for (const Token *tok2 = tok; tok2 && tok2->str() == "["; tok2 = tok2->link()->next()) { if (!indexToken)
if (!tok2->astOperand2()) {
indexTokens.clear();
break;
}
indexTokens.emplace_back(tok2->astOperand2());
}
if (indexTokens.empty())
continue; continue;
std::vector<Dimension> dimensions; std::vector<Dimension> dimensions;
@ -219,17 +212,7 @@ void CheckBufferOverrun::arrayIndex()
if (array->variable()->isArray() && !array->variable()->dimensions().empty()) { if (array->variable()->isArray() && !array->variable()->dimensions().empty()) {
dimensions = array->variable()->dimensions(); dimensions = array->variable()->dimensions();
if (dimensions.size() >= 1 && (dimensions[0].num <= 1 || !dimensions[0].tok)) { mightBeLarger = (dimensions.size() >= 1 && (dimensions[0].num <= 1 || !dimensions[0].tok));
mightBeLarger = false;
visitAstNodes(tok->astOperand1(),
[&](const Token *child) {
if (child->originalName() == "->") {
mightBeLarger = true;
return ChildrenToVisit::none;
}
return ChildrenToVisit::op1_and_op2;
});
}
} else if (const Token *stringLiteral = array->getValueTokenMinStrSize()) { } else if (const Token *stringLiteral = array->getValueTokenMinStrSize()) {
Dimension dim; Dimension dim;
dim.tok = nullptr; dim.tok = nullptr;
@ -253,94 +236,53 @@ void CheckBufferOverrun::arrayIndex()
if (dimensions.empty()) if (dimensions.empty())
continue; continue;
const MathLib::bigint dim = dimensions[0].num;
// Positive index // Positive index
if (!mightBeLarger) { // TODO check arrays with dim 1 also if (!mightBeLarger) { // TODO check arrays with dim 1 also
for (int cond = 0; cond < 2; cond++) { for (int cond = 0; cond < 2; cond++) {
bool equal = false; const ValueFlow::Value *value = indexToken->getMaxValue(cond == 1);
bool overflow = false;
bool allKnown = true;
std::vector<const ValueFlow::Value *> indexes;
for (size_t i = 0; i < dimensions.size() && i < indexTokens.size(); ++i) {
const ValueFlow::Value *value = indexTokens[i]->getMaxValue(cond == 1);
indexes.push_back(value);
if (!value) if (!value)
continue; continue;
if (!value->isKnown()) { const MathLib::bigint index = value->intvalue;
if (!allKnown) if (index < dim)
continue; continue;
allKnown = false; if (index == dim) {
}
if (array->variable()->isArray() && dimensions[i].num == 0)
continue;
if (value->intvalue == dimensions[i].num)
equal = true;
else if (value->intvalue > dimensions[i].num)
overflow = true;
}
if (!overflow && equal) {
const Token *parent = tok; const Token *parent = tok;
while (Token::simpleMatch(parent, "[")) while (Token::simpleMatch(parent, "["))
parent = parent->astParent(); parent = parent->astParent();
if (parent->isUnaryOp("&")) if (parent->isUnaryOp("&"))
continue; continue;
} }
if (overflow || equal) { arrayIndexError(tok, dimensions, value);
arrayIndexError(tok, dimensions, indexes);
break;
}
} }
} }
// Negative index // Negative index
bool neg = false; const ValueFlow::Value *negativeValue = indexToken->getValueLE(-1, mSettings);
std::vector<const ValueFlow::Value *> negativeIndexes; if (negativeValue) {
for (size_t i = 0; i < indexTokens.size(); ++i) { negativeIndexError(tok, dimensions, negativeValue);
const ValueFlow::Value *negativeValue = indexTokens[i]->getValueLE(-1, mSettings);
negativeIndexes.emplace_back(negativeValue);
if (negativeValue)
neg = true;
}
if (neg) {
negativeIndexError(tok, dimensions, negativeIndexes);
} }
} }
} }
static std::string stringifyIndexes(const std::string &array, const std::vector<const ValueFlow::Value *> &indexValues) static std::string arrayIndexMessage(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *index)
{
if (indexValues.size() == 1)
return MathLib::toString(indexValues[0]->intvalue);
std::ostringstream ret;
ret << array;
for (const ValueFlow::Value *index : indexValues) {
ret << "[";
if (index)
ret << index->intvalue;
else
ret << "*";
ret << "]";
}
return ret.str();
}
static std::string arrayIndexMessage(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexValues, const Token *condition)
{ {
std::string array = tok->astOperand1()->expressionString(); std::string array = tok->astOperand1()->expressionString();
for (const Dimension &dim : dimensions) for (const Dimension &dim : dimensions)
array += "[" + MathLib::toString(dim.num) + "]"; array += "[" + MathLib::toString(dim.num) + "]";
std::ostringstream errmsg; std::ostringstream errmsg;
if (condition) if (index->condition)
errmsg << ValueFlow::eitherTheConditionIsRedundant(condition) errmsg << ValueFlow::eitherTheConditionIsRedundant(index->condition)
<< " or the array '" + array + "' is accessed at index " << stringifyIndexes(tok->astOperand1()->expressionString(), indexValues) << ", which is out of bounds."; << " or the array '" + array + "' is accessed at index " << index->intvalue << ", which is out of bounds.";
else else
errmsg << "Array '" << array << "' accessed at index " << stringifyIndexes(tok->astOperand1()->expressionString(), indexValues) << ", which is out of bounds."; errmsg << "Array '" << array << "' accessed at index " << index->intvalue << ", which is out of bounds.";
return errmsg.str(); return errmsg.str();
} }
void CheckBufferOverrun::arrayIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexes) void CheckBufferOverrun::arrayIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *index)
{ {
if (!tok) { if (!tok) {
reportError(tok, Severity::error, "arrayIndexOutOfBounds", "Array 'arr[16]' accessed at index 16, which is out of bounds.", CWE788, false); reportError(tok, Severity::error, "arrayIndexOutOfBounds", "Array 'arr[16]' accessed at index 16, which is out of bounds.", CWE788, false);
@ -348,51 +290,28 @@ void CheckBufferOverrun::arrayIndexError(const Token *tok, const std::vector<Dim
return; return;
} }
const Token *condition = nullptr;
const ValueFlow::Value *index = nullptr;
for (const ValueFlow::Value *indexValue: indexes) {
if (!indexValue)
continue;
if (!indexValue->errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
return;
if (indexValue->condition)
condition = indexValue->condition;
if (!index || !indexValue->errorPath.empty())
index = indexValue;
}
reportError(getErrorPath(tok, index, "Array index out of bounds"), reportError(getErrorPath(tok, index, "Array index out of bounds"),
index->errorSeverity() ? Severity::error : Severity::warning, index->errorSeverity() ? Severity::error : Severity::warning,
index->condition ? "arrayIndexOutOfBoundsCond" : "arrayIndexOutOfBounds", index->condition ? "arrayIndexOutOfBoundsCond" : "arrayIndexOutOfBounds",
arrayIndexMessage(tok, dimensions, indexes, condition), arrayIndexMessage(tok, dimensions, index),
CWE788, CWE788,
index->isInconclusive()); index->isInconclusive());
} }
void CheckBufferOverrun::negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexes) void CheckBufferOverrun::negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *negativeValue)
{ {
if (!tok) { if (!negativeValue) {
reportError(tok, Severity::error, "negativeIndex", "Negative array index", CWE786, false); reportError(tok, Severity::error, "negativeIndex", "Negative array index", CWE786, false);
return; return;
} }
const Token *condition = nullptr; if (!negativeValue->errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
const ValueFlow::Value *negativeValue = nullptr;
for (const ValueFlow::Value *indexValue: indexes) {
if (!indexValue)
continue;
if (!indexValue->errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
return; return;
if (indexValue->condition)
condition = indexValue->condition;
if (!negativeValue || !indexValue->errorPath.empty())
negativeValue = indexValue;
}
reportError(getErrorPath(tok, negativeValue, "Negative array index"), reportError(getErrorPath(tok, negativeValue, "Negative array index"),
negativeValue->errorSeverity() ? Severity::error : Severity::warning, negativeValue->errorSeverity() ? Severity::error : Severity::warning,
"negativeIndex", "negativeIndex",
arrayIndexMessage(tok, dimensions, indexes, condition), arrayIndexMessage(tok, dimensions, negativeValue),
CWE786, CWE786,
negativeValue->isInconclusive()); negativeValue->isInconclusive());
} }

8
lib/checkbufferoverrun.h
View File

@ -70,8 +70,8 @@ public:
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE { void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
CheckBufferOverrun c(nullptr, settings, errorLogger); CheckBufferOverrun c(nullptr, settings, errorLogger);
c.arrayIndexError(nullptr, std::vector<Dimension>(), std::vector<const ValueFlow::Value *>()); c.arrayIndexError(nullptr, std::vector<Dimension>(), nullptr);
c.negativeIndexError(nullptr, std::vector<Dimension>(), std::vector<const ValueFlow::Value *>()); c.negativeIndexError(nullptr, std::vector<Dimension>(), nullptr);
c.arrayIndexThenCheckError(nullptr, "i"); c.arrayIndexThenCheckError(nullptr, "i");
c.bufferOverflowError(nullptr, nullptr); c.bufferOverflowError(nullptr, nullptr);
} }
@ -79,8 +79,8 @@ public:
private: private:
void arrayIndex(); void arrayIndex();
void arrayIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexes); void arrayIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *index);
void negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexes); void negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *negativeValue);
void bufferOverflow(); void bufferOverflow();
void bufferOverflowError(const Token *tok, const ValueFlow::Value *value); void bufferOverflowError(const Token *tok, const ValueFlow::Value *value);

53
test/testbufferoverrun.cpp
View File

@ -113,7 +113,7 @@ private:
TEST_CASE(array_index_31); // ticket #2120 - out of bounds in subfunction when type is unknown TEST_CASE(array_index_31); // ticket #2120 - out of bounds in subfunction when type is unknown
TEST_CASE(array_index_32); TEST_CASE(array_index_32);
TEST_CASE(array_index_33); // ticket #3044 TEST_CASE(array_index_33); // ticket #3044
TEST_CASE(array_index_34); // ticket #3063 // TODO multidim TEST_CASE(array_index_34); // ticket #3063
TEST_CASE(array_index_35); // ticket #2889 TEST_CASE(array_index_35); // ticket #2889
TEST_CASE(array_index_36); // ticket #2960 TEST_CASE(array_index_36); // ticket #2960
TEST_CASE(array_index_37); TEST_CASE(array_index_37);
@ -127,7 +127,7 @@ private:
TEST_CASE(array_index_45); // #4207 - calling function with variable number of parameters (...) TEST_CASE(array_index_45); // #4207 - calling function with variable number of parameters (...)
TEST_CASE(array_index_46); // #4840 - two-statement for loop TEST_CASE(array_index_46); // #4840 - two-statement for loop
TEST_CASE(array_index_47); // #5849 TEST_CASE(array_index_47); // #5849
TEST_CASE(array_index_multidim); // TODO multidim TEST_CASE(array_index_multidim);
TEST_CASE(array_index_switch_in_for); TEST_CASE(array_index_switch_in_for);
TEST_CASE(array_index_for_in_for); // FP: #2634 TEST_CASE(array_index_for_in_for); // FP: #2634
TEST_CASE(array_index_calculation); TEST_CASE(array_index_calculation);
@ -571,7 +571,7 @@ private:
" struct ABC x;\n" " struct ABC x;\n"
" x.str[1] = 0;" " x.str[1] = 0;"
"}"); "}");
ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.str[1]' accessed at index 1, which is out of bounds.\n", errout.str()); TODO_ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.str[1]' accessed at index 1, which is out of bounds.\n", "", errout.str());
check("struct foo\n" check("struct foo\n"
"{\n" "{\n"
@ -596,7 +596,7 @@ private:
"void f() {\n" "void f() {\n"
" ab.a[2] = 0;\n" " ab.a[2] = 0;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'ab.a[1]' accessed at index 2, which is out of bounds.\n", errout.str()); ASSERT_EQUALS("", errout.str());
} }
@ -1134,8 +1134,8 @@ private:
" y[0][2][0] = 0;\n" " y[0][2][0] = 0;\n"
" y[0][0][2] = 0;\n" " y[0][0][2] = 0;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'y[2][2][2]' accessed at index y[0][2][0], which is out of bounds.\n" ASSERT_EQUALS("[test.cpp:3]: (error) Array 'y[2][2][2]' index y[0][2][0] out of bounds.\n"
"[test.cpp:4]: (error) Array 'y[2][2][2]' accessed at index y[0][0][2], which is out of bounds.\n", errout.str()); "[test.cpp:4]: (error) Array 'y[2][2][2]' index y[0][0][2] out of bounds.\n", errout.str());
check("struct TEST\n" check("struct TEST\n"
"{\n" "{\n"
@ -1155,11 +1155,14 @@ private:
" ptest->b[0][19] = 4;\n" " ptest->b[0][19] = 4;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:9]: (error) Array 'test.a[10]' accessed at index 10, which is out of bounds.\n" ASSERT_EQUALS("[test.cpp:9]: (error) Array 'test.a[10]' accessed at index 10, which is out of bounds.\n"
"[test.cpp:10]: (error) Array 'test.b[10][5]' accessed at index test.b[10][2], which is out of bounds.\n" "[test.cpp:10]: (error) Array 'test.b[10][5]' index test.b[10][2] out of bounds.\n"
"[test.cpp:11]: (error) Array 'test.b[10][5]' accessed at index test.b[0][19], which is out of bounds.\n" "[test.cpp:11]: (error) Array 'test.b[10][5]' index test.b[0][19] out of bounds.\n"
"[test.cpp:14]: (error) Array 'ptest->a[10]' accessed at index 10, which is out of bounds.\n" "[test.cpp:14]: (error) Array 'ptest->a[10]' accessed at index 10, which is out of bounds.\n"
"[test.cpp:15]: (error) Array 'ptest->b[10][5]' accessed at index ptest->b[10][2], which is out of bounds.\n" "[test.cpp:15]: (error) Array 'ptest->b[10][5]' index ptest->b[10][2] out of bounds.\n"
"[test.cpp:16]: (error) Array 'ptest->b[10][5]' accessed at index ptest->b[0][19], which is out of bounds.\n", errout.str()); "[test.cpp:16]: (error) Array 'ptest->b[10][5]' index ptest->b[0][19] out of bounds.\n"
"[test.cpp:14]: (error) Array 'ptest.a[10]' accessed at index 10, which is out of bounds.\n"
"[test.cpp:15]: (error) Array 'ptest.b[10][5]' index ptest.b[10][2] out of bounds.\n"
"[test.cpp:16]: (error) Array 'ptest.b[10][5]' index ptest.b[0][19] out of bounds.\n", errout.str());
check("struct TEST\n" check("struct TEST\n"
"{\n" "{\n"
@ -1175,10 +1178,12 @@ private:
" ptest->a[9][5] = 4;\n" " ptest->a[9][5] = 4;\n"
" ptest->a[0][50] = 4;\n" " ptest->a[0][50] = 4;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:8]: (error) Array 'test.a[10][5]' accessed at index test.a[9][5], which is out of bounds.\n" ASSERT_EQUALS("[test.cpp:8]: (error) Array 'test.a[10][5]' index test.a[9][5] out of bounds.\n"
"[test.cpp:9]: (error) Array 'test.a[10][5]' accessed at index test.a[0][50], which is out of bounds.\n" "[test.cpp:9]: (error) Array 'test.a[10][5]' index test.a[0][50] out of bounds.\n"
"[test.cpp:12]: (error) Array 'ptest->a[10][5]' accessed at index ptest->a[9][5], which is out of bounds.\n" "[test.cpp:12]: (error) Array 'ptest->a[10][5]' index ptest->a[9][5] out of bounds.\n"
"[test.cpp:13]: (error) Array 'ptest->a[10][5]' accessed at index ptest->a[0][50], which is out of bounds.\n", errout.str()); "[test.cpp:13]: (error) Array 'ptest->a[10][5]' index ptest->a[0][50] out of bounds.\n"
"[test.cpp:12]: (error) Array 'ptest.a[10][5]' index ptest.a[9][5] out of bounds.\n"
"[test.cpp:13]: (error) Array 'ptest.a[10][5]' index ptest.a[0][50] out of bounds.\n", errout.str());
} }
void array_index_35() { // ticket #2889 void array_index_35() { // ticket #2889
@ -1545,67 +1550,67 @@ private:
" char a[2][2];\n" " char a[2][2];\n"
" a[2][1] = 'a';\n" " a[2][1] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' accessed at index a[2][1], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' index a[2][1] out of bounds.\n", errout.str());
check("void f()\n" check("void f()\n"
"{\n" "{\n"
" char a[2][2];\n" " char a[2][2];\n"
" a[1][2] = 'a';\n" " a[1][2] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' accessed at index a[1][2], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' index a[1][2] out of bounds.\n", errout.str());
check("void f()\n" check("void f()\n"
"{\n" "{\n"
" char a[2][2][2];\n" " char a[2][2][2];\n"
" a[2][1][1] = 'a';\n" " a[2][1][1] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' accessed at index a[2][1][1], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[2][1][1] out of bounds.\n", errout.str());
check("void f()\n" check("void f()\n"
"{\n" "{\n"
" char a[2][2][2];\n" " char a[2][2][2];\n"
" a[1][2][1] = 'a';\n" " a[1][2][1] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' accessed at index a[1][2][1], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[1][2][1] out of bounds.\n", errout.str());
check("void f()\n" check("void f()\n"
"{\n" "{\n"
" char a[2][2][2][2];\n" " char a[2][2][2][2];\n"
" a[1][2][1][1] = 'a';\n" " a[1][2][1][1] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2][2]' accessed at index a[1][2][1][1], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2][2]' index a[1][2][1][1] out of bounds.\n", errout.str());
check("void f()\n" check("void f()\n"
"{\n" "{\n"
" char a[2][2][2];\n" " char a[2][2][2];\n"
" a[1][1][2] = 'a';\n" " a[1][1][2] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' accessed at index a[1][1][2], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[1][1][2] out of bounds.\n", errout.str());
check("void f()\n" check("void f()\n"
"{\n" "{\n"
" char a[10][10][10];\n" " char a[10][10][10];\n"
" a[2*3][4*3][2] = 'a';\n" " a[2*3][4*3][2] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10][10][10]' accessed at index a[6][12][2], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10][10][10]' index a[6][12][2] out of bounds.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" char a[10][10][10];\n" " char a[10][10][10];\n"
" a[6][40][10] = 'a';\n" " a[6][40][10] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[10][10][10]' accessed at index a[6][40][10], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[10][10][10]' index a[6][40][10] out of bounds.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" char a[1][1][1];\n" " char a[1][1][1];\n"
" a[2][2][2] = 'a';\n" " a[2][2][2] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[1][1][1]' accessed at index a[2][2][2], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[1][1][1]' index a[2][2][2] out of bounds.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" char a[6][6][6];\n" " char a[6][6][6];\n"
" a[6][6][2] = 'a';\n" " a[6][6][2] = 'a';\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[6][6][6]' accessed at index a[6][6][2], which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[6][6][6]' index a[6][6][2] out of bounds.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" int a[2][2];\n" " int a[2][2];\n"