From 9c56e7ea8db4e9c7936a7d5db87fe9b646f14639 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 17 Jan 2022 20:35:30 +0100 Subject: [PATCH] Fix #6475 FN uninitialized variable usage not detected (#3700) --- lib/ctu.cpp | 45 +++++++++++++++++++++++++++--------------- test/testuninitvar.cpp | 16 +++++++++++++++ 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/lib/ctu.cpp b/lib/ctu.cpp index 07573f458..cb563d73c 100644 --- a/lib/ctu.cpp +++ b/lib/ctu.cpp @@ -306,13 +306,14 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) for (const Scope &scope : symbolDatabase->scopeList) { if (!scope.isExecutable() || scope.type != Scope::eFunction || !scope.function) continue; - const Function *const function = scope.function; + const Function *const scopeFunction = scope.function; // source function calls for (const Token *tok = scope.bodyStart; tok != scope.bodyEnd; tok = tok->next()) { if (tok->str() != "(" || !tok->astOperand1() || !tok->astOperand2()) continue; - if (!tok->astOperand1()->function()) + const Function* tokFunction = tok->astOperand1()->function(); + if (!tokFunction) continue; const std::vector args(getArguments(tok->previous())); for (int argnr = 0; argnr < args.size(); ++argnr) { @@ -327,7 +328,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) continue; FileInfo::FunctionCall functionCall; functionCall.callValueType = value.valueType; - functionCall.callId = getFunctionId(tokenizer, tok->astOperand1()->function()); + functionCall.callId = getFunctionId(tokenizer, tokFunction); functionCall.callFunctionName = tok->astOperand1()->expressionString(); functionCall.location = FileInfo::Location(tokenizer,tok); functionCall.callArgNr = argnr + 1; @@ -348,7 +349,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) if (argtok->variable() && argtok->variable()->isArray() && argtok->variable()->dimensions().size()==1 && argtok->variable()->dimension(0)>1) { FileInfo::FunctionCall functionCall; functionCall.callValueType = ValueFlow::Value::ValueType::BUFFER_SIZE; - functionCall.callId = getFunctionId(tokenizer, tok->astOperand1()->function()); + functionCall.callId = getFunctionId(tokenizer, tokFunction); functionCall.callFunctionName = tok->astOperand1()->expressionString(); functionCall.location = FileInfo::Location(tokenizer, tok); functionCall.callArgNr = argnr + 1; @@ -361,7 +362,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) if (argtok->isUnaryOp("&") && argtok->astOperand1()->variable() && argtok->astOperand1()->valueType() && !argtok->astOperand1()->variable()->isArray()) { FileInfo::FunctionCall functionCall; functionCall.callValueType = ValueFlow::Value::ValueType::BUFFER_SIZE; - functionCall.callId = getFunctionId(tokenizer, tok->astOperand1()->function()); + functionCall.callId = getFunctionId(tokenizer, tokFunction); functionCall.callFunctionName = tok->astOperand1()->expressionString(); functionCall.location = FileInfo::Location(tokenizer, tok); functionCall.callArgNr = argnr + 1; @@ -370,19 +371,31 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) functionCall.warning = false; fileInfo->functionCalls.push_back(functionCall); } - // pointer to uninitialized data.. - if (!argtok->isUnaryOp("&")) - continue; - argtok = argtok->astOperand1(); - if (!argtok || !argtok->valueType() || argtok->valueType()->pointer != 0) - continue; - if (argtok->values().size() != 1U) + // pointer/reference to uninitialized data + auto isAddressOfArg = [](const Token* argtok) -> const Token* { + if (!argtok->isUnaryOp("&")) + return nullptr; + argtok = argtok->astOperand1(); + if (!argtok || !argtok->valueType() || argtok->valueType()->pointer != 0) + return nullptr; + return argtok; + }; + auto isReferenceArg = [&](const Token* argtok) -> const Token* { + const Variable* argvar = tokFunction->getArgumentVar(argnr); + if (!argvar || !argvar->valueType() || argvar->valueType()->reference == Reference::None) + return nullptr; + return argtok; + }; + const Token* addr = isAddressOfArg(argtok); + argtok = addr ? addr : isReferenceArg(argtok); + if (!argtok || argtok->values().size() != 1U) continue; + const ValueFlow::Value &v = argtok->values().front(); if (v.valueType == ValueFlow::Value::ValueType::UNINIT && !v.isInconclusive()) { FileInfo::FunctionCall functionCall; functionCall.callValueType = ValueFlow::Value::ValueType::UNINIT; - functionCall.callId = getFunctionId(tokenizer, tok->astOperand1()->function()); + functionCall.callId = getFunctionId(tokenizer, tokFunction); functionCall.callFunctionName = tok->astOperand1()->expressionString(); functionCall.location = FileInfo::Location(tokenizer, tok); functionCall.callArgNr = argnr + 1; @@ -396,11 +409,11 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) } // Nested function calls - for (int argnr = 0; argnr < function->argCount(); ++argnr) { + for (int argnr = 0; argnr < scopeFunction->argCount(); ++argnr) { const Token *tok; const int argnr2 = isCallFunction(&scope, argnr, &tok); if (argnr2 > 0) { - FileInfo::NestedCall nestedCall(tokenizer, function, tok); + FileInfo::NestedCall nestedCall(tokenizer, scopeFunction, tok); nestedCall.myArgNr = argnr + 1; nestedCall.callArgNr = argnr2; fileInfo->nestedCalls.push_back(nestedCall); @@ -415,7 +428,7 @@ static std::list> getUnsafeFunction(co { std::list> ret; const Variable * const argvar = scope->function->getArgumentVar(argnr); - if (!argvar->isPointer()) + if (!argvar->isPointer() && !argvar->isReference()) return ret; for (const Token *tok2 = scope->bodyStart; tok2 != scope->bodyEnd; tok2 = tok2->next()) { if (Token::Match(tok2, ")|else {")) { diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 278412a2f..a8b46305e 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -6136,6 +6136,22 @@ private: " f(&x);\n" "}"); ASSERT_EQUALS("", errout.str()); + + ctu("void increment(int& i) { ++i; }\n" // #6475 + "int f() {\n" + " int n;\n" + " increment(n);\n" + " return n;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (error) Using argument i that points at uninitialized variable n\n", errout.str()); + + ctu("void increment(int* i) { ++(*i); }\n" + "int f() {\n" + " int n;\n" + " increment(&n);\n" + " return n;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (error) Using argument i that points at uninitialized variable n\n", errout.str()); } };