[analyzer] Only adjust the type of 'this' when we devirtualize a method call.

With reinterpret_cast, we can get completely unrelated types in a region
hierarchy together; this was resulting in CXXBaseObjectRegions being layered
directly on an (untyped) SymbolicRegion, whose symbol was from a completely
different type hierarchy. This was what was causing the internal buildbot to
fail.

Reverts r161911, which merely masked the problem.

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

3 files changed, 27 insertions, 9 deletions

diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index db48fba60e..c625f24e07 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -427,16 +427,17 @@ void CXXInstanceCall::getInitialStackFrameContents(
   AnyFunctionCall::getInitialStackFrameContents(CalleeCtx, Bindings);
 
   // Handle the binding of 'this' in the new stack frame.
-  // We need to make sure we have the proper layering of CXXBaseObjectRegions.
   SVal ThisVal = getCXXThisVal();
   if (!ThisVal.isUnknown()) {
     ProgramStateManager &StateMgr = getState()->getStateManager();
     SValBuilder &SVB = StateMgr.getSValBuilder();
-    
+
     const CXXMethodDecl *MD = cast<CXXMethodDecl>(CalleeCtx->getDecl());
     Loc ThisLoc = SVB.getCXXThis(MD, CalleeCtx);
 
-    if (const MemRegion *ThisReg = ThisVal.getAsRegion()) {
+    // If we devirtualized to a different member function, we need to make sure
+    // we have the proper layering of CXXBaseObjectRegions.
+    if (MD->getCanonicalDecl() != getDecl()->getCanonicalDecl()) {
       ASTContext &Ctx = SVB.getContext();
       const CXXRecordDecl *Class = MD->getParent();
       QualType Ty = Ctx.getPointerType(Ctx.getRecordType(Class));
@@ -445,13 +446,10 @@ void CXXInstanceCall::getInitialStackFrameContents(
       bool Failed;
       ThisVal = StateMgr.getStoreManager().evalDynamicCast(ThisVal, Ty, Failed);
       assert(!Failed && "Calling an incorrectly devirtualized method");
-
-      // 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));
+    if (!ThisVal.isUnknown())
+      Bindings.push_back(std::make_pair(ThisLoc, ThisVal));
   }
 }
 
diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp
index 4f33bad492..62e602a7e1 100644
--- a/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1111,7 +1111,7 @@ RegionOffset MemRegion::getAsOffset() const {
       }
       
       const CXXRecordDecl *Child = Ty->getAsCXXRecordDecl();
-      if (!Child || !Child->isCompleteDefinition()) {
+      if (!Child) {
         // We cannot compute the offset of the base class.
         SymbolicOffsetBase = R;
       }
diff --git a/test/Analysis/reinterpret-cast.cpp b/test/Analysis/reinterpret-cast.cpp
new file mode 100644
index 0000000000..73f2e2de73
--- /dev/null
+++ b/test/Analysis/reinterpret-cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=inlining -verify %s
+
+void clang_analyzer_eval(bool);
+
+typedef struct Opaque *Data;
+struct IntWrapper {
+  int x;
+};
+
+struct Child : public IntWrapper {
+  void set() { x = 42; }
+};
+
+void test(Data data) {
+  Child *wrapper = reinterpret_cast<Child*>(data);
+  // Don't crash when upcasting here.
+  // We don't actually know if 'data' is a Child.
+  wrapper->set();
+  clang_analyzer_eval(wrapper->x == 42); // expected-warning{{TRUE}}
+}