[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

diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index 6a5faa054ceb04aff30957915e068cd45c2e651c..cacd3474424f853b0a4018a08498e25c0903f116 100644 (file)
--- 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 9a867849e5ca938b4489ec9566ed0f22c47af0f0..4eaed9fed13c5b23eceda957bfc9b4e5f09dd0d0 100644 (file)
--- 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();
+  }
+}