[analyzer] Be more careful when downcasting for devirtualization.

Virtual base regions are never layered, so simply stripping them off won't
necessarily get you to the correct casted class. Instead, what we want is
the same logic for evaluating dynamic_cast: strip off base regions if possible,
but add new base regions if necessary.

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

2 files changed, 66 insertions, 12 deletions

diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index 6a5faa054c..cacd347442 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -419,21 +419,18 @@ void CXXInstanceCall::getInitialStackFrameContents(
     Loc ThisLoc = SVB.getCXXThis(MD, CalleeCtx);
 
     if (const MemRegion *ThisReg = ThisVal.getAsRegion()) {
+      ASTContext &Ctx = SVB.getContext();
       const CXXRecordDecl *Class = MD->getParent();
+      QualType Ty = Ctx.getPointerType(Ctx.getRecordType(Class));
 
-      // We may be downcasting to call a devirtualized virtual method.
-      // Search through the base casts we already have to see if we can just
-      // strip them off.
-      const CXXBaseObjectRegion *BaseReg;
-      while ((BaseReg = dyn_cast<CXXBaseObjectRegion>(ThisReg))) {
-        if (BaseReg->getDecl() == Class)
-          break;
-        ThisReg = BaseReg->getSuperRegion();
-      }
+      // FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
+      bool Failed;
+      ThisVal = StateMgr.getStoreManager().evalDynamicCast(ThisVal, Ty, Failed);
+      assert(!Failed && "Calling an incorrectly devirtualized method");
 
-      // Either we found the right base class, or we stripped all the casts to
-      // the most derived type. Either one is good.
-      ThisVal = loc::MemRegionVal(ThisReg);
+      // If we couldn't build the correct cast, just strip off all casts.
+      if (ThisVal.isUnknown())
+        ThisVal = loc::MemRegionVal(ThisReg->StripCasts());
     }
 
     Bindings.push_back(std::make_pair(ThisLoc, ThisVal));
diff --git a/test/Analysis/inline.cpp b/test/Analysis/inline.cpp
index 9a867849e5..4eaed9fed1 100644
--- a/test/Analysis/inline.cpp
+++ b/test/Analysis/inline.cpp
@@ -106,6 +106,63 @@ namespace PR13569 {
     // Don't crash when inlining and devirtualizing.
     x.interface();
   }
+
+
+  class Grandchild : public Child {};
+
+  void testDevirtualizeToMiddle() {
+    Grandchild x;
+    x.m_child = 42;
+
+    // Don't crash when inlining and devirtualizing.
+    x.interface();
+  }
 }
 
+namespace PR13569_virtual {
+  class Parent {
+  protected:
+    int m_parent;
+    virtual int impl() const = 0;
 
+    Parent() : m_parent(0) {}
+
+  public:
+    int interface() const {
+      clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+      return impl();
+    }
+  };
+
+  class Child : virtual public Parent {
+  protected:
+    virtual int impl() const {
+      clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+      return m_parent + m_child;
+    }
+
+  public:
+    Child() : m_child(0) {}
+
+    int m_child;
+  };
+
+  void testVirtual() {
+    Child x;
+    x.m_child = 42;
+
+    // Don't crash when inlining and devirtualizing.
+    x.interface();
+  }
+
+
+  class Grandchild : virtual public Child {};
+
+  void testDevirtualizeToMiddle() {
+    Grandchild x;
+    x.m_child = 42;
+
+    // Don't crash when inlining and devirtualizing.
+    x.interface();
+  }
+}