[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.

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

diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp
index e1b8b186c7e867c661ffcbc4be5187549b518843..12e43537aa3e82048e4a1a92e1e09b3a6c67a420 100644 (file)
--- a/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/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/test/Analysis/reinterpret-cast.cpp b/test/Analysis/reinterpret-cast.cpp
index d1aed80a0c35efb52e7539eb553a910f1ee15669..59e6a539a11f4673f57c7d43583f2414720b0e9b 100644 (file)
--- a/test/Analysis/reinterpret-cast.cpp
+++ b/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}}
+  };
+}