From 3630c399727acf2936ea14cb2d51e8b1e747bfbd Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Fri, 21 Nov 2014 03:10:30 +0000 Subject: [PATCH] Extend -Wuninitialized to warn when accessing uninitialized base classes in a constructor. llvm-svn: 222503 --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 ++ clang/lib/Sema/SemaDeclCXX.cpp | 53 ++++++++++++++++++------ clang/test/SemaCXX/uninitialized.cpp | 39 +++++++++++++++++ 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index aa93e63..bd95c50 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -1496,6 +1496,9 @@ def note_uninit_reference_member : Note< "uninitialized reference member is here">; def warn_field_is_uninit : Warning<"field %0 is uninitialized when used here">, InGroup; +def warn_base_class_is_uninit : Warning< + "base class %0 is uninitialized when used here to access %q1">, + InGroup; def warn_reference_field_is_uninit : Warning< "reference %0 is not yet bound to a value when used here">, InGroup; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index f23d1a0..90ca97b 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -2237,6 +2237,9 @@ namespace { // List of Decls to generate a warning on. Also remove Decls that become // initialized. llvm::SmallPtrSetImpl &Decls; + // List of base classes of the record. Classes are removed after their + // initializers. + llvm::SmallPtrSetImpl &BaseClasses; // Vector of decls to be removed from the Decl set prior to visiting the // nodes. These Decls may have been initialized in the prior initializer. llvm::SmallVector DeclsToRemove; @@ -2252,9 +2255,10 @@ namespace { public: typedef EvaluatedExprVisitor Inherited; UninitializedFieldVisitor(Sema &S, - llvm::SmallPtrSetImpl &Decls) - : Inherited(S.Context), S(S), Decls(Decls), Constructor(nullptr), - InitList(false), InitListFieldDecl(nullptr) {} + llvm::SmallPtrSetImpl &Decls, + llvm::SmallPtrSetImpl &BaseClasses) + : Inherited(S.Context), S(S), Decls(Decls), BaseClasses(BaseClasses), + Constructor(nullptr), InitList(false), InitListFieldDecl(nullptr) {} // Returns true if the use of ME is not an uninitialized use. bool IsInitListMemberExprInitialized(MemberExpr *ME, @@ -2309,7 +2313,8 @@ namespace { bool AllPODFields = FieldME->getType().isPODType(S.Context); Expr *Base = ME; - while (MemberExpr *SubME = dyn_cast(Base)) { + while (MemberExpr *SubME = + dyn_cast(Base->IgnoreParenImpCasts())) { if (isa(SubME->getMemberDecl())) return; @@ -2321,10 +2326,10 @@ namespace { if (!FieldME->getType().isPODType(S.Context)) AllPODFields = false; - Base = SubME->getBase()->IgnoreParenImpCasts(); + Base = SubME->getBase(); } - if (!isa(Base)) + if (!isa(Base->IgnoreParenImpCasts())) return; if (AddressOf && AllPODFields) @@ -2332,6 +2337,21 @@ namespace { ValueDecl* FoundVD = FieldME->getMemberDecl(); + if (ImplicitCastExpr *BaseCast = dyn_cast(Base)) { + while (isa(BaseCast->getSubExpr())) { + BaseCast = cast(BaseCast->getSubExpr()); + } + + if (BaseCast->getCastKind() == CK_UncheckedDerivedToBase) { + QualType T = BaseCast->getType(); + if (T->isPointerType() && + BaseClasses.count(T->getPointeeType())) { + S.Diag(FieldME->getExprLoc(), diag::warn_base_class_is_uninit) + << T->getPointeeType() << FoundVD; + } + } + } + if (!Decls.count(FoundVD)) return; @@ -2420,7 +2440,7 @@ namespace { } void CheckInitializer(Expr *E, const CXXConstructorDecl *FieldConstructor, - FieldDecl *Field) { + FieldDecl *Field, const Type *BaseClass) { // Remove Decls that may have been initialized in the previous // initializer. for (ValueDecl* VD : DeclsToRemove) @@ -2442,6 +2462,8 @@ namespace { if (Field) Decls.erase(Field); + if (BaseClass) + BaseClasses.erase(BaseClass->getCanonicalTypeInternal()); } void VisitMemberExpr(MemberExpr *ME) { @@ -2578,14 +2600,19 @@ namespace { } } - if (UninitializedFields.empty()) + llvm::SmallPtrSet UninitializedBaseClasses; + for (auto I : RD->bases()) + UninitializedBaseClasses.insert(I.getType().getCanonicalType()); + + if (UninitializedFields.empty() && UninitializedBaseClasses.empty()) return; UninitializedFieldVisitor UninitializedChecker(SemaRef, - UninitializedFields); + UninitializedFields, + UninitializedBaseClasses); for (const auto *FieldInit : Constructor->inits()) { - if (UninitializedFields.empty()) + if (UninitializedFields.empty() && UninitializedBaseClasses.empty()) break; Expr *InitExpr = FieldInit->getInit(); @@ -2599,10 +2626,12 @@ namespace { continue; // In class initializers will point to the constructor. UninitializedChecker.CheckInitializer(InitExpr, Constructor, - FieldInit->getAnyMember()); + FieldInit->getAnyMember(), + FieldInit->getBaseClass()); } else { UninitializedChecker.CheckInitializer(InitExpr, nullptr, - FieldInit->getAnyMember()); + FieldInit->getAnyMember(), + FieldInit->getBaseClass()); } } } diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp index 96dc011..ac08183 100644 --- a/clang/test/SemaCXX/uninitialized.cpp +++ b/clang/test/SemaCXX/uninitialized.cpp @@ -1302,3 +1302,42 @@ C c; // expected-note@-1 {{in instantiation of member function 'template_class::C::C' requested here}} } + +namespace base_class_access { +struct A { + A(); + A(int); + + int i; + int foo(); + + static int bar(); +}; + +struct B : public A { + B(int (*)[1]) : A() {} + B(int (*)[2]) : A(bar()) {} + + B(int (*)[3]) : A(i) {} + // expected-warning@-1 {{base class 'base_class_access::A' is uninitialized when used here to access 'base_class_access::A::i'}} + + B(int (*)[4]) : A(foo()) {} + // expected-warning@-1 {{base_class_access::A' is uninitialized when used here to access 'base_class_access::A::foo'}} +}; + +struct C { + C(int) {} +}; + +struct D : public C, public A { + D(int (*)[1]) : C(0) {} + D(int (*)[2]) : C(bar()) {} + + D(int (*)[3]) : C(i) {} + // expected-warning@-1 {{base class 'base_class_access::A' is uninitialized when used here to access 'base_class_access::A::i'}} + + D(int (*)[4]) : C(foo()) {} + // expected-warning@-1 {{base_class_access::A' is uninitialized when used here to access 'base_class_access::A::foo'}} +}; + +} -- 2.7.4