[cfe-commits] r60990 - in /cfe/trunk: include/clang/Analysis/PathSensitive/MemRegion.h lib/Analysis/BasicStore.cpp lib/Analysis/GRExprEngine.cpp lib/Analysis/MemRegion.cpp lib/Analysis/RegionStore.cpp test/Analysis/rdar-6442306-1.m

Ted Kremenek kremenek at apple.com
Sat Dec 13 11:24:37 PST 2008


Author: kremenek
Date: Sat Dec 13 13:24:37 2008
New Revision: 60990

URL: http://llvm.org/viewvc/llvm-project?rev=60990&view=rev
Log:
A series of cleanups/fixes motivated by <rdar://problem/6442306>:

GRExprEngine (VisitCast):
- When using StoreManager::CastRegion, always use the state and value it returns to generate the next node.  Failure to do so means that region values returned that don't require the state to be modified will get ignored.

MemRegion:
- Tighten the interface for ElementRegion.  Now ElementRegion can only be created with a super region that is a 'TypedRegion' instead of any MemRegion.  Code in BasicStoreManager/RegionStoreManager already assumed this, but it would result in a dynamic assertion check (and crash) rather than just having the compiler forbid the construction of such regions.
- Added ElementRegion::getArrayRegion() to return the 'typed version' of an ElementRegion's super region.
- Removed bogus assertion in ElementRegion::getType() that assumed that the super region was an AnonTypedRegion.  All that matters is that it is a TypedRegion, which is now true all the time by design.

BasicStore:
- Modified getLValueElement() to check if the 'array' region is a TypedRegion before creating an ElementRegion.  This conforms to the updated interface for ElementRegion.

RegionStore:
- In ArrayToPointer() gracefully handle things we don't reason about, and only create an ElementRegion if the array region is indeed a TypedRegion.

Added:
    cfe/trunk/test/Analysis/rdar-6442306-1.m
Modified:
    cfe/trunk/include/clang/Analysis/PathSensitive/MemRegion.h
    cfe/trunk/lib/Analysis/BasicStore.cpp
    cfe/trunk/lib/Analysis/GRExprEngine.cpp
    cfe/trunk/lib/Analysis/MemRegion.cpp
    cfe/trunk/lib/Analysis/RegionStore.cpp

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/MemRegion.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/MemRegion.h?rev=60990&r1=60989&r2=60990&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/MemRegion.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/MemRegion.h Sat Dec 13 13:24:37 2008
@@ -424,7 +424,7 @@
            cast<nonloc::ConcreteInt>(&Idx)->getValue().isSigned()) &&
            "The index must be signed");
   }
-
+  
   static void ProfileRegion(llvm::FoldingSetNodeID& ID, SVal Idx, 
                             const MemRegion* superRegion);
 
@@ -434,6 +434,13 @@
 
   QualType getType(ASTContext&) const;
 
+  /// getArrayRegion - Return the region of the enclosing array.  This is
+  ///  the same as getSuperRegion() except that this returns a TypedRegion*
+  ///  instead of a MemRegion*.
+  const TypedRegion* getArrayRegion() const {
+    return cast<TypedRegion>(getSuperRegion());
+  }
+  
   void print(llvm::raw_ostream& os) const;
 
   void Profile(llvm::FoldingSetNodeID& ID) const;
@@ -500,13 +507,14 @@
   ///  a specified VarDecl.
   VarRegion* getVarRegion(const VarDecl* vd);
   
-  ElementRegion* getElementRegion(SVal Idx, const MemRegion* superRegion);
+  ElementRegion* getElementRegion(SVal Idx, const TypedRegion* superRegion);
 
   /// getFieldRegion - Retrieve or create the memory region associated with
   ///  a specified FieldDecl.  'superRegion' corresponds to the containing
   ///  memory region (which typically represents the memory representing
   ///  a structure or class).
-  FieldRegion* getFieldRegion(const FieldDecl* fd, const MemRegion* superRegion);
+  FieldRegion* getFieldRegion(const FieldDecl* fd,
+                              const MemRegion* superRegion);
   
   /// getObjCObjectRegion - Retrieve or create the memory region associated with
   ///  the instance of a specified Objective-C class.

Modified: cfe/trunk/lib/Analysis/BasicStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BasicStore.cpp?rev=60990&r1=60989&r2=60990&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/BasicStore.cpp (original)
+++ cfe/trunk/lib/Analysis/BasicStore.cpp Sat Dec 13 13:24:37 2008
@@ -171,7 +171,7 @@
     return Base;
   
   Loc BaseL = cast<Loc>(Base);  
-  const MemRegion* BaseR = 0;
+  const TypedRegion* BaseR = 0;
   
   switch(BaseL.getSubKind()) {
     case loc::SymbolValKind: {
@@ -194,9 +194,19 @@
       // Technically we can get here if people do funny things with casts.
       return UndefinedVal();
       
-    case loc::MemRegionKind:
-      BaseR = cast<loc::MemRegionVal>(BaseL).getRegion();
+    case loc::MemRegionKind: {
+      const MemRegion *R = cast<loc::MemRegionVal>(BaseL).getRegion();
+      if (const TypedRegion *TR = dyn_cast<TypedRegion>(R)) {
+        BaseR = TR;
+        break;
+      }
+      
+      // FIXME: Handle SymbolRegions?  Shouldn't be possible in 
+      // BasicStoreManager.
+      assert(!isa<SymbolicRegion>(R));
+      
       break;
+    }
       
     case loc::ConcreteIntKind:
       // While these seem funny, this can happen through casts.
@@ -210,9 +220,10 @@
       return Base;
   }
   
-  // We return an "unknown" index because we aren't reasoning about indices
-  // at all.
-  return loc::MemRegionVal(MRMgr.getElementRegion(UnknownVal(), BaseR));
+  if (BaseR)  
+    return loc::MemRegionVal(MRMgr.getElementRegion(UnknownVal(), BaseR));
+  else
+    return UnknownVal();
 }
 
 SVal BasicStoreManager::Retrieve(const GRState* state, Loc LV, QualType T) {

Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=60990&r1=60989&r2=60990&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Sat Dec 13 13:24:37 2008
@@ -1731,7 +1731,7 @@
       continue;
     }
 
-    // Check for casts from AllocaRegion pointer to typed pointer.
+    // Check for casts from a pointer to a region to typed pointer.
     if (isa<loc::MemRegionVal>(V)) {
       assert(Loc::IsLocType(T));
       assert(Loc::IsLocType(ExTy));
@@ -1740,14 +1740,8 @@
       std::pair<const GRState*, SVal> Res =  
         getStoreManager().CastRegion(St, V, T, CastE);
 
-      const GRState* NewSt = Res.first;
-      SVal NewPtr = Res.second;
-
-      // If no new region is created, fall through to the default case.
-      if (NewSt != St) {
-        MakeNode(Dst, CastE, N, BindExpr(NewSt, CastE, NewPtr));
-        continue;
-      }
+      MakeNode(Dst, CastE, N, BindExpr(Res.first, CastE, Res.second));
+      continue;
     }
 
     // All other cases.

Modified: cfe/trunk/lib/Analysis/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/MemRegion.cpp?rev=60990&r1=60989&r2=60990&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/MemRegion.cpp (original)
+++ cfe/trunk/lib/Analysis/MemRegion.cpp Sat Dec 13 13:24:37 2008
@@ -107,14 +107,13 @@
 }
 
 QualType ElementRegion::getType(ASTContext& C) const {
-  QualType T = cast<TypedRegion>(superRegion)->getType(C);
+  QualType T = getArrayRegion()->getType(C);
 
   if (isa<ArrayType>(T.getTypePtr())) {
     ArrayType* AT = cast<ArrayType>(T.getTypePtr());
     return AT->getElementType();
   }
   else {
-    assert (isa<AnonTypedRegion>(superRegion));
     PointerType* PtrT = cast<PointerType>(T.getTypePtr());
     QualType PTy = PtrT->getPointeeType();
     return C.getCanonicalType(PTy);
@@ -278,8 +277,9 @@
   return R;
 }
 
-ElementRegion* MemRegionManager::getElementRegion(SVal Idx,
-                                                  const MemRegion* superRegion){
+ElementRegion*
+MemRegionManager::getElementRegion(SVal Idx, const TypedRegion* superRegion){
+
   llvm::FoldingSetNodeID ID;
   ElementRegion::ProfileRegion(ID, Idx, superRegion);
 

Modified: cfe/trunk/lib/Analysis/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RegionStore.cpp?rev=60990&r1=60989&r2=60990&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/RegionStore.cpp (original)
+++ cfe/trunk/lib/Analysis/RegionStore.cpp Sat Dec 13 13:24:37 2008
@@ -287,7 +287,7 @@
 
     SVal NewIdx = CI1->EvalBinOp(getBasicVals(), BinaryOperator::Add, *CI2);
     return loc::MemRegionVal(MRMgr.getElementRegion(NewIdx, 
-                                                    ElemR->getSuperRegion()));
+                                                    ElemR->getArrayRegion()));
   }
 
   return UnknownVal();
@@ -360,8 +360,18 @@
 // Cast 'pointer to array' to 'pointer to the first element of array'.
 
 SVal RegionStoreManager::ArrayToPointer(SVal Array) {
-  const MemRegion* ArrayR = cast<loc::MemRegionVal>(&Array)->getRegion();
-
+  if (Array.isUnknownOrUndef())
+    return Array;
+  
+  if (!isa<loc::MemRegionVal>(Array))
+    return UnknownVal();
+  
+  const MemRegion* R = cast<loc::MemRegionVal>(&Array)->getRegion();
+  const TypedRegion* ArrayR = dyn_cast<TypedRegion>(R);
+  
+  if (ArrayR)
+    return UnknownVal();
+  
   nonloc::ConcreteInt Idx(getBasicVals().getZeroWithPtrWidth(false));
   ElementRegion* ER = MRMgr.getElementRegion(Idx, ArrayR);
   

Added: cfe/trunk/test/Analysis/rdar-6442306-1.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/rdar-6442306-1.m?rev=60990&view=auto

==============================================================================
--- cfe/trunk/test/Analysis/rdar-6442306-1.m (added)
+++ cfe/trunk/test/Analysis/rdar-6442306-1.m Sat Dec 13 13:24:37 2008
@@ -0,0 +1,31 @@
+// RUN: clang -checker-cfref %s --analyzer-store-basic -verify &&
+// RUN: clang -checker-cfref %s --analyzer-store-region -verify
+
+typedef int bar_return_t;
+typedef struct {
+  unsigned char int_rep;
+} Foo_record_t;
+extern Foo_record_t Foo_record;
+struct QuxSize {};
+typedef struct QuxSize QuxSize;
+typedef struct {
+  Foo_record_t Foo;
+  QuxSize size;
+} __Request__SetPortalSize_t;
+
+static __inline__ bar_return_t
+__Beeble_check__Request__SetPortalSize_t(__attribute__((__unused__)) __Request__SetPortalSize_t *In0P) {
+  if (In0P->Foo.int_rep != Foo_record.int_rep) {
+    do {
+      int __i__, __C__ = (2);
+      for (__i__ = 0;
+           __i__ < __C__;
+           __i__++) do {
+        *(&((double *)(&In0P->size))[__i__]) =
+          __Foo_READSWAP__double(&((double *)(&In0P->size))[__i__]);
+      }
+      while (0);
+    }
+    while (0);
+  }
+}





More information about the cfe-commits mailing list