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

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

4 files changed, 17 insertions, 13 deletions

diff --git a/lib/Checker/MemRegion.cpp b/lib/Checker/MemRegion.cpp
index 3f706e145a..ddcb7d2687 100644
--- a/lib/Checker/MemRegion.cpp
+++ b/lib/Checker/MemRegion.cpp
@@ -624,7 +624,7 @@ MemRegionManager::getCompoundLiteralRegion(const CompoundLiteralExpr* CL,
 }
 
 const ElementRegion*
-MemRegionManager::getElementRegion(QualType elementType, SVal Idx,
+MemRegionManager::getElementRegion(QualType elementType, NonLoc Idx,
                                    const MemRegion* superRegion,
                                    ASTContext& Ctx){
 
diff --git a/lib/Checker/RegionStore.cpp b/lib/Checker/RegionStore.cpp
index 4051f4d39b..91f7cfaf6c 100644
--- a/lib/Checker/RegionStore.cpp
+++ b/lib/Checker/RegionStore.cpp
@@ -786,7 +786,7 @@ SVal RegionStoreManager::ArrayToPointer(Loc Array) {
   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 @@ SVal RegionStoreManager::EvalBinOp(BinaryOperator::Opcode Op, Loc L, NonLoc R,
       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 RegionStoreManager::EvalBinOp(BinaryOperator::Opcode Op, Loc L, NonLoc R,
       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 @@ Store RegionStoreManager::BindArray(Store store, const TypedRegion* R,
     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())
diff --git a/lib/Checker/SVals.cpp b/lib/Checker/SVals.cpp
index 97ba74e948..937b948fc9 100644
--- a/lib/Checker/SVals.cpp
+++ b/lib/Checker/SVals.cpp
@@ -270,7 +270,7 @@ void SVal::dump() const { dumpToStream(llvm::errs()); }
 void SVal::dumpToStream(llvm::raw_ostream& os) const {
   switch (getBaseKind()) {
     case UnknownKind:
-      os << "Invalid";
+      os << "Unknown";
       break;
     case NonLocKind:
       cast<NonLoc>(this)->dumpToStream(os);
diff --git a/lib/Checker/Store.cpp b/lib/Checker/Store.cpp
index 1cb5cd70ca..aaa518edc8 100644
--- a/lib/Checker/Store.cpp
+++ b/lib/Checker/Store.cpp
@@ -28,7 +28,7 @@ Store StoreManager::EnterStackFrame(const GRState *state,
 
 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 @@ static bool IsCompleteType(ASTContext &Ctx, QualType Ty) {
 
 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 @@ SVal StoreManager::getLValueFieldOrIvar(const Decl* D, SVal Base) {
   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 @@ SVal StoreManager::getLValueElement(QualType elementType, SVal Offset,
   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 @@ SVal StoreManager::getLValueElement(QualType elementType, SVal Offset,
   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();