valueFlowUninit: Handle arrays and pod types (#3917)

* valueFlowUninit: Handle arrays and pod types

* Format

* Catch another array case
This commit is contained in:
Paul Fultz II 2022-03-24 00:35:44 -05:00 committed by GitHub
parent 3bcbba598d
commit 4b4037540a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 73 additions and 35 deletions

View File

@ -230,7 +230,7 @@
<noreturn>false</noreturn> <noreturn>false</noreturn>
<returnValue type="void"/> <returnValue type="void"/>
<leak-ignore/> <leak-ignore/>
<arg nr="1"> <arg nr="1" direction="out">
<not-null/> <not-null/>
<minsize type="argvalue" arg="2"/> <minsize type="argvalue" arg="2"/>
</arg> </arg>

View File

@ -1539,34 +1539,41 @@ void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string
"$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE_USE_OF_UNINITIALIZED_VARIABLE, Certainty::normal); "$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE_USE_OF_UNINITIALIZED_VARIABLE, Certainty::normal);
} }
enum class FunctionUsage { None, PassedByReference, Used }; enum class ExprUsage { None, PassedByReference, Used };
static FunctionUsage getFunctionUsage(const Token* tok, int indirect, const Settings* settings) static ExprUsage getFunctionUsage(const Token* tok, int indirect, const Settings* settings)
{ {
const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&"); const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&");
int argnr; int argnr;
const Token* ftok = getTokenArgumentFunction(tok, argnr); const Token* ftok = getTokenArgumentFunction(tok, argnr);
if (!ftok) if (!ftok)
return FunctionUsage::None; return ExprUsage::None;
if (ftok->function()) { if (ftok->function()) {
std::vector<const Variable*> args = getArgumentVars(ftok, argnr); std::vector<const Variable*> args = getArgumentVars(ftok, argnr);
for (const Variable* arg : args) { for (const Variable* arg : args) {
if (!arg) if (!arg)
continue; continue;
if (arg->isReference()) if (arg->isReference())
return FunctionUsage::PassedByReference; return ExprUsage::PassedByReference;
} }
} else { } else {
const bool isnullbad = settings->library.isnullargbad(ftok, argnr + 1); const bool isnullbad = settings->library.isnullargbad(ftok, argnr + 1);
if (indirect == 0 && astIsPointer(tok) && !addressOf && isnullbad) if (indirect == 0 && astIsPointer(tok) && !addressOf && isnullbad)
return FunctionUsage::Used; return ExprUsage::Used;
bool hasIndirect = false; bool hasIndirect = false;
const bool isuninitbad = settings->library.isuninitargbad(ftok, argnr + 1, indirect, &hasIndirect); const bool isuninitbad = settings->library.isuninitargbad(ftok, argnr + 1, indirect, &hasIndirect);
if (isuninitbad && (!addressOf || isnullbad)) if (isuninitbad && (!addressOf || isnullbad))
return FunctionUsage::Used; return ExprUsage::Used;
} }
return FunctionUsage::None; return ExprUsage::None;
}
static ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings)
{
if (indirect == 0 && Token::Match(tok->astParent(), "%cop%|%assign%") && tok->astParent()->str() != "=")
return ExprUsage::Used;
return getFunctionUsage(tok, indirect, settings);
} }
static bool isLeafDot(const Token* tok) static bool isLeafDot(const Token* tok)
@ -1634,10 +1641,10 @@ void CheckUninitVar::valueFlowUninit()
if (Token::Match(tok->astParent(), ". %var%") && !isleaf) if (Token::Match(tok->astParent(), ". %var%") && !isleaf)
continue; continue;
} }
FunctionUsage fusage = getFunctionUsage(tok, v->indirect, mSettings); ExprUsage usage = getExprUsage(tok, v->indirect, mSettings);
if (!v->subexpressions.empty() && fusage == FunctionUsage::PassedByReference) if (!v->subexpressions.empty() && usage == ExprUsage::PassedByReference)
continue; continue;
if (fusage != FunctionUsage::Used) { if (usage != ExprUsage::Used) {
if (!(Token::Match(tok->astParent(), ". %name% (") && uninitderef) && if (!(Token::Match(tok->astParent(), ". %name% (") && uninitderef) &&
isVariableChanged(tok, v->indirect, mSettings, mTokenizer->isCPP())) isVariableChanged(tok, v->indirect, mSettings, mTokenizer->isCPP()))
continue; continue;

View File

@ -6920,6 +6920,10 @@ bool ValueType::fromLibraryType(const std::string &typestr, const Settings *sett
type = ValueType::Type::UNKNOWN_INT; type = ValueType::Type::UNKNOWN_INT;
sign = (podtype->sign == 'u') ? ValueType::UNSIGNED : ValueType::SIGNED; sign = (podtype->sign == 'u') ? ValueType::UNSIGNED : ValueType::SIGNED;
return true; return true;
} else if (podtype && podtype->stdtype == Library::PodType::Type::NO) {
type = ValueType::Type::POD;
sign = ValueType::UNKNOWN_SIGN;
return true;
} }
const Library::PlatformType *platformType = settings->library.platform_type(typestr, settings->platformString()); const Library::PlatformType *platformType = settings->library.platform_type(typestr, settings->platformString());

View File

@ -1216,6 +1216,7 @@ public:
enum Sign { UNKNOWN_SIGN, SIGNED, UNSIGNED } sign; enum Sign { UNKNOWN_SIGN, SIGNED, UNSIGNED } sign;
enum Type { enum Type {
UNKNOWN_TYPE, UNKNOWN_TYPE,
POD,
NONSTD, NONSTD,
RECORD, RECORD,
SMART_POINTER, SMART_POINTER,

View File

@ -2338,7 +2338,7 @@ struct ValueFlowAnalyzer : Analyzer {
} else { } else {
return analyzeMatch(tok, d) | Action::Match; return analyzeMatch(tok, d) | Action::Match;
} }
} else if (ref->isUnaryOp("*")) { } else if (ref->isUnaryOp("*") && !match(ref->astOperand1())) {
const Token* lifeTok = nullptr; const Token* lifeTok = nullptr;
for (const ValueFlow::Value& v:ref->astOperand1()->values()) { for (const ValueFlow::Value& v:ref->astOperand1()->values()) {
if (!v.isLocalLifetimeValue()) if (!v.isLocalLifetimeValue())
@ -7030,7 +7030,7 @@ static bool needsInitialization(const Variable* var, bool cpp)
return true; return true;
if (var->type() && var->type()->needInitialization == Type::NeedInitialization::True) if (var->type() && var->type()->needInitialization == Type::NeedInitialization::True)
return true; return true;
if (var->valueType() && var->valueType()->isPrimitive()) if (var->valueType() && (var->valueType()->isPrimitive() || var->valueType()->type == ValueType::Type::POD))
return true; return true;
return false; return false;
} }
@ -7057,36 +7057,26 @@ static void addToErrorPath(ValueFlow::Value& value, const ValueFlow::Value& from
static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDatabase*/, const Settings* settings) static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDatabase*/, const Settings* settings)
{ {
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
if (!Token::Match(tok,"[;{}] %type%"))
continue;
if (!tok->scope()->isExecutable()) if (!tok->scope()->isExecutable())
continue; continue;
const Token *vardecl = tok->next(); if (!Token::Match(tok, "%var% ;|["))
bool stdtype = false; continue;
bool pointer = false; const Variable* var = tok->variable();
while (Token::Match(vardecl, "%name%|::|*") && vardecl->varId() == 0) { if (!var)
stdtype |= vardecl->isStandardType(); continue;
pointer |= vardecl->str() == "*"; if (var->nameToken() != tok || var->isInit())
vardecl = vardecl->next();
}
// if (!stdtype && !pointer)
// continue;
if (!Token::Match(vardecl, "%var% ;"))
continue; continue;
const Variable *var = vardecl->variable();
if (!needsInitialization(var, tokenlist->isCPP())) if (!needsInitialization(var, tokenlist->isCPP()))
continue; continue;
if (var->nameToken() != vardecl || var->isInit())
continue;
if (!var->isLocal() || var->isStatic() || var->isExtern() || var->isReference() || var->isThrow()) if (!var->isLocal() || var->isStatic() || var->isExtern() || var->isReference() || var->isThrow())
continue; continue;
if (!var->type() && !stdtype && !pointer)
continue;
ValueFlow::Value uninitValue; ValueFlow::Value uninitValue;
uninitValue.setKnown(); uninitValue.setKnown();
uninitValue.valueType = ValueFlow::Value::ValueType::UNINIT; uninitValue.valueType = ValueFlow::Value::ValueType::UNINIT;
uninitValue.tokvalue = vardecl; uninitValue.tokvalue = tok;
if (var->isArray())
uninitValue.indirect = 1;
bool partial = false; bool partial = false;
@ -7104,8 +7094,8 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba
partial = true; partial = true;
continue; continue;
} }
MemberExpressionAnalyzer analyzer(memVar.nameToken()->str(), vardecl, uninitValue, tokenlist); MemberExpressionAnalyzer analyzer(memVar.nameToken()->str(), tok, uninitValue, tokenlist);
valueFlowGenericForward(vardecl->next(), vardecl->scope()->bodyEnd, analyzer, settings); valueFlowGenericForward(tok->next(), tok->scope()->bodyEnd, analyzer, settings);
for (auto&& p : *analyzer.partialReads) { for (auto&& p : *analyzer.partialReads) {
Token* tok2 = p.first; Token* tok2 = p.first;
@ -7135,7 +7125,7 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba
if (partial) if (partial)
continue; continue;
valueFlowForward(vardecl->next(), vardecl->scope()->bodyEnd, var->nameToken(), {uninitValue}, tokenlist, settings); valueFlowForward(tok->next(), tok->scope()->bodyEnd, var->nameToken(), {uninitValue}, tokenlist, settings);
} }
} }

View File

@ -96,6 +96,7 @@ private:
TEST_CASE(uninitvar_nonmember); // crash in ycmd test TEST_CASE(uninitvar_nonmember); // crash in ycmd test
TEST_CASE(isVariableUsageDeref); // *p TEST_CASE(isVariableUsageDeref); // *p
TEST_CASE(isVariableUsageDerefValueflow); // *p
TEST_CASE(uninitvar_memberaccess); // (&(a))->b <=> a.b TEST_CASE(uninitvar_memberaccess); // (&(a))->b <=> a.b
@ -6099,6 +6100,41 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void isVariableUsageDerefValueflow()
{
// *p
valueFlowUninit("void f() {\n"
" char a[10];\n"
" char c = *a;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: *a\n", errout.str());
// extracttests.start: extern const int SIZE;
valueFlowUninit("void f() {\n"
" char a[SIZE+10];\n"
" char c = *a;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: *a\n", errout.str());
valueFlowUninit("void f() {\n"
" char a[10];\n"
" *a += 10;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: *a\n", errout.str());
valueFlowUninit("void f() {\n"
" int a[10][10];\n"
" dostuff(*a);\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("void f() {\n"
" void (*fp[1]) (void) = {function1};\n"
" (*fp[0])();\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void uninitvar_memberaccess() { void uninitvar_memberaccess() {
valueFlowUninit("struct foo{char *bar;};\n" valueFlowUninit("struct foo{char *bar;};\n"
"void f(unsigned long long *p) {\n" "void f(unsigned long long *p) {\n"