[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

llvm-svn: 340272

clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h

@@ -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::add(const FieldNodeT &FN) {
   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
 

clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

@@ -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::isNonUnionUninit(const TypedValueRegion *R,
                                  .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;

clang/test/Analysis/cxx-uninitialized-object-inheritance.cpp

@@ -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) {}
 };