r177453 - [analyzer] Do not believe lazy binding when symbolic region types do not match

Anna Zaks ganna at apple.com
Tue Mar 19 15:38:10 PDT 2013


Author: zaks
Date: Tue Mar 19 17:38:09 2013
New Revision: 177453

URL: http://llvm.org/viewvc/llvm-project?rev=177453&view=rev
Log:
[analyzer] Do not believe lazy binding when symbolic region types do not match

This fixes a crash when analyzing LLVM that was exposed by r177220 (modeling of
trivial copy/move assignment operators).

When we look up a lazy binding for “Builder”, we see the direct binding of Loc at offset 0.
Previously, we believed the binding, which led to a crash. Now, we do not believe it as
the types do not match.

Added:
    cfe/trunk/test/Analysis/region-store.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/test/Analysis/uninit-vals.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=177453&r1=177452&r2=177453&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Tue Mar 19 17:38:09 2013
@@ -1265,6 +1265,17 @@ SVal RegionStoreManager::getBinding(Regi
   return svalBuilder.getRegionValueSymbolVal(R);
 }
 
+static QualType getUnderlyingType(const SubRegion *R) {
+  QualType RegionTy;
+  if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(R))
+    RegionTy = TVR->getValueType();
+
+  if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R))
+    RegionTy = SR->getSymbol()->getType();
+
+  return RegionTy;
+}
+
 /// Checks to see if store \p B has a lazy binding for region \p R.
 ///
 /// If \p AllowSubregionBindings is \c false, a lazy binding will be rejected
@@ -1283,11 +1294,11 @@ getExistingLazyBinding(SValBuilder &SVB,
   if (!LCV)
     return None;
 
-  // If the LCV is for a subregion, the types won't match, and we shouldn't
-  // reuse the binding. Unfortuately we can only check this if the destination
-  // region is a TypedValueRegion.
-  if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(R)) {
-    QualType RegionTy = TVR->getValueType();
+  // If the LCV is for a subregion, the types might not match, and we shouldn't
+  // reuse the binding.
+  QualType RegionTy = getUnderlyingType(R);
+  if (!RegionTy.isNull() &&
+      !RegionTy->isVoidPointerType()) {
     QualType SourceRegionTy = LCV->getRegion()->getValueType();
     if (!SVB.getContext().hasSameUnqualifiedType(RegionTy, SourceRegionTy))
       return None;

Added: cfe/trunk/test/Analysis/region-store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/region-store.cpp?rev=177453&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/region-store.cpp (added)
+++ cfe/trunk/test/Analysis/region-store.cpp Tue Mar 19 17:38:09 2013
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix -verify %s
+// expected-no-diagnostics
+
+class Loc {
+  int x;
+};
+class P1 {
+public:
+  Loc l;
+  void setLoc(Loc L) {
+    l = L;
+  }
+  
+};
+class P2 {
+public:
+  int m;
+  int accessBase() {
+    return m;
+  }
+};
+class Derived: public P1, public P2 {
+};
+int radar13445834(Derived *Builder, Loc l) {
+  Builder->setLoc(l);
+  return Builder->accessBase();
+  
+}
\ No newline at end of file

Modified: cfe/trunk/test/Analysis/uninit-vals.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.m?rev=177453&r1=177452&r2=177453&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/uninit-vals.m (original)
+++ cfe/trunk/test/Analysis/uninit-vals.m Tue Mar 19 17:38:09 2013
@@ -80,12 +80,8 @@ void PR14765_incorrectBehavior(Circle *t
 
   testObj->origin = makePoint(0.0, 0.0);
 
-  // FIXME: Assigning to 'testObj->origin' kills the default binding for the
-  // whole region, meaning that we've forgotten that testObj->size should also
-  // default to 0. Tracked by <rdar://problem/12701038>.
-  // This should be TRUE.
-  clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{UNKNOWN}}
-
+  clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{TRUE}}
+  
   free(testObj);
 }
 





More information about the cfe-commits mailing list