[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


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@340272 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 70 insertions, 7 deletions

diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
index bcecfb76ce..257186e270 100644
--- a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ b/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
 
diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
index 2281d66847..288412e840 100644
--- a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ b/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;