Add subexpression analyzer to set values for uninitialized members of struct (#3517)

* Add subexpression analyzer

* Fix errors

* Add tests

* Format

* Add more tests

* Run members first

* Format

* Uncomment todo assert

* Formatting

* Use simpleMatch
This commit is contained in:
Paul Fultz II 2021-10-21 07:44:48 -05:00 committed by GitHub
parent 257efb4019
commit ca83222bae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 516 additions and 12 deletions

View File

@ -1581,7 +1581,10 @@ void CheckUninitVar::valueFlowUninit()
if (isVariableChangedByFunctionCall(tok, v->indirect, mSettings, &inconclusive) || inconclusive)
continue;
}
uninitvarError(tok, tok->expressionString(), v->errorPath);
const Token* ltok = tok;
if (Token::simpleMatch(tok->astParent(), ".") && astIsRHS(tok))
ltok = tok->astParent();
uninitvarError(ltok, ltok->expressionString(), v->errorPath);
ids.insert(tok->exprId());
if (v->tokvalue)
ids.insert(v->tokvalue->exprId());

View File

@ -561,6 +561,8 @@ static void setTokenValue(Token* tok, ValueFlow::Value value, const Settings* se
}
if (value.isUninitValue()) {
if (Token::Match(tok, ". %var%"))
setTokenValue(tok->next(), value, settings);
ValueFlow::Value pvalue = value;
if (parent->isUnaryOp("&")) {
pvalue.indirect++;
@ -2688,6 +2690,51 @@ struct OppositeExpressionAnalyzer : ExpressionAnalyzer {
}
};
struct SubExpressionAnalyzer : ExpressionAnalyzer {
SubExpressionAnalyzer() : ExpressionAnalyzer() {}
SubExpressionAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t)
: ExpressionAnalyzer(e, val, t)
{}
virtual bool submatch(const Token* tok, bool exact = true) const = 0;
virtual bool isAlias(const Token* tok, bool& inconclusive) const OVERRIDE
{
if (tok->exprId() == expr->exprId() && tok->astParent() && submatch(tok->astParent(), false))
return false;
return ExpressionAnalyzer::isAlias(tok, inconclusive);
}
virtual bool match(const Token* tok) const OVERRIDE
{
return tok->astOperand1() && tok->astOperand1()->exprId() == expr->exprId() && submatch(tok);
}
// No reanalysis for subexression
virtual ValuePtr<Analyzer> reanalyze(Token*, const std::string&) const OVERRIDE {
return {};
}
};
struct MemberExpressionAnalyzer : SubExpressionAnalyzer {
std::string varname;
MemberExpressionAnalyzer() : SubExpressionAnalyzer(), varname() {}
MemberExpressionAnalyzer(std::string varname, const Token* e, const ValueFlow::Value& val, const TokenList* t)
: SubExpressionAnalyzer(e, val, t), varname(std::move(varname))
{}
virtual bool submatch(const Token* tok, bool exact) const OVERRIDE
{
if (!Token::Match(tok, ". %var%"))
return false;
if (!exact)
return true;
return tok->next()->str() == varname;
}
};
static Analyzer::Result valueFlowForwardExpression(Token* startToken,
const Token* endToken,
const Token* exprTok,
@ -6315,6 +6362,23 @@ static void valueFlowFunctionReturn(TokenList *tokenlist, ErrorLogger *errorLogg
}
}
static bool needsInitialization(const Variable* var, bool cpp)
{
if (!var)
return false;
if (var->isPointer())
return true;
if (var->type() && var->type()->isUnionType())
return false;
if (!cpp)
return true;
if (var->type() && var->type()->needInitialization == Type::NeedInitialization::True)
return true;
if (var->valueType() && var->valueType()->isPrimitive())
return true;
return false;
}
static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDatabase*/, const Settings* settings)
{
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
@ -6335,10 +6399,11 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba
if (!Token::Match(vardecl, "%var% ;"))
continue;
const Variable *var = vardecl->variable();
if (!var || var->nameToken() != vardecl || var->isInit())
if (!needsInitialization(var, tokenlist->isCPP()))
continue;
if ((!var->isPointer() && var->type() && var->type()->needInitialization != Type::NeedInitialization::True) ||
!var->isLocal() || var->isStatic() || var->isExtern() || var->isReference() || var->isThrow())
if (var->nameToken() != vardecl || var->isInit())
continue;
if (!var->isLocal() || var->isStatic() || var->isExtern() || var->isReference() || var->isThrow())
continue;
if (!var->type() && !stdtype && !pointer)
continue;
@ -6347,10 +6412,28 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba
uninitValue.setKnown();
uninitValue.valueType = ValueFlow::Value::ValueType::UNINIT;
uninitValue.tokvalue = vardecl;
std::list<ValueFlow::Value> values;
values.push_back(uninitValue);
valueFlowForward(vardecl->next(), vardecl->scope()->bodyEnd, var->nameToken(), values, tokenlist, settings);
bool partial = false;
if (const Scope* scope = var->typeScope()) {
if (Token::findsimplematch(scope->bodyStart, "union", scope->bodyEnd))
continue;
for (const Variable& memVar : scope->varlist) {
if (!memVar.isPublic())
continue;
if (!needsInitialization(&memVar, tokenlist->isCPP())) {
partial = true;
continue;
}
MemberExpressionAnalyzer analyzer(memVar.nameToken()->str(), vardecl, uninitValue, tokenlist);
valueFlowGenericForward(vardecl->next(), vardecl->scope()->bodyEnd, analyzer, settings);
}
}
if (partial)
continue;
valueFlowForward(vardecl->next(), vardecl->scope()->bodyEnd, var->nameToken(), {uninitValue}, tokenlist, settings);
}
}

View File

@ -87,6 +87,7 @@ private:
TEST_CASE(trac_5970);
TEST_CASE(valueFlowUninit);
TEST_CASE(valueFlowUninitBreak);
TEST_CASE(valueFlowUninitStructMembers);
TEST_CASE(uninitvar_ipa);
TEST_CASE(uninitvar_memberfunction);
TEST_CASE(uninitvar_nonmember); // crash in ycmd test
@ -4381,7 +4382,8 @@ private:
// FP Unknown type ASSERT_EQUALS("", errout.str());
}
void valueFlowUninit(const char code[]) {
void valueFlowUninit(const char code[], const char fname[] = "test.cpp")
{
// Clear the error buffer..
errout.str("");
@ -4391,14 +4393,13 @@ private:
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
tokenizer.tokenize(istr, fname);
// Check for redundant code..
CheckUninitVar checkuninitvar(&tokenizer, &settings, this);
checkuninitvar.valueFlowUninit();
}
void valueFlowUninit() {
// Ticket #2207 - False negative
valueFlowUninit("void foo() {\n"
@ -5004,7 +5005,7 @@ private:
" someType_t gVar;\n"
" bar(&gVar);\n"
"}");
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:5]: (error) Uninitialized variable: flags\n", errout.str());
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:5]: (error) Uninitialized variable: p->flags\n", errout.str());
valueFlowUninit("typedef struct\n"
"{\n"
@ -5014,7 +5015,7 @@ private:
" someType_t gVar;\n"
" if(gVar.flags[1] == 42){}\n"
"}");
ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: flags\n", errout.str());
ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: gVar.flags\n", errout.str());
valueFlowUninit("void foo() {\n" // #10293
" union {\n"
@ -5209,6 +5210,423 @@ private:
}
void valueFlowUninitStructMembers()
{
valueFlowUninit("struct AB { int a; int b; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.a = 1;\n"
" if (ab.b == 2) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: ab.b\n", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n"
"void do_something(const struct AB &ab) { a = ab.a; }\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.a = 0;\n"
" do_something(ab);\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" int a = ab.a;\n"
"}\n",
"test.c");
ASSERT_EQUALS("[test.c:4]: (error) Uninitialized variable: ab.a\n", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n"
"void f(void) {\n"
" AB ab1;\n"
" AB ab2 = { ab1.a, 0 };\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: ab1.a\n", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" buf[ab.a] = 0;\n"
"}\n",
"test.c");
ASSERT_EQUALS("[test.c:4]: (error) Uninitialized variable: ab.a\n", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n" // pass struct member by address
"void f(void) {\n"
" struct AB ab;\n"
" assign(&ab.a, 0);\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
valueFlowUninit(
"struct Cstring { char *text; int size, alloc; };\n"
"int maybe();\n"
"void f() {\n"
" Cstring res;\n"
" if ( ! maybe() ) return;\n" // <- fp goes away if this is removed
" ( ((res).text = (void*)0), ((res).size = (res).alloc = 0) );\n" // <- fp goes away if parentheses are removed
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n"
"void do_something(const struct AB ab);\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.a = 0;\n"
" ab.b = 0;\n"
" do_something(ab);\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
{
valueFlowUninit("struct AB { char a[10]; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" strcpy(ab.a, STR);\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct AB { unsigned char a[10]; };\n" // #8999 - cast
"void f(void) {\n"
" struct AB ab;\n"
" strcpy((char *)ab.a, STR);\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct AB { char a[10]; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" strcpy(x, ab.a);\n"
"}\n",
"test.c");
TODO_ASSERT_EQUALS("[test.c:4]: (error) Uninitialized variable: ab.a\n", "", errout.str());
valueFlowUninit("struct AB { int a; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" dosomething(ab.a);\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
}
valueFlowUninit("struct AB { int a; int b; };\n"
"void do_something(const struct AB ab);\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab = getAB();\n"
" do_something(ab);\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
{
// #6769 - calling method that might assign struct members
valueFlowUninit("struct AB { int a; int b; void set(); };\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.set();\n"
" x = ab;\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct AB { int a; int get() const; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.get();\n"
" x = ab;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: ab\n", errout.str());
valueFlowUninit("struct AB { int a; void dostuff() {} };\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.dostuff();\n"
" x = ab;\n"
"}");
TODO_ASSERT_EQUALS("error", "", errout.str());
}
valueFlowUninit("struct AB { int a; struct { int b; int c; } s; };\n"
"void do_something(const struct AB ab);\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.a = 1;\n"
" ab.s.b = 2;\n"
" ab.s.c = 3;\n"
" do_something(ab);\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct conf {\n"
" char x;\n"
"};\n"
"\n"
"void do_something(struct conf ant_conf);\n"
"\n"
"void f(void) {\n"
" struct conf c;\n"
" initdata(&c);\n"
" do_something(c);\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct PIXEL {\n"
" union {\n"
" struct { unsigned char red,green,blue,alpha; };\n"
" unsigned int color;\n"
" };\n"
"};\n"
"\n"
"unsigned char f() {\n"
" struct PIXEL p1;\n"
" p1.color = 255;\n"
" return p1.red;\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n"
"int f() {\n"
" struct AB *ab;\n"
" for (i = 1; i < 10; i++) {\n"
" if (condition && (ab = getab()) != NULL) {\n"
" a = ab->a;\n"
" }\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n"
"int f(int x) {\n"
" struct AB *ab;\n"
" if (x == 0) {\n"
" ab = getab();\n"
" }\n"
" if (x == 0 && (ab != NULL || ab->a == 0)) { }\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct A { int *x; };\n" // declarationId is 0 for "delete"
"void foo(void *info, void*p);\n"
"void bar(void) {\n"
" struct A *delete = 0;\n"
" foo( info, NULL );\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct ABC { int a; int b; int c; };\n"
"void foo(int x, const struct ABC *abc);\n"
"void bar(void) {\n"
" struct ABC abc;\n"
" foo(123, &abc);\n"
" return abc.b;\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: abc.a\n"
"[test.cpp:5]: (error) Uninitialized variable: abc.b\n"
"[test.cpp:5]: (error) Uninitialized variable: abc.c\n",
"",
errout.str());
valueFlowUninit("struct ABC { int a; int b; int c; };\n"
"void foo() {\n"
" struct ABC abc;\n"
" dostuff((uint32_t *)&abc.a);\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("void f(void) {\n"
" struct tm t;\n"
" t.tm_year = 123;\n"
"}");
ASSERT_EQUALS("", errout.str());
// return
valueFlowUninit("struct AB { int a; int b; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.a = 0;\n"
" return ab.b;\n"
"}\n",
"test.c");
ASSERT_EQUALS("[test.c:5]: (error) Uninitialized variable: ab.b\n", errout.str());
valueFlowUninit("struct AB { int a; int b; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" ab.a = 0;\n"
" return ab.a;\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct S { int a; int b; };\n" // #8299
"void f(void) {\n"
" struct S s;\n"
" s.a = 0;\n"
" return s;\n"
"}\n");
TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: s.b\n", "", errout.str());
valueFlowUninit("struct S { int a; int b; };\n" // #9810
"void f(void) {\n"
" struct S s;\n"
" return s.a ? 1 : 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: s.a\n", errout.str());
// checkIfForWhileHead
valueFlowUninit("struct FRED {\n"
" int a;\n"
" int b;\n"
"};\n"
"\n"
"void f(void) {\n"
" struct FRED fred;\n"
" fred.a = do_something();\n"
" if (fred.a == 0) { }\n"
"}\n",
"test.c");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct FRED {\n"
" int a;\n"
" int b;\n"
"};\n"
"\n"
"void f(void) {\n"
" struct FRED fred;\n"
" fred.a = do_something();\n"
" if (fred.b == 0) { }\n"
"}\n",
"test.c");
ASSERT_EQUALS("[test.c:9]: (error) Uninitialized variable: fred.b\n", errout.str());
valueFlowUninit("struct Fred { int a; };\n"
"void f() {\n"
" struct Fred fred;\n"
" if (fred.a==1) {}\n"
"}",
"test.c");
ASSERT_EQUALS("[test.c:4]: (error) Uninitialized variable: fred.a\n", errout.str());
valueFlowUninit("struct S { int n; int m; };\n"
"void f(void) {\n"
" struct S s;\n"
" for (s.n = 0; s.n <= 10; s.n++) { }\n"
"}",
"test.c");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("void test2() {\n"
" struct { char type; } s_d;\n"
" if (foo(&s_d.type)){}\n"
"}");
ASSERT_EQUALS("", errout.str());
// for
valueFlowUninit("struct AB { int a; };\n"
"void f() {\n"
" struct AB ab;\n"
" while (x) { clear(ab); z = ab.a; }\n"
"}");
ASSERT_EQUALS("", errout.str());
valueFlowUninit("struct AB { int a; };\n"
"void f() {\n"
" struct AB ab;\n"
" while (x) { ab.a = ab.a + 1; }\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: ab.a\n", "", errout.str());
valueFlowUninit("struct AB { int a; };\n"
"void f() {\n"
" struct AB ab;\n"
" while (x) { init(&ab); z = ab.a; }\n"
"}");
ASSERT_EQUALS("", errout.str());
// address of member
valueFlowUninit("struct AB { int a[10]; int b; };\n"
"void f() {\n"
" struct AB ab;\n"
" int *p = ab.a;\n"
"}");
ASSERT_EQUALS("", errout.str());
// Reference
valueFlowUninit("struct A { int x; };\n"
"void foo() {\n"
" struct A a;\n"
" int& x = a.x;\n"
" x = 0;\n"
" return a.x;\n"
"}");
ASSERT_EQUALS("", errout.str());
// non static data-member initialization
valueFlowUninit("struct AB { int a=1; int b; };\n"
"void f(void) {\n"
" struct AB ab;\n"
" int a = ab.a;\n"
" int b = ab.b;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: ab.a\n", errout.str());
// STL class member
valueFlowUninit("struct A {\n"
" std::map<int, int> m;\n"
" int i;\n"
"};\n"
"void foo() {\n"
" A a;\n"
" x = a.m;\n"
"}");
ASSERT_EQUALS("", errout.str());
// Unknown type (C++)
valueFlowUninit("struct A {\n"
" C m;\n"
" int i;\n"
"};\n"
"void foo() {\n"
" A a;\n"
" x = a.m;\n"
"}",
"test.cpp");
ASSERT_EQUALS("", errout.str());
// Unknown type (C)
valueFlowUninit("struct A {\n"
" C m;\n"
" int i;\n"
"};\n"
"void foo() {\n"
" A a;\n"
" x = a.m;\n"
"}",
"test.c");
ASSERT_EQUALS("[test.c:7]: (error) Uninitialized variable: a.m\n", errout.str());
// Type with constructor
valueFlowUninit("class C { C(); }\n"
"struct A {\n"
" C m;\n"
" int i;\n"
"};\n"
"void foo() {\n"
" A a;\n"
" x = a.m;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void uninitvar_memberfunction() {
// # 8715
valueFlowUninit("struct C {\n"