Author: kremenek
Date: Tue Sep 14 22:13:30 2010
New Revision: 113920

URL: http://llvm.org/viewvc/llvm-project?rev=113920&view=rev
Log:
Disallow the use of UnknownVal as the index for ElementRegions.  UnknownVals 
can be used as
the index when the value evaluation isn't powerful enough.  By creating 
ElementRegions with
UnknownVals as the index, this gives the false impression that they are the 
same element, when
they really aren't.  This becomes really problematic when deriving symbols from 
these regions
(e.g., those representing the initial value of the index), since two different 
indices will
get the same symbol for their binding.

This fixes an issue with the idempotent operations checker that would cause two 
indices that
are clearly not the same to make it appear as if they always had the same value.

Fixes <rdar://problem/8431728>.

Modified:
    cfe/trunk/include/clang/Checker/PathSensitive/GRState.h
    cfe/trunk/include/clang/Checker/PathSensitive/MemRegion.h
    cfe/trunk/include/clang/Checker/PathSensitive/Store.h
    cfe/trunk/lib/Checker/MemRegion.cpp
    cfe/trunk/lib/Checker/RegionStore.cpp
    cfe/trunk/lib/Checker/SVals.cpp
    cfe/trunk/lib/Checker/Store.cpp
    cfe/trunk/test/Analysis/idempotent-operations.c

Modified: cfe/trunk/include/clang/Checker/PathSensitive/GRState.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Checker/PathSensitive/GRState.h?rev=113920&r1=113919&r2=113920&view=diff
==============================================================================
--- cfe/trunk/include/clang/Checker/PathSensitive/GRState.h (original)
+++ cfe/trunk/include/clang/Checker/PathSensitive/GRState.h Tue Sep 14 22:13:30 
2010
@@ -650,7 +650,9 @@
 }
 
 inline SVal GRState::getLValue(QualType ElementType, SVal Idx, SVal Base) 
const{
-  return getStateManager().StoreMgr->getLValueElement(ElementType, Idx, Base);
+  if (NonLoc *N = dyn_cast<NonLoc>(&Idx))
+    return getStateManager().StoreMgr->getLValueElement(ElementType, *N, Base);
+  return UnknownVal();
 }
 
 inline const llvm::APSInt *GRState::getSymVal(SymbolRef sym) const {

Modified: cfe/trunk/include/clang/Checker/PathSensitive/MemRegion.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Checker/PathSensitive/MemRegion.h?rev=113920&r1=113919&r2=113920&view=diff
==============================================================================
--- cfe/trunk/include/clang/Checker/PathSensitive/MemRegion.h (original)
+++ cfe/trunk/include/clang/Checker/PathSensitive/MemRegion.h Tue Sep 14 
22:13:30 2010
@@ -788,9 +788,9 @@
   friend class MemRegionManager;
 
   QualType ElementType;
-  SVal Index;
+  NonLoc Index;
 
-  ElementRegion(QualType elementType, SVal Idx, const MemRegion* sReg)
+  ElementRegion(QualType elementType, NonLoc Idx, const MemRegion* sReg)
     : TypedRegion(sReg, ElementRegionKind),
       ElementType(elementType), Index(Idx) {
     assert((!isa<nonloc::ConcreteInt>(&Idx) ||
@@ -803,7 +803,7 @@
 
 public:
 
-  SVal getIndex() const { return Index; }
+  NonLoc getIndex() const { return Index; }
 
   QualType getValueType() const {
     return ElementType;
@@ -942,7 +942,7 @@
   
   /// getElementRegion - Retrieve the memory region associated with the
   ///  associated element type, index, and super region.
-  const ElementRegion *getElementRegion(QualType elementType, SVal Idx,
+  const ElementRegion *getElementRegion(QualType elementType, NonLoc Idx,
                                         const MemRegion *superRegion,
                                         ASTContext &Ctx);
 

Modified: cfe/trunk/include/clang/Checker/PathSensitive/Store.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Checker/PathSensitive/Store.h?rev=113920&r1=113919&r2=113920&view=diff
==============================================================================
--- cfe/trunk/include/clang/Checker/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/Checker/PathSensitive/Store.h Tue Sep 14 22:13:30 
2010
@@ -112,7 +112,7 @@
     return getLValueFieldOrIvar(D, Base);
   }
 
-  virtual SVal getLValueElement(QualType elementType, SVal offset, SVal Base);
+  virtual SVal getLValueElement(QualType elementType, NonLoc offset, SVal 
Base);
 
   // FIXME: This should soon be eliminated altogether; clients should deal with
   // region extents directly.

Modified: cfe/trunk/lib/Checker/MemRegion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/MemRegion.cpp?rev=113920&r1=113919&r2=113920&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/MemRegion.cpp (original)
+++ cfe/trunk/lib/Checker/MemRegion.cpp Tue Sep 14 22:13:30 2010
@@ -624,7 +624,7 @@
 }
 
 const ElementRegion*
-MemRegionManager::getElementRegion(QualType elementType, SVal Idx,
+MemRegionManager::getElementRegion(QualType elementType, NonLoc Idx,
                                    const MemRegion* superRegion,
                                    ASTContext& Ctx){
 

Modified: cfe/trunk/lib/Checker/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/RegionStore.cpp?rev=113920&r1=113919&r2=113920&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/RegionStore.cpp (original)
+++ cfe/trunk/lib/Checker/RegionStore.cpp Tue Sep 14 22:13:30 2010
@@ -786,7 +786,7 @@
   ArrayType *AT = cast<ArrayType>(T);
   T = AT->getElementType();
 
-  SVal ZeroIdx = ValMgr.makeZeroArrayIndex();
+  NonLoc ZeroIdx = ValMgr.makeZeroArrayIndex();
   return loc::MemRegionVal(MRMgr.getElementRegion(T, ZeroIdx, ArrayR, Ctx));
 }
 
@@ -828,14 +828,14 @@
       else
         EleTy = T->getAs<ObjCObjectPointerType>()->getPointeeType();
 
-      SVal ZeroIdx = ValMgr.makeZeroArrayIndex();
+      const NonLoc &ZeroIdx = ValMgr.makeZeroArrayIndex();
       ER = MRMgr.getElementRegion(EleTy, ZeroIdx, SR, Ctx);
       break;
     }
     case MemRegion::AllocaRegionKind: {
       const AllocaRegion *AR = cast<AllocaRegion>(MR);
       QualType EleTy = Ctx.CharTy; // Create an ElementRegion of bytes.
-      SVal ZeroIdx = ValMgr.makeZeroArrayIndex();
+      NonLoc ZeroIdx = ValMgr.makeZeroArrayIndex();
       ER = MRMgr.getElementRegion(EleTy, ZeroIdx, AR, Ctx);
       break;
     }
@@ -889,8 +889,12 @@
       SVal NewIdx =
         Base->evalBinOp(ValMgr, Op,
                 
cast<nonloc::ConcreteInt>(ValMgr.convertToArrayIndex(*Offset)));
+
+      if (!isa<NonLoc>(NewIdx))
+        return UnknownVal();
+
       const MemRegion* NewER =
-        MRMgr.getElementRegion(ER->getElementType(), NewIdx,
+        MRMgr.getElementRegion(ER->getElementType(), cast<NonLoc>(NewIdx),
                                ER->getSuperRegion(), Ctx);
       return ValMgr.makeLoc(NewER);
     }
@@ -1449,7 +1453,7 @@
     if (VI == VE)
       break;
 
-    SVal Idx = ValMgr.makeArrayIndex(i);
+    const NonLoc &Idx = ValMgr.makeArrayIndex(i);
     const ElementRegion *ER = MRMgr.getElementRegion(ElementTy, Idx, R, Ctx);
 
     if (ElementTy->isStructureOrClassType())

Modified: cfe/trunk/lib/Checker/SVals.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/SVals.cpp?rev=113920&r1=113919&r2=113920&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/SVals.cpp (original)
+++ cfe/trunk/lib/Checker/SVals.cpp Tue Sep 14 22:13:30 2010
@@ -270,7 +270,7 @@
 void SVal::dumpToStream(llvm::raw_ostream& os) const {
   switch (getBaseKind()) {
     case UnknownKind:
-      os << "Invalid";
+      os << "Unknown";
       break;
     case NonLocKind:
       cast<NonLoc>(this)->dumpToStream(os);

Modified: cfe/trunk/lib/Checker/Store.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/Store.cpp?rev=113920&r1=113919&r2=113920&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/Store.cpp (original)
+++ cfe/trunk/lib/Checker/Store.cpp Tue Sep 14 22:13:30 2010
@@ -28,7 +28,7 @@
 
 const MemRegion *StoreManager::MakeElementRegion(const MemRegion *Base,
                                               QualType EleTy, uint64_t index) {
-  SVal idx = ValMgr.makeArrayIndex(index);
+  NonLoc idx = ValMgr.makeArrayIndex(index);
   return MRMgr.getElementRegion(EleTy, idx, Base, ValMgr.getContext());
 }
 
@@ -45,7 +45,7 @@
 
 const ElementRegion *StoreManager::GetElementZeroRegion(const MemRegion *R, 
                                                         QualType T) {
-  SVal idx = ValMgr.makeZeroArrayIndex();
+  NonLoc idx = ValMgr.makeZeroArrayIndex();
   assert(!T.isNull());
   return MRMgr.getElementRegion(T, idx, R, Ctx);
 }
@@ -267,7 +267,7 @@
   return loc::MemRegionVal(MRMgr.getFieldRegion(cast<FieldDecl>(D), BaseR));
 }
 
-SVal StoreManager::getLValueElement(QualType elementType, SVal Offset, 
+SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset, 
                                     SVal Base) {
 
   // If the base is an unknown or undefined value, just return it back.
@@ -283,7 +283,7 @@
   const ElementRegion *ElemR = dyn_cast<ElementRegion>(BaseRegion);
 
   // Convert the offset to the appropriate size and signedness.
-  Offset = ValMgr.convertToArrayIndex(Offset);
+  Offset = cast<NonLoc>(ValMgr.convertToArrayIndex(Offset));
 
   if (!ElemR) {
     //
@@ -322,8 +322,8 @@
   assert(BaseIdxI.isSigned());
 
   // Compute the new index.
-  SVal NewIdx = nonloc::ConcreteInt(
-                      ValMgr.getBasicValueFactory().getValue(BaseIdxI + OffI));
+  nonloc::ConcreteInt NewIdx(ValMgr.getBasicValueFactory().getValue(BaseIdxI +
+                                                                    OffI));
 
   // Construct the new ElementRegion.
   const MemRegion *ArrayR = ElemR->getSuperRegion();

Modified: cfe/trunk/test/Analysis/idempotent-operations.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/idempotent-operations.c?rev=113920&r1=113919&r2=113920&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/idempotent-operations.c (original)
+++ cfe/trunk/test/Analysis/idempotent-operations.c Tue Sep 14 22:13:30 2010
@@ -194,3 +194,33 @@
   a = (short)a; // no-warning
   test(a);
 }
+
+// This test case previously flagged a warning at 'b == c' because the
+// analyzer previously allowed 'UnknownVal' as the index for ElementRegions.
+typedef struct RDar8431728_F {
+  int RDar8431728_A;
+  unsigned char *RDar8431728_B;
+  int RDar8431728_E[6];
+} RDar8431728_D;
+static inline int RDar8431728_C(RDar8431728_D * s, int n,
+    unsigned char **RDar8431728_B_ptr) {
+  int xy, wrap, pred, a, b, c;
+
+  xy = s->RDar8431728_E[n];
+  wrap = s->RDar8431728_A;
+
+  a = s->RDar8431728_B[xy - 1];
+  b = s->RDar8431728_B[xy - 1 - wrap];
+  c = s->RDar8431728_B[xy - wrap];
+
+  if (b == c) { // no-warning
+    pred = a;
+  } else {
+    pred = c;
+  }
+
+  *RDar8431728_B_ptr = &s->RDar8431728_B[xy];
+
+  return pred;
+}
+


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to