diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 88645b392..ebd3d3e79 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -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()); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c0dae5b33..45a0a27ba 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -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 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 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); } } diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index d4d54424f..de03fe293 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -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 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"