[cfe-commits] r161960 - in /cfe/trunk: lib/StaticAnalyzer/Core/CallEvent.cpp lib/StaticAnalyzer/Core/MemRegion.cpp test/Analysis/reinterpret-cast.cpp

[Jordan Rose]

Author: jrose
Date: Wed Aug 15 12:33:34 2012
New Revision: 161960

URL: http://llvm.org/viewvc/llvm-project?rev=161960&view=rev
Log:
[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.

Added:
    cfe/trunk/test/Analysis/reinterpret-cast.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=161960&r1=161959&r2=161960&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Wed Aug 15 12:33:34 2012
@@ -427,16 +427,17 @@
   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 @@
       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));
   }
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=161960&r1=161959&r2=161960&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Aug 15 12:33:34 2012
@@ -1111,7 +1111,7 @@
       }
       
       const CXXRecordDecl *Child = Ty->getAsCXXRecordDecl();
-      if (!Child || !Child->isCompleteDefinition()) {
+      if (!Child) {
         // We cannot compute the offset of the base class.
         SymbolicOffsetBase = R;
       }

Added: cfe/trunk/test/Analysis/reinterpret-cast.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reinterpret-cast.cpp?rev=161960&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/reinterpret-cast.cpp (added)
+++ cfe/trunk/test/Analysis/reinterpret-cast.cpp Wed Aug 15 12:33:34 2012
@@ -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}}
+}