From f9e9a9f41bef7b0200deefa27786bb3b8e25b25d Mon Sep 17 00:00:00 2001
From: Jordan Rose <jordan_rose@apple.com>
Date: Mon, 25 Feb 2013 18:36:15 +0000
Subject: [PATCH] [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.

llvm-svn: 176034
---
 clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 32 +++++++++++++++++++++
 clang/test/Analysis/reinterpret-cast.cpp    | 22 ++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index e1b8b186c7e8..12e43537aa3e 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1092,6 +1092,23 @@ RegionRawOffset ElementRegion::getAsArrayOffset() const {
   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() const {
       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() const {
         // 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() const {
         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)
diff --git a/clang/test/Analysis/reinterpret-cast.cpp b/clang/test/Analysis/reinterpret-cast.cpp
index d1aed80a0c35..59e6a539a11f 100644
--- a/clang/test/Analysis/reinterpret-cast.cpp
+++ b/clang/test/Analysis/reinterpret-cast.cpp
@@ -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}}
+  };
+}