Author: szelethus Date: Tue Aug 21 05:16:59 2018 New Revision: 340272 URL: http://llvm.org/viewvc/llvm-project?rev=340272&view=rev Log: [analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members
For the following example: struct Base { int x; }; // In a different translation unit struct Derived : public Base { Derived() {} }; For a call to Derived::Derived(), we'll receive a note that this->x is uninitialized. Since x is not a direct field of Derived, it could be a little confusing. This patch aims to fix this, as well as the case when the derived object has a field that has the name as an inherited uninitialized data member: struct Base { int x; // note: uninitialized field 'this->Base::x' }; struct Derived : public Base { int x = 5; Derived() {} }; Differential Revision: https://reviews.llvm.org/D50905 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h?rev=340272&r1=340271&r2=340272&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h Tue Aug 21 05:16:59 2018 @@ -32,10 +32,10 @@ class FieldNode { protected: const FieldRegion *FR; - ~FieldNode() = default; + /* non-virtual */ ~FieldNode() = default; public: - FieldNode(const FieldRegion *FR) : FR(FR) { assert(FR); } + FieldNode(const FieldRegion *FR) : FR(FR) {} FieldNode() = delete; FieldNode(const FieldNode &) = delete; @@ -47,11 +47,21 @@ public: /// FoldingSet. void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(this); } - bool operator<(const FieldNode &Other) const { return FR < Other.FR; } - bool isSameRegion(const FieldRegion *OtherFR) const { return FR == OtherFR; } + // Helper method for uniqueing. + bool isSameRegion(const FieldRegion *OtherFR) const { + // Special FieldNode descendants may wrap nullpointers -- we wouldn't like + // to unique these objects. + if (FR == nullptr) + return false; + + return FR == OtherFR; + } const FieldRegion *getRegion() const { return FR; } - const FieldDecl *getDecl() const { return FR->getDecl(); } + const FieldDecl *getDecl() const { + assert(FR); + return FR->getDecl(); + } // When a fieldchain is printed (a list of FieldNode objects), it will have // the following format: @@ -71,6 +81,8 @@ public: /// Print the separator. For example, fields may be separated with '.' or /// "->". virtual void printSeparator(llvm::raw_ostream &Out) const = 0; + + virtual bool isBase() const { return false; } }; /// Returns with Field's name. This is a helper function to get the correct name @@ -94,15 +106,24 @@ private: FieldChain::Factory &ChainFactory; FieldChain Chain; + FieldChainInfo(FieldChain::Factory &F, FieldChain NewChain) + : FieldChainInfo(F) { + Chain = NewChain; + } + public: FieldChainInfo() = delete; FieldChainInfo(FieldChain::Factory &F) : ChainFactory(F) {} FieldChainInfo(const FieldChainInfo &Other) = default; template <class FieldNodeT> FieldChainInfo add(const FieldNodeT &FN); + template <class FieldNodeT> FieldChainInfo replaceHead(const FieldNodeT &FN); bool contains(const FieldRegion *FR) const; + bool isEmpty() const { return Chain.isEmpty(); } + const FieldRegion *getUninitRegion() const; + const FieldNode &getHead() { return Chain.getHead(); } void printNoteMsg(llvm::raw_ostream &Out) const; }; @@ -250,6 +271,12 @@ inline FieldChainInfo FieldChainInfo::ad return NewChain; } +template <class FieldNodeT> +inline FieldChainInfo FieldChainInfo::replaceHead(const FieldNodeT &FN) { + FieldChainInfo NewChain(ChainFactory, Chain.getTail()); + return NewChain.add(FN); +} + } // end of namespace ento } // end of namespace clang Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340272&r1=340271&r2=340272&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Tue Aug 21 05:16:59 2018 @@ -93,6 +93,33 @@ public: } }; +/// Represents that the FieldNode that comes after this is declared in a base +/// of the previous FieldNode. +class BaseClass final : public FieldNode { + const QualType BaseClassT; + +public: + BaseClass(const QualType &T) : FieldNode(nullptr), BaseClassT(T) { + assert(!T.isNull()); + assert(T->getAsCXXRecordDecl()); + } + + virtual void printNoteMsg(llvm::raw_ostream &Out) const override { + llvm_unreachable("This node can never be the final node in the " + "fieldchain!"); + } + + virtual void printPrefix(llvm::raw_ostream &Out) const override {} + + virtual void printNode(llvm::raw_ostream &Out) const override { + Out << BaseClassT->getAsCXXRecordDecl()->getName() << "::"; + } + + virtual void printSeparator(llvm::raw_ostream &Out) const override {} + + virtual bool isBase() const { return true; } +}; + } // end of anonymous namespace // Utility function declarations. @@ -295,8 +322,17 @@ bool FindUninitializedFields::isNonUnion .castAs<loc::MemRegionVal>() .getRegionAs<TypedValueRegion>(); - if (isNonUnionUninit(BaseRegion, LocalChain)) - ContainsUninitField = true; + // If the head of the list is also a BaseClass, we'll overwrite it to avoid + // note messages like 'this->A::B::x'. + if (!LocalChain.isEmpty() && LocalChain.getHead().isBase()) { + if (isNonUnionUninit(BaseRegion, LocalChain.replaceHead( + BaseClass(BaseSpec.getType())))) + ContainsUninitField = true; + } else { + if (isNonUnionUninit(BaseRegion, + LocalChain.add(BaseClass(BaseSpec.getType())))) + ContainsUninitField = true; + } } return ContainsUninitField; Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp?rev=340272&r1=340271&r2=340272&view=diff ============================================================================== --- cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp (original) +++ cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp Tue Aug 21 05:16:59 2018 @@ -35,7 +35,7 @@ void fNonPolymorphicInheritanceTest1() { } class NonPolymorphicBaseClass2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass2::x'}} protected: int y; @@ -62,7 +62,7 @@ class NonPolymorphicBaseClass3 { int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass3::y'}} public: NonPolymorphicBaseClass3() = default; NonPolymorphicBaseClass3(int) : x(7) {} @@ -140,7 +140,7 @@ void fPolymorphicInheritanceTest1() { } class PolymorphicRight1 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->PolymorphicRight1::x'}} protected: int y; @@ -168,7 +168,7 @@ class PolymorphicBaseClass3 { int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->PolymorphicBaseClass3::y'}} public: virtual ~PolymorphicBaseClass3() = default; PolymorphicBaseClass3() = default; @@ -248,7 +248,7 @@ void fVirtualInheritanceTest1() { } class VirtualPolymorphicRight1 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualPolymorphicRight1::x'}} protected: int y; @@ -276,7 +276,7 @@ class VirtualPolymorphicBaseClass3 { int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->VirtualPolymorphicBaseClass3::y'}} public: virtual ~VirtualPolymorphicBaseClass3() = default; VirtualPolymorphicBaseClass3() = default; @@ -358,7 +358,7 @@ struct Left2 { Left2(int) : x(36) {} }; struct Right2 { - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->Right2::y'}} Right2() = default; Right2(int) : y(37) {} }; @@ -378,7 +378,7 @@ void fMultipleInheritanceTest2() { } struct Left3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->Left3::x'}} Left3() = default; Left3(int) : x(39) {} }; @@ -433,7 +433,7 @@ struct Left5 { Left5(int) : x(44) {} }; struct Right5 { - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->Right5::y'}} Right5() = default; Right5(int) : y(45) {} }; @@ -514,7 +514,7 @@ void fNonVirtualDiamondInheritanceTest1( } struct NonVirtualBase2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase2::x'}} NonVirtualBase2() = default; NonVirtualBase2(int) : x(52) {} }; @@ -542,7 +542,7 @@ void fNonVirtualDiamondInheritanceTest2( } struct NonVirtualBase3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase3::x'}} NonVirtualBase3() = default; NonVirtualBase3(int) : x(54) {} }; @@ -570,8 +570,8 @@ void fNonVirtualDiamondInheritanceTest3( } struct NonVirtualBase4 { - int x; // expected-note{{uninitialized field 'this->x'}} - // expected-note@-1{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase4::x'}} + // expected-note@-1{{uninitialized field 'this->NonVirtualBase4::x'}} NonVirtualBase4() = default; NonVirtualBase4(int) : x(56) {} }; @@ -626,7 +626,7 @@ void fNonVirtualDiamondInheritanceTest5( } struct NonVirtualBase6 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase6::x'}} NonVirtualBase6() = default; NonVirtualBase6(int) : x(59) {} }; @@ -712,7 +712,7 @@ void fVirtualDiamondInheritanceTest1() { } struct VirtualBase2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualBase2::x'}} VirtualBase2() = default; VirtualBase2(int) : x(63) {} }; @@ -751,7 +751,7 @@ void fVirtualDiamondInheritanceTest2() { } struct VirtualBase3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualBase3::x'}} VirtualBase3() = default; VirtualBase3(int) : x(66) {} }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits