Skip to content

Commit d7d165a

Browse files
committed
Fix buffer size value flow for global new arrays
1 parent e6f7b92 commit d7d165a

2 files changed

Lines changed: 94 additions & 0 deletions

File tree

lib/valueflow.cpp

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7064,6 +7064,77 @@ static void valueFlowDynamicBufferSize(const TokenList& tokenlist, const SymbolD
70647064
return sizeValue;
70657065
};
70667066

7067+
std::map<const Variable*, ValueFlow::Value> globalBufferSizes;
7068+
7069+
// Get buffer sizes for global pointers initialized with new
7070+
for (const Variable* var : symboldatabase.variableList()) {
7071+
if (!var || !var->isGlobal() || !var->isPointer() || var->isExtern())
7072+
continue;
7073+
7074+
const Token* nameTok = var->nameToken();
7075+
if (!Token::Match(nameTok, "%var% ; %var% ="))
7076+
continue;
7077+
7078+
const Token* initLhs = nameTok->tokAt(2);
7079+
if (!initLhs || initLhs->variable() != var)
7080+
continue;
7081+
7082+
const Token* assignTok = initLhs->next();
7083+
const Token* rhs = assignTok->astOperand2();
7084+
while (rhs && rhs->isCast())
7085+
rhs = rhs->astOperand2() ? rhs->astOperand2() : rhs->astOperand1();
7086+
7087+
if (!rhs || !rhs->isCpp() || rhs->str() != "new")
7088+
continue;
7089+
7090+
const MathLib::bigint sizeValue = getBufferSizeFromNew(rhs);
7091+
if (sizeValue < 0)
7092+
continue;
7093+
7094+
ValueFlow::Value value(sizeValue);
7095+
value.errorPath.emplace_back(assignTok, "Assign " + initLhs->str() + ", buffer with size " + MathLib::toString(sizeValue));
7096+
value.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE;
7097+
value.setKnown();
7098+
globalBufferSizes.emplace(var, std::move(value));
7099+
}
7100+
7101+
// Remove initial buffer sizes if the pointers are changed later
7102+
for (const Token* tok = tokenlist.front(); tok && !globalBufferSizes.empty(); tok = tok->next()) {
7103+
const Variable* var = tok->variable();
7104+
if (!var)
7105+
continue;
7106+
7107+
const auto it = globalBufferSizes.find(var);
7108+
if (it == globalBufferSizes.end())
7109+
continue;
7110+
7111+
const Token* nameTok = var->nameToken();
7112+
const Token* initLhs = Token::Match(nameTok, "%var% ; %var% =") ? nameTok->tokAt(2) : nullptr;
7113+
if (tok == nameTok || tok == initLhs)
7114+
continue;
7115+
7116+
if (isVariableChanged(tok, 0, settings))
7117+
globalBufferSizes.erase(it);
7118+
}
7119+
7120+
// Propagate the buffer size through main()
7121+
for (const auto& entry : globalBufferSizes) {
7122+
const Variable* var = entry.first;
7123+
const ValueFlow::Value& value = entry.second;
7124+
const Token* nameTok = var->nameToken();
7125+
const Token* initLhs = nameTok->tokAt(2);
7126+
7127+
for (const Scope* functionScope : symboldatabase.functionScopes) {
7128+
if (functionScope->className != "main")
7129+
continue;
7130+
if (!functionScope->bodyStart || !functionScope->bodyEnd)
7131+
continue;
7132+
7133+
valueFlowForward(const_cast<Token*>(functionScope->bodyStart->next()), functionScope->bodyEnd, initLhs, value, tokenlist, errorLogger, settings);
7134+
break;
7135+
}
7136+
}
7137+
70677138
for (const Scope *functionScope : symboldatabase.functionScopes) {
70687139
for (const Token *tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) {
70697140
if (!Token::Match(tok, "[;{}] %var% ="))

test/testbufferoverrun.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3078,6 +3078,29 @@ class TestBufferOverrun : public TestFixture {
30783078
" delete[] z;\n"
30793079
"}\n");
30803080
ASSERT_EQUALS("[test.cpp:4:10]: (error) Array 'z[5]' accessed at index 7, which is out of bounds. [arrayIndexOutOfBounds]\n", errout_str());
3081+
3082+
// #14934
3083+
check("int *a = new int[2];\n"
3084+
"int main() {\n"
3085+
" return a[5];\n"
3086+
"}\n");
3087+
ASSERT_EQUALS("[test.cpp:3:13]: (error) Array 'a[2]' accessed at index 5, which is out of bounds. [arrayIndexOutOfBounds]\n", errout_str());
3088+
3089+
check("int *a = new int[2];\n"
3090+
"int main() {\n"
3091+
" a = new int[10];\n"
3092+
" return a[5];\n"
3093+
"}\n");
3094+
ASSERT_EQUALS("", errout_str());
3095+
3096+
check("int *a = new int[2];\n"
3097+
"void reset();\n"
3098+
"int main() {\n"
3099+
" reset();\n"
3100+
" return a[5];\n"
3101+
"}\n");
3102+
ASSERT_EQUALS("", errout_str());
3103+
30813104
}
30823105

30833106
void buffer_overrun_2_struct() {

0 commit comments

Comments
 (0)