r175556 - [analyzer] Don't accidentally strip off base object regions for lazy bindings.

Jordan Rose jordan_rose at apple.com
Tue Feb 19 12:28:33 PST 2013


Author: jrose
Date: Tue Feb 19 14:28:33 2013
New Revision: 175556

URL: http://llvm.org/viewvc/llvm-project?rev=175556&view=rev
Log:
[analyzer] Don't accidentally strip off base object regions for lazy bindings.

If a base object is at a 0 offset, RegionStoreManager may find a lazy
binding for the entire object, then try to attach a FieldRegion or
grandparent CXXBaseObjectRegion on top of that (skipping the intermediate
region). We now preserve as many layers of base object regions necessary
to make the types match.

<rdar://problem/13239840>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/test/Analysis/derived-to-base.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=175556&r1=175555&r2=175556&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Tue Feb 19 14:28:33 2013
@@ -497,8 +497,11 @@ public: // Part of public interface to c
                                                   const TypedValueRegion *R,
                                                   QualType Ty);
 
-  /// Get the state and region whose binding this region R corresponds to.
-  std::pair<Store, const MemRegion*>
+  /// Get the state and region whose binding this region \p R corresponds to.
+  ///
+  /// If there is no lazy binding for \p R, the returned value will have a null
+  /// \c second. Note that a null pointer can represents a valid Store.
+  std::pair<Store, const MemRegion *>
   getLazyBinding(RegionBindingsConstRef B, const MemRegion *R,
                  const MemRegion *originalRegion);
 
@@ -1256,49 +1259,68 @@ std::pair<Store, const MemRegion *>
 RegionStoreManager::getLazyBinding(RegionBindingsConstRef B,
                                    const MemRegion *R,
                                    const MemRegion *originalRegion) {
+  typedef std::pair<Store, const MemRegion *> StoreRegionPair;
+
   if (originalRegion != R) {
     if (Optional<SVal> OV = B.getDefaultBinding(R)) {
       if (const nonloc::LazyCompoundVal *V =
-          dyn_cast<nonloc::LazyCompoundVal>(OV.getPointer()))
+            dyn_cast<nonloc::LazyCompoundVal>(OV.getPointer()))
         return std::make_pair(V->getStore(), V->getRegion());
     }
   }
   
   if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
-    const std::pair<Store, const MemRegion *> &X =
-      getLazyBinding(B, ER->getSuperRegion(), originalRegion);
+    StoreRegionPair X = getLazyBinding(B, ER->getSuperRegion(), originalRegion);
 
     if (X.second)
       return std::make_pair(X.first,
                             MRMgr.getElementRegionWithSuper(ER, X.second));
   }
   else if (const FieldRegion *FR = dyn_cast<FieldRegion>(R)) {
-    const std::pair<Store, const MemRegion *> &X =
-      getLazyBinding(B, FR->getSuperRegion(), originalRegion);
+    StoreRegionPair X = getLazyBinding(B, FR->getSuperRegion(), originalRegion);
 
     if (X.second) {
       return std::make_pair(X.first,
                             MRMgr.getFieldRegionWithSuper(FR, X.second));
     }
         
-  }
-  // C++ base object region is another kind of region that we should blast
-  // through to look for lazy compound value. It is like a field region.
-  else if (const CXXBaseObjectRegion *baseReg = 
-            dyn_cast<CXXBaseObjectRegion>(R)) {
-    const std::pair<Store, const MemRegion *> &X =
-      getLazyBinding(B, baseReg->getSuperRegion(), originalRegion);
+  } else if (const CXXBaseObjectRegion *BaseReg =
+               dyn_cast<CXXBaseObjectRegion>(R)) {
+    // C++ base object region is another kind of region that we should blast
+    // through to look for lazy compound value. It is like a field region.
+    StoreRegionPair Result = getLazyBinding(B, BaseReg->getSuperRegion(),
+                                            originalRegion);
     
-    if (X.second) {
-      return std::make_pair(X.first,
-                            MRMgr.getCXXBaseObjectRegionWithSuper(baseReg,
-                                                                  X.second));
+    if (Result.second) {
+      // Make sure the types match up.
+      const CXXRecordDecl *LazyD = 0;
+      if (const TypedValueRegion *LazyR =
+            dyn_cast<TypedValueRegion>(Result.second)) {
+        LazyD = LazyR->getValueType()->getAsCXXRecordDecl();
+        if (LazyD)
+          LazyD = LazyD->getCanonicalDecl();
+      }
+
+      typedef SmallVector<const CXXBaseObjectRegion *, 4> BaseListTy;
+      BaseListTy BaseRegions;
+      do {
+        BaseRegions.push_back(BaseReg);
+        BaseReg = dyn_cast<CXXBaseObjectRegion>(BaseReg->getSuperRegion());
+      } while (BaseReg && LazyD != BaseReg->getDecl()->getCanonicalDecl());
+
+      // Layer the base regions on the result in least-to-most derived order.
+      for (BaseListTy::const_reverse_iterator I = BaseRegions.rbegin(),
+                                              E = BaseRegions.rend();
+           I != E; ++I) {
+        Result.second = MRMgr.getCXXBaseObjectRegionWithSuper(*I,
+                                                              Result.second);
+      }
     }
+
+    return Result;
   }
 
-  // The NULL MemRegion indicates an non-existent lazy binding. A NULL Store is
-  // possible for a valid lazy binding.
-  return std::make_pair((Store) 0, (const MemRegion *) 0);
+  return StoreRegionPair();
 }
 
 SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,

Modified: cfe/trunk/test/Analysis/derived-to-base.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/derived-to-base.cpp?rev=175556&r1=175555&r2=175556&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/derived-to-base.cpp (original)
+++ cfe/trunk/test/Analysis/derived-to-base.cpp Tue Feb 19 14:28:33 2013
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -DCONSTRUCTORS=1 -analyzer-config c++-inlining=constructors -verify %s
 
 void clang_analyzer_eval(bool);
 
@@ -135,3 +136,171 @@ namespace DynamicMultipleInheritanceUpca
     clang_analyzer_eval(testCast(&d)); // expected-warning{{TRUE}}
   }
 }
+
+namespace LazyBindings {
+  struct Base {
+    int x;
+  };
+
+  struct Derived : public Base {
+    int y;
+  };
+
+  struct DoubleDerived : public Derived {
+    int z;
+  };
+
+  int getX(const Base &obj) {
+    return obj.x;
+  }
+
+  int getY(const Derived &obj) {
+    return obj.y;
+  }
+
+  void testDerived() {
+    Derived d;
+    d.x = 1;
+    d.y = 2;
+    clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}}
+
+    Base b(d);
+    clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}}
+
+    Derived d2(d);
+    clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}}
+  }
+
+  void testDoubleDerived() {
+    DoubleDerived d;
+    d.x = 1;
+    d.y = 2;
+    clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}}
+
+    Base b(d);
+    clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}}
+
+    Derived d2(d);
+    clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}}
+
+    DoubleDerived d3(d);
+    clang_analyzer_eval(getX(d3) == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(getY(d3) == 2); // expected-warning{{TRUE}}
+  }
+
+  namespace WithOffset {
+    struct Offset {
+      int padding;
+    };
+
+    struct OffsetDerived : private Offset, public Base {
+      int y;
+    };
+
+    struct DoubleOffsetDerived : public OffsetDerived {
+      int z;
+    };
+
+    int getY(const OffsetDerived &obj) {
+      return obj.y;
+    }
+
+    void testDerived() {
+      OffsetDerived d;
+      d.x = 1;
+      d.y = 2;
+      clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}}
+
+      Base b(d);
+      clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}}
+
+      OffsetDerived d2(d);
+      clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}}
+    }
+
+    void testDoubleDerived() {
+      DoubleOffsetDerived d;
+      d.x = 1;
+      d.y = 2;
+      clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}}
+
+      Base b(d);
+      clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}}
+
+      OffsetDerived d2(d);
+      clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}}
+
+      DoubleOffsetDerived d3(d);
+      clang_analyzer_eval(getX(d3) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d3) == 2); // expected-warning{{TRUE}}
+    }
+  }
+
+  namespace WithVTable {
+    struct DerivedVTBL : public Base {
+      int y;
+      virtual void method();
+    };
+
+    struct DoubleDerivedVTBL : public DerivedVTBL {
+      int z;
+    };
+
+    int getY(const DerivedVTBL &obj) {
+      return obj.y;
+    }
+
+    int getZ(const DoubleDerivedVTBL &obj) {
+      return obj.z;
+    }
+
+    void testDerived() {
+      DerivedVTBL d;
+      d.x = 1;
+      d.y = 2;
+      clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}}
+
+      Base b(d);
+      clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}}
+
+#if CONSTRUCTORS
+      DerivedVTBL d2(d);
+      clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}}
+#endif
+    }
+
+#if CONSTRUCTORS
+    void testDoubleDerived() {
+      DoubleDerivedVTBL d;
+      d.x = 1;
+      d.y = 2;
+      d.z = 3;
+      clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getZ(d) == 3); // expected-warning{{TRUE}}
+
+      Base b(d);
+      clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}}
+
+      DerivedVTBL d2(d);
+      clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}}
+
+      DoubleDerivedVTBL d3(d);
+      clang_analyzer_eval(getX(d3) == 1); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getY(d3) == 2); // expected-warning{{TRUE}}
+      clang_analyzer_eval(getZ(d3) == 3); // expected-warning{{TRUE}}
+    }
+#endif
+  }
+}





More information about the cfe-commits mailing list