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

3 files changed, 8 insertions, 6 deletions

diff --git a/include/clang/Checker/PathSensitive/GRState.h b/include/clang/Checker/PathSensitive/GRState.h
index ac382898d8..878f564491 100644
--- a/include/clang/Checker/PathSensitive/GRState.h
+++ b/include/clang/Checker/PathSensitive/GRState.h
@@ -650,7 +650,9 @@ inline SVal GRState::getLValue(const FieldDecl* D, SVal Base) const {
 }
 
 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 {
diff --git a/include/clang/Checker/PathSensitive/MemRegion.h b/include/clang/Checker/PathSensitive/MemRegion.h
index 96f906af28..82b0c14f20 100644
--- a/include/clang/Checker/PathSensitive/MemRegion.h
+++ b/include/clang/Checker/PathSensitive/MemRegion.h
@@ -788,9 +788,9 @@ class ElementRegion : public TypedRegion {
   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 @@ class ElementRegion : public TypedRegion {
 
 public:
 
-  SVal getIndex() const { return Index; }
+  NonLoc getIndex() const { return Index; }
 
   QualType getValueType() const {
     return ElementType;
@@ -942,7 +942,7 @@ public:
   
   /// 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);
 
diff --git a/include/clang/Checker/PathSensitive/Store.h b/include/clang/Checker/PathSensitive/Store.h
index a1a41847a2..1ead466136 100644
--- a/include/clang/Checker/PathSensitive/Store.h
+++ b/include/clang/Checker/PathSensitive/Store.h
@@ -112,7 +112,7 @@ public:
     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.