r176034 - [analyzer] Base regions may be invalid when layered on symbolic regions.

[Jordan Rose]

Author: jrose
Date: Mon Feb 25 12:36:15 2013
New Revision: 176034

URL: http://llvm.org/viewvc/llvm-project?rev=176034&view=rev
Log:
[analyzer] Base regions may be invalid when layered on symbolic regions.

While RegionStore checks to make sure casts on TypedValueRegions are valid,
it does not do the same for SymbolicRegions, which do not have perfect type
info anyway. Additionally, MemRegion::getAsOffset does not take a
ProgramState, so it can't use dynamic type info to determine a better type
for the regions. (This could also be dangerous if the type of a super-region
changes!)

Account for this by checking that a base object region is valid on top of a
symbolic region, and falling back to "symbolic offset" mode if not.

Fixes PR15345.

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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=176034&r1=176033&r2=176034&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Mon Feb 25 12:36:15 2013
@@ -1092,6 +1092,23 @@ RegionRawOffset ElementRegion::getAsArra
   return RegionRawOffset(superR, offset);
 }
 
+
+/// Returns true if \p Base is an immediate base class of \p Child
+static bool isImmediateBase(const CXXRecordDecl *Child,
+                            const CXXRecordDecl *Base) {
+  // Note that we do NOT canonicalize the base class here, because
+  // ASTRecordLayout doesn't either. If that leads us down the wrong path,
+  // so be it; at least we won't crash.
+  for (CXXRecordDecl::base_class_const_iterator I = Child->bases_begin(),
+                                                E = Child->bases_end();
+       I != E; ++I) {
+    if (I->getType()->getAsCXXRecordDecl() == Base)
+      return true;
+  }
+
+  return false;
+}
+
 RegionOffset MemRegion::getAsOffset() const {
   const MemRegion *R = this;
   const MemRegion *SymbolicOffsetBase = 0;
@@ -1145,6 +1162,7 @@ RegionOffset MemRegion::getAsOffset() co
       R = BOR->getSuperRegion();
 
       QualType Ty;
+      bool RootIsSymbolic = false;
       if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(R)) {
         Ty = TVR->getDesugaredValueType(getContext());
       } else if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R)) {
@@ -1152,6 +1170,7 @@ RegionOffset MemRegion::getAsOffset() co
         // Pretend the type of the symbol is the true dynamic type.
         // (This will at least be self-consistent for the life of the symbol.)
         Ty = SR->getSymbol()->getType()->getPointeeType();
+        RootIsSymbolic = true;
       }
       
       const CXXRecordDecl *Child = Ty->getAsCXXRecordDecl();
@@ -1160,6 +1179,19 @@ RegionOffset MemRegion::getAsOffset() co
         SymbolicOffsetBase = R;
       }
 
+      if (RootIsSymbolic) {
+        // Base layers on symbolic regions may not be type-correct.
+        // Double-check the inheritance here, and revert to a symbolic offset
+        // if it's invalid (e.g. due to a reinterpret_cast).
+        if (BOR->isVirtual()) {
+          if (!Child->isVirtuallyDerivedFrom(BOR->getDecl()))
+            SymbolicOffsetBase = R;
+        } else {
+          if (!isImmediateBase(Child, BOR->getDecl()))
+            SymbolicOffsetBase = R;
+        }
+      }
+
       // Don't bother calculating precise offsets if we already have a
       // symbolic offset somewhere in the chain.
       if (SymbolicOffsetBase)

Modified: cfe/trunk/test/Analysis/reinterpret-cast.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reinterpret-cast.cpp?rev=176034&r1=176033&r2=176034&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/reinterpret-cast.cpp (original)
+++ cfe/trunk/test/Analysis/reinterpret-cast.cpp Mon Feb 25 12:36:15 2013
@@ -64,3 +64,25 @@ namespace rdar13249297 {
     clang_analyzer_eval(reinterpret_cast<IntWrapperSubclass *>(ww)->x == 42); // expected-warning{{FALSE}}
   }
 }
+
+namespace PR15345 {
+  class C {};
+
+  class Base {
+  public:
+    void (*f)();
+    int x;
+  };
+
+  class Derived : public Base {};
+
+  void test() {
+	Derived* p;
+	*(reinterpret_cast<void**>(&p)) = new C;
+	p->f();
+
+    // We should still be able to do some reasoning about bindings.
+    p->x = 42;
+    clang_analyzer_eval(p->x == 42); // expected-warning{{TRUE}}
+  };
+}