From 496202dd41a820122de5a1a104489279f4e20a1e Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Wed, 1 Oct 2014 03:44:58 +0000 Subject: [PATCH] Improve -Wuninitialized warnings for fields that are record types. Get the record handling code from SelfReferenceChecker into UninitializedFieldVisitor as well as copying the testcases. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@218740 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaDeclCXX.cpp | 90 ++++++++++++++++++++------------ test/SemaCXX/uninitialized.cpp | 93 ++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 31 deletions(-) diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 24d7c8f417..b35bca2c84 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -2221,7 +2221,8 @@ namespace { llvm::SmallPtrSetImpl &Decls) : Inherited(S.Context), S(S), Decls(Decls) { } - void HandleMemberExpr(MemberExpr *ME, bool CheckReferenceOnly) { + void HandleMemberExpr(MemberExpr *ME, bool CheckReferenceOnly, + bool AddressOf) { if (isa(ME->getMemberDecl())) return; @@ -2229,6 +2230,8 @@ namespace { // or union. MemberExpr *FieldME = ME; + bool AllPODFields = FieldME->getType().isPODType(S.Context); + Expr *Base = ME; while (isa(Base)) { ME = cast(Base); @@ -2240,12 +2243,18 @@ namespace { if (!FD->isAnonymousStructOrUnion()) FieldME = ME; - Base = ME->getBase(); + if (!FieldME->getType().isPODType(S.Context)) + AllPODFields = false; + + Base = ME->getBase()->IgnoreParenImpCasts(); } if (!isa(Base)) return; + if (AddressOf && AllPODFields) + return; + ValueDecl* FoundVD = FieldME->getMemberDecl(); if (!Decls.count(FoundVD)) @@ -2254,7 +2263,7 @@ namespace { const bool IsReference = FoundVD->getType()->isReferenceType(); // Prevent double warnings on use of unbounded references. - if (IsReference != CheckReferenceOnly) + if (CheckReferenceOnly && !IsReference) return; unsigned diag = IsReference @@ -2268,44 +2277,51 @@ namespace { } - void HandleValue(Expr *E) { + void HandleValue(Expr *E, bool AddressOf) { E = E->IgnoreParens(); if (MemberExpr *ME = dyn_cast(E)) { - HandleMemberExpr(ME, false /*CheckReferenceOnly*/); + HandleMemberExpr(ME, false /*CheckReferenceOnly*/, + AddressOf /*AddressOf*/); return; } if (ConditionalOperator *CO = dyn_cast(E)) { - HandleValue(CO->getTrueExpr()); - HandleValue(CO->getFalseExpr()); + Visit(CO->getCond()); + HandleValue(CO->getTrueExpr(), AddressOf); + HandleValue(CO->getFalseExpr(), AddressOf); return; } if (BinaryConditionalOperator *BCO = dyn_cast(E)) { - HandleValue(BCO->getFalseExpr()); + Visit(BCO->getCond()); + HandleValue(BCO->getFalseExpr(), AddressOf); return; } if (OpaqueValueExpr *OVE = dyn_cast(E)) { - HandleValue(OVE->getSourceExpr()); + HandleValue(OVE->getSourceExpr(), AddressOf); return; } if (BinaryOperator *BO = dyn_cast(E)) { switch (BO->getOpcode()) { default: - return; + break; case(BO_PtrMemD): case(BO_PtrMemI): - HandleValue(BO->getLHS()); + HandleValue(BO->getLHS(), AddressOf); + Visit(BO->getRHS()); return; case(BO_Comma): - HandleValue(BO->getRHS()); + Visit(BO->getLHS()); + HandleValue(BO->getRHS(), AddressOf); return; } } + + Visit(E); } void CheckInitializer(Expr *E, const CXXConstructorDecl *FieldConstructor, @@ -2324,14 +2340,14 @@ namespace { void VisitMemberExpr(MemberExpr *ME) { // All uses of unbounded reference fields will warn. - HandleMemberExpr(ME, true /*CheckReferenceOnly*/); - - Inherited::VisitMemberExpr(ME); + HandleMemberExpr(ME, true /*CheckReferenceOnly*/, false /*AddressOf*/); } void VisitImplicitCastExpr(ImplicitCastExpr *E) { - if (E->getCastKind() == CK_LValueToRValue) - HandleValue(E->getSubExpr()); + if (E->getCastKind() == CK_LValueToRValue) { + HandleValue(E->getSubExpr(), false /*AddressOf*/); + return; + } Inherited::VisitImplicitCastExpr(E); } @@ -2339,23 +2355,24 @@ namespace { void VisitCXXConstructExpr(CXXConstructExpr *E) { if (E->getConstructor()->isCopyConstructor()) { Expr *ArgExpr = E->getArg(0); - if (ImplicitCastExpr* ICE = dyn_cast(ArgExpr)) { - if (ICE->getCastKind() == CK_NoOp) { + if (InitListExpr *ILE = dyn_cast(ArgExpr)) + if (ILE->getNumInits() == 1) + ArgExpr = ILE->getInit(0); + if (ImplicitCastExpr *ICE = dyn_cast(ArgExpr)) + if (ICE->getCastKind() == CK_NoOp) ArgExpr = ICE->getSubExpr(); - } - } - - if (MemberExpr *ME = dyn_cast(ArgExpr)) { - HandleMemberExpr(ME, false /*CheckReferenceOnly*/); - } + HandleValue(ArgExpr, false /*AddressOf*/); + return; } Inherited::VisitCXXConstructExpr(E); } void VisitCXXMemberCallExpr(CXXMemberCallExpr *E) { Expr *Callee = E->getCallee(); - if (isa(Callee)) - HandleValue(Callee); + if (isa(Callee)) { + HandleValue(Callee, false /*AddressOf*/); + return; + } Inherited::VisitCXXMemberCallExpr(E); } @@ -2365,7 +2382,8 @@ namespace { if (E->getNumArgs() == 1) { if (FunctionDecl *FD = E->getDirectCallee()) { if (FD->getIdentifier() && FD->getIdentifier()->isStr("move")) { - HandleValue(E->getArg(0)); + HandleValue(E->getArg(0), false /*AddressOf*/); + return; } } } @@ -2383,15 +2401,25 @@ namespace { DeclsToRemove.push_back(FD); if (E->isCompoundAssignmentOp()) { - HandleValue(E->getLHS()); + HandleValue(E->getLHS(), false /*AddressOf*/); + Visit(E->getRHS()); + return; } Inherited::VisitBinaryOperator(E); } void VisitUnaryOperator(UnaryOperator *E) { - if (E->isIncrementDecrementOp()) - HandleValue(E->getSubExpr()); + if (E->isIncrementDecrementOp()) { + HandleValue(E->getSubExpr(), false /*AddressOf*/); + return; + } + if (E->getOpcode() == UO_AddrOf) { + if (MemberExpr *ME = dyn_cast(E->getSubExpr())) { + HandleValue(ME->getBase(), true /*AddressOf*/); + return; + } + } Inherited::VisitUnaryOperator(E); } diff --git a/test/SemaCXX/uninitialized.cpp b/test/SemaCXX/uninitialized.cpp index b73b2b9c1f..ddf5ce6f6d 100644 --- a/test/SemaCXX/uninitialized.cpp +++ b/test/SemaCXX/uninitialized.cpp @@ -295,6 +295,61 @@ A a38({a38}); // expected-warning {{variable 'a38' is uninitialized when used w A a39 = {a39}; // expected-warning {{variable 'a39' is uninitialized when used within its own initialization}} A a40 = A({a40}); // expected-warning {{variable 'a40' is uninitialized when used within its own initialization}} +class T { + A a, a2; + const A c_a; + A* ptr_a; + + T() {} + T(bool (*)[1]) : a() {} + T(bool (*)[2]) : a2(a.get()) {} + T(bool (*)[3]) : a2(a) {} + T(bool (*)[4]) : a(&a) {} + T(bool (*)[5]) : a(a.zero()) {} + T(bool (*)[6]) : a(a.ONE) {} + T(bool (*)[7]) : a(getA()) {} + T(bool (*)[8]) : a2(getA(a.TWO)) {} + T(bool (*)[9]) : a(getA(&a)) {} + T(bool (*)[10]) : a(a.count) {} + + T(bool (*)[11]) : a(a) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[12]) : a(a.get()) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[13]) : a(a.num) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[14]) : a(A(a)) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[15]) : a(getA(a.num)) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[16]) : a(&a.num) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[17]) : a(a.get2()) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[18]) : a2(cond ? a2 : a) {} // expected-warning {{field 'a2' is uninitialized when used here}} + T(bool (*)[19]) : a2(cond ? a2 : a) {} // expected-warning {{field 'a2' is uninitialized when used here}} + T(bool (*)[20]) : a{a} {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[21]) : a({a}) {} // expected-warning {{field 'a' is uninitialized when used here}} + + T(bool (*)[22]) : ptr_a(new A(ptr_a->count)) {} + T(bool (*)[23]) : ptr_a(new A(ptr_a->ONE)) {} + T(bool (*)[24]) : ptr_a(new A(ptr_a->TWO)) {} + T(bool (*)[25]) : ptr_a(new A(ptr_a->zero())) {} + + T(bool (*)[26]) : ptr_a(new A(ptr_a->get())) {} // expected-warning {{field 'ptr_a' is uninitialized when used here}} + T(bool (*)[27]) : ptr_a(new A(ptr_a->get2())) {} // expected-warning {{field 'ptr_a' is uninitialized when used here}} + T(bool (*)[28]) : ptr_a(new A(ptr_a->num)) {} // expected-warning {{field 'ptr_a' is uninitialized when used here}} + + T(bool (*)[29]) : c_a(c_a) {} // expected-warning {{field 'c_a' is uninitialized when used here}} + T(bool (*)[30]) : c_a(A(c_a)) {} // expected-warning {{field 'c_a' is uninitialized when used here}} + + T(bool (*)[31]) : a(std::move(a)) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[32]) : a(moveA(std::move(a))) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[33]) : a(A(std::move(a))) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[34]) : a(A(std::move(a))) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[35]) : a2(std::move(x ? a : (37, a2))) {} // expected-warning {{field 'a2' is uninitialized when used here}} + + T(bool (*)[36]) : a(const_refA(a)) {} + T(bool (*)[37]) : a(A(const_refA(a))) {} + + T(bool (*)[38]) : a({a}) {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[39]) : a{a} {} // expected-warning {{field 'a' is uninitialized when used here}} + T(bool (*)[40]) : a({a}) {} // expected-warning {{field 'a' is uninitialized when used here}} +}; + struct B { // POD struct. int x; @@ -383,6 +438,44 @@ B b22 = moveB(std::move(b22)); // expected-warning {{variable 'b22' is uninitia B b23 = B(std::move(b23)); // expected-warning {{variable 'b23' is uninitialized when used within its own initialization}} B b24 = std::move(x ? b23 : (18, b24)); // expected-warning {{variable 'b24' is uninitialized when used within its own initialization}} +class U { + B b1, b2; + B *ptr1, *ptr2; + const B constb = {}; + + U() {} + U(bool (*)[1]) : b1() {} + U(bool (*)[2]) : b2(b1) {} + U(bool (*)[3]) : b1{ 5, &b1.x } {} + U(bool (*)[4]) : b1(getB()) {} + U(bool (*)[5]) : b1(getB(&b1)) {} + U(bool (*)[6]) : b1(getB(&b1.x)) {} + + U(bool (*)[7]) : b1(b1) {} // expected-warning {{field 'b1' is uninitialized when used here}} + U(bool (*)[8]) : b1(getB(b1.x)) {} // expected-warning {{field 'b1' is uninitialized when used here}} + U(bool (*)[9]) : b1(getB(b1.y)) {} // expected-warning {{field 'b1' is uninitialized when used here}} + U(bool (*)[10]) : b1(getB(-b1.x)) {} // expected-warning {{field 'b1' is uninitialized when used here}} + + U(bool (*)[11]) : ptr1(0) {} + U(bool (*)[12]) : ptr1(0), ptr2(ptr1) {} + U(bool (*)[13]) : ptr1(getPtrB()) {} + U(bool (*)[14]) : ptr1(getPtrB(&ptr1)) {} + + U(bool (*)[15]) : ptr1(getPtrB(ptr1->x)) {} // expected-warning {{field 'ptr1' is uninitialized when used here}} + U(bool (*)[16]) : ptr2(getPtrB(ptr2->y)) {} // expected-warning {{field 'ptr2' is uninitialized when used here}} + + U(bool (*)[17]) : b1 { b1.x = 5, b1.y = 0 } {} + U(bool (*)[18]) : b1 { b1.x + 1, b1.y } {} // expected-warning 2{{field 'b1' is uninitialized when used here}} + + U(bool (*)[19]) : constb(constb) {} // expected-warning {{field 'constb' is uninitialized when used here}} + U(bool (*)[20]) : constb(B(constb)) {} // expected-warning {{field 'constb' is uninitialized when used here}} + + U(bool (*)[21]) : b1(std::move(b1)) {} // expected-warning {{field 'b1' is uninitialized when used here}} + U(bool (*)[22]) : b1(moveB(std::move(b1))) {} // expected-warning {{field 'b1' is uninitialized when used here}} + U(bool (*)[23]) : b1(B(std::move(b1))) {} // expected-warning {{field 'b1' is uninitialized when used here}} + U(bool (*)[24]) : b2(std::move(x ? b1 : (18, b2))) {} // expected-warning {{field 'b2' is uninitialized when used here}} +}; + struct C { char a[100], *e; } car = { .e = car.a }; // -- 2.40.0