[cfe-commits] r161510 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h include/clang/StaticAnalyzer/Core/PathSensitive/Store.h lib/StaticAnalyzer/Core/MemRegion.cpp lib/StaticAnalyzer/Core/ProgramState.cpp lib/StaticAnalyzer/Core/RegionStore.cpp lib/StaticAnalyzer/Core/Store.cpp test/Analysis/array-struct-region.c

Jordan Rose jordan_rose at apple.com
Wed Aug 8 11:23:27 PDT 2012


Author: jrose
Date: Wed Aug  8 13:23:27 2012
New Revision: 161510

URL: http://llvm.org/viewvc/llvm-project?rev=161510&view=rev
Log:
[analyzer] Revamp RegionStore to distinguish regions with symbolic offsets.

RegionStore currently uses a (Region, Offset) pair to describe the locations
of memory bindings. However, this representation breaks down when we have
regions like 'array[index]', where 'index' is unknown. We used to store this
as (SubRegion, 0); now we mark them specially as (SubRegion, SYMBOLIC).

Furthermore, ProgramState::scanReachableSymbols depended on the existence of
a sub-region map, but RegionStore's implementation doesn't provide for such
a thing. Moving the store-traversing logic of scanReachableSymbols into the
StoreManager allows us to eliminate the notion of SubRegionMap altogether.

This fixes some particularly awkward broken test cases, now in
array-struct-region.c.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
    cfe/trunk/test/Analysis/array-struct-region.c

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=161510&r1=161509&r2=161510&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Wed Aug  8 13:23:27 2012
@@ -52,11 +52,13 @@
   int64_t Offset;
 
 public:
-  RegionOffset(const MemRegion *r) : R(r), Offset(0) {}
+  RegionOffset() : R(0) {}
   RegionOffset(const MemRegion *r, int64_t off) : R(r), Offset(off) {}
 
   const MemRegion *getRegion() const { return R; }
   int64_t getOffset() const { return Offset; }
+
+  bool isValid() const { return R; }
 };
 
 //===----------------------------------------------------------------------===//
@@ -490,6 +492,8 @@
     return T.getTypePtrOrNull() ? T.getDesugaredType(Context) : T;
   }
 
+  DefinedOrUnknownSVal getExtent(SValBuilder &svalBuilder) const;
+
   static bool classof(const MemRegion* R) {
     unsigned k = R->getKind();
     return k >= BEG_TYPED_VALUE_REGIONS && k <= END_TYPED_VALUE_REGIONS;
@@ -806,8 +810,6 @@
   const Decl *getDecl() const { return D; }
   void Profile(llvm::FoldingSetNodeID& ID) const;
 
-  DefinedOrUnknownSVal getExtent(SValBuilder &svalBuilder) const;
-
   static bool classof(const MemRegion* R) {
     unsigned k = R->getKind();
     return k >= BEG_DECL_REGIONS && k <= END_DECL_REGIONS;

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=161510&r1=161509&r2=161510&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Wed Aug  8 13:23:27 2012
@@ -795,14 +795,12 @@
 /// \class ScanReachableSymbols
 /// A Utility class that allows to visit the reachable symbols using a custom
 /// SymbolVisitor.
-class ScanReachableSymbols : public SubRegionMap::Visitor  {
-  virtual void anchor();
+class ScanReachableSymbols {
   typedef llvm::DenseMap<const void*, unsigned> VisitedItems;
 
   VisitedItems visited;
   ProgramStateRef state;
   SymbolVisitor &visitor;
-  OwningPtr<SubRegionMap> SRM;
 public:
 
   ScanReachableSymbols(ProgramStateRef st, SymbolVisitor& v)
@@ -812,11 +810,6 @@
   bool scan(SVal val);
   bool scan(const MemRegion *R);
   bool scan(const SymExpr *sym);
-
-  // From SubRegionMap::Visitor.
-  bool Visit(const MemRegion* Parent, const MemRegion* SubRegion) {
-    return scan(SubRegion);
-  }
 };
 
 } // end GR namespace

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h?rev=161510&r1=161509&r2=161510&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h Wed Aug  8 13:23:27 2012
@@ -32,7 +32,7 @@
 class CallEvent;
 class ProgramState;
 class ProgramStateManager;
-class SubRegionMap;
+class ScanReachableSymbols;
 
 class StoreManager {
 protected:
@@ -85,11 +85,6 @@
   ///  used to query and manipulate MemRegion objects.
   MemRegionManager& getRegionManager() { return MRMgr; }
 
-  /// getSubRegionMap - Returns an opaque map object that clients can query
-  ///  to get the subregions of a given MemRegion object.  It is the
-  //   caller's responsibility to 'delete' the returned map.
-  virtual SubRegionMap *getSubRegionMap(Store store) = 0;
-
   virtual Loc getLValueVar(const VarDecl *VD, const LocationContext *LC) {
     return svalBuilder.makeLoc(MRMgr.getVarRegion(VD, LC));
   }
@@ -203,6 +198,12 @@
                            const CallEvent &Call,
                            const StackFrameContext *CalleeCtx);
 
+  /// Finds the transitive closure of symbols within the given region.
+  ///
+  /// Returns false if the visitor aborted the scan.
+  virtual bool scanReachableSymbols(Store S, const MemRegion *R,
+                                    ScanReachableSymbols &Visitor) = 0;
+
   virtual void print(Store store, raw_ostream &Out,
                      const char* nl, const char *sep) = 0;
 
@@ -274,24 +275,6 @@
   return *this;
 }
 
-// FIXME: Do we still need this?
-/// SubRegionMap - An abstract interface that represents a queryable map
-///  between MemRegion objects and their subregions.
-class SubRegionMap {
-  virtual void anchor();
-public:
-  virtual ~SubRegionMap() {}
-
-  class Visitor {
-    virtual void anchor();
-  public:
-    virtual ~Visitor() {}
-    virtual bool Visit(const MemRegion* Parent, const MemRegion* SubRegion) = 0;
-  };
-
-  virtual bool iterSubRegions(const MemRegion *region, Visitor& V) const = 0;
-};
-
 // FIXME: Do we need to pass ProgramStateManager anymore?
 StoreManager *CreateRegionStoreManager(ProgramStateManager& StMgr);
 StoreManager *CreateFieldsOnlyRegionStoreManager(ProgramStateManager& StMgr);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=161510&r1=161509&r2=161510&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Aug  8 13:23:27 2012
@@ -179,7 +179,7 @@
 // Region extents.
 //===----------------------------------------------------------------------===//
 
-DefinedOrUnknownSVal DeclRegion::getExtent(SValBuilder &svalBuilder) const {
+DefinedOrUnknownSVal TypedValueRegion::getExtent(SValBuilder &svalBuilder) const {
   ASTContext &Ctx = svalBuilder.getContext();
   QualType T = getDesugaredValueType(Ctx);
 
@@ -470,7 +470,7 @@
 }
 
 void CXXBaseObjectRegion::dumpToStream(raw_ostream &os) const {
-  os << "base " << decl->getName();
+  os << "base{" << superRegion << ',' << decl->getName() << '}';
 }
 
 void CXXThisRegion::dumpToStream(raw_ostream &os) const {
@@ -1031,30 +1031,63 @@
   while (1) {
     switch (R->getKind()) {
     default:
-      return RegionOffset(0);
+      return RegionOffset();
     case SymbolicRegionKind:
     case AllocaRegionKind:
     case CompoundLiteralRegionKind:
     case CXXThisRegionKind:
     case StringRegionKind:
     case VarRegionKind:
+    case ObjCIvarRegionKind:
     case CXXTempObjectRegionKind:
       goto Finish;
+    case CXXBaseObjectRegionKind: {
+      const CXXBaseObjectRegion *BOR = cast<CXXBaseObjectRegion>(R);
+      R = BOR->getSuperRegion();
+
+      QualType Ty;
+      if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(R)) {
+        Ty = TVR->getDesugaredValueType(getContext());
+      } else if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R)) {
+        // If our base region is symbolic, we don't know what type it really is.
+        // Pretend the type of the symbol is the true dynamic type.
+        // (This will at least be self-consistent for the life of the symbol.)
+        Ty = SR->getSymbol()->getType(getContext())->getPointeeType();
+      }
+      
+      const CXXRecordDecl *Child = Ty->getAsCXXRecordDecl();
+      if (!Child) {
+        // We cannot compute the offset of the base class.
+        return RegionOffset();
+      }
+      const ASTRecordLayout &Layout = getContext().getASTRecordLayout(Child);
+
+      CharUnits BaseOffset;
+      const CXXRecordDecl *Base = BOR->getDecl();
+      if (Child->isVirtuallyDerivedFrom(Base))
+        BaseOffset = Layout.getVBaseClassOffset(Base);
+      else
+        BaseOffset = Layout.getBaseClassOffset(Base);
+
+      // The base offset is in chars, not in bits.
+      Offset += BaseOffset.getQuantity() * getContext().getCharWidth();
+      break;
+    }
     case ElementRegionKind: {
       const ElementRegion *ER = cast<ElementRegion>(R);
       QualType EleTy = ER->getValueType();
 
       if (!IsCompleteType(getContext(), EleTy))
-        return RegionOffset(0);
+        return RegionOffset();
 
       SVal Index = ER->getIndex();
       if (const nonloc::ConcreteInt *CI=dyn_cast<nonloc::ConcreteInt>(&Index)) {
         int64_t i = CI->getValue().getSExtValue();
-        CharUnits Size = getContext().getTypeSizeInChars(EleTy);
-        Offset += i * Size.getQuantity() * 8;
+        // This type size is in bits.
+        Offset += i * getContext().getTypeSize(EleTy);
       } else {
         // We cannot compute offset for non-concrete index.
-        return RegionOffset(0);
+        return RegionOffset();
       }
       R = ER->getSuperRegion();
       break;
@@ -1064,7 +1097,7 @@
       const RecordDecl *RD = FR->getDecl()->getParent();
       if (!RD->isCompleteDefinition())
         // We cannot compute offset for incomplete type.
-        return RegionOffset(0);
+        return RegionOffset();
       // Get the field number.
       unsigned idx = 0;
       for (RecordDecl::field_iterator FI = RD->field_begin(), 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp?rev=161510&r1=161509&r2=161510&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp Wed Aug  8 13:23:27 2012
@@ -499,8 +499,6 @@
   return getPersistentState(NewState);
 }
 
-void ScanReachableSymbols::anchor() { }
-
 bool ScanReachableSymbols::scan(nonloc::CompoundVal val) {
   for (nonloc::CompoundVal::iterator I=val.begin(), E=val.end(); I!=E; ++I)
     if (!scan(*I))
@@ -578,10 +576,19 @@
       return false;
 
   // If this is a subregion, also visit the parent regions.
-  if (const SubRegion *SR = dyn_cast<SubRegion>(R))
-    if (!scan(SR->getSuperRegion()))
+  if (const SubRegion *SR = dyn_cast<SubRegion>(R)) {
+    const MemRegion *Super = SR->getSuperRegion();
+    if (!scan(Super))
       return false;
 
+    // When we reach the topmost region, scan all symbols in it.
+    if (isa<MemSpaceRegion>(Super)) {
+      StoreManager &StoreMgr = state->getStateManager().getStoreManager();
+      if (!StoreMgr.scanReachableSymbols(state->getStore(), SR, *this))
+        return false;
+    }
+  }
+
   // Regions captured by a block are also implicitly reachable.
   if (const BlockDataRegion *BDR = dyn_cast<BlockDataRegion>(R)) {
     BlockDataRegion::referenced_vars_iterator I = BDR->referenced_vars_begin(),
@@ -592,16 +599,7 @@
     }
   }
 
-  // Now look at the binding to this region (if any).
-  if (!scan(state->getSValAsScalarOrLoc(R)))
-    return false;
-
-  // Now look at the subregions.
-  if (!SRM.get())
-    SRM.reset(state->getStateManager().getStoreManager().
-                                           getSubRegionMap(state->getStore()));
-
-  return SRM->iterSubRegions(R, *this);
+  return true;
 }
 
 bool ProgramState::scanReachableSymbols(SVal val, SymbolVisitor& visitor) const {

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=161510&r1=161509&r2=161510&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Aug  8 13:23:27 2012
@@ -42,17 +42,27 @@
 public:
   enum Kind { Direct = 0x0, Default = 0x1 };
 private:
-  llvm ::PointerIntPair<const MemRegion*, 1> P;
+  enum { SYMBOLIC = UINT64_MAX };
+
+  llvm::PointerIntPair<const MemRegion *, 1, Kind> P;
   uint64_t Offset;
 
+  explicit BindingKey(const MemRegion *r, Kind k)
+    : P(r, k), Offset(SYMBOLIC) {}
   explicit BindingKey(const MemRegion *r, uint64_t offset, Kind k)
-    : P(r, (unsigned) k), Offset(offset) {}
+    : P(r, k), Offset(offset) {}
 public:
 
   bool isDirect() const { return P.getInt() == Direct; }
+  bool hasSymbolicOffset() const { return Offset == SYMBOLIC; }
 
   const MemRegion *getRegion() const { return P.getPointer(); }
-  uint64_t getOffset() const { return Offset; }
+  uint64_t getOffset() const {
+    assert(!hasSymbolicOffset());
+    return Offset;
+  }
+
+  const MemRegion *getConcreteOffsetRegion() const;
 
   void Profile(llvm::FoldingSetNodeID& ID) const {
     ID.AddPointer(P.getOpaqueValue());
@@ -82,17 +92,36 @@
 
 BindingKey BindingKey::Make(const MemRegion *R, Kind k) {
   const RegionOffset &RO = R->getAsOffset();
-  if (RO.getRegion())
+  if (RO.isValid())
     return BindingKey(RO.getRegion(), RO.getOffset(), k);
 
-  return BindingKey(R, 0, k);
+  return BindingKey(R, k);
+}
+
+const MemRegion *BindingKey::getConcreteOffsetRegion() const {
+  const MemRegion *R = getRegion();
+  if (!hasSymbolicOffset())
+    return R;
+
+  RegionOffset RO;
+  do {
+    const SubRegion *SR = dyn_cast<SubRegion>(R);
+    if (!SR)
+      break;
+    R = SR->getSuperRegion();
+    RO = R->getAsOffset();
+  } while (!RO.isValid());
+
+  return R;
 }
 
 namespace llvm {
   static inline
   raw_ostream &operator<<(raw_ostream &os, BindingKey K) {
-    os << '(' << K.getRegion() << ',' << K.getOffset()
-       << ',' << (K.isDirect() ? "direct" : "default")
+    os << '(' << K.getRegion();
+    if (!K.hasSymbolicOffset())
+      os << ',' << K.getOffset();
+    os << ',' << (K.isDirect() ? "direct" : "default")
        << ')';
     return os;
   }
@@ -133,60 +162,6 @@
 
 namespace {
 
-class RegionStoreSubRegionMap : public SubRegionMap {
-public:
-  typedef llvm::ImmutableSet<const MemRegion*> Set;
-  typedef llvm::DenseMap<const MemRegion*, Set> Map;
-private:
-  Set::Factory F;
-  Map M;
-public:
-  bool add(const MemRegion* Parent, const MemRegion* SubRegion) {
-    Map::iterator I = M.find(Parent);
-
-    if (I == M.end()) {
-      M.insert(std::make_pair(Parent, F.add(F.getEmptySet(), SubRegion)));
-      return true;
-    }
-
-    I->second = F.add(I->second, SubRegion);
-    return false;
-  }
-
-  void process(SmallVectorImpl<const SubRegion*> &WL, const SubRegion *R);
-
-  ~RegionStoreSubRegionMap() {}
-
-  const Set *getSubRegions(const MemRegion *Parent) const {
-    Map::const_iterator I = M.find(Parent);
-    return I == M.end() ? NULL : &I->second;
-  }
-
-  bool iterSubRegions(const MemRegion* Parent, Visitor& V) const {
-    Map::const_iterator I = M.find(Parent);
-
-    if (I == M.end())
-      return true;
-
-    Set S = I->second;
-    for (Set::iterator SI=S.begin(),SE=S.end(); SI != SE; ++SI) {
-      if (!V.Visit(Parent, *SI))
-        return false;
-    }
-
-    return true;
-  }
-};
-
-void
-RegionStoreSubRegionMap::process(SmallVectorImpl<const SubRegion*> &WL,
-                                 const SubRegion *R) {
-  const MemRegion *superR = R->getSuperRegion();
-  if (add(superR, R))
-    if (const SubRegion *sr = dyn_cast<SubRegion>(superR))
-      WL.push_back(sr);
-}
-
 class RegionStoreManager : public StoreManager {
   const RegionStoreFeatures Features;
   RegionBindings::Factory RBFactory;
@@ -197,12 +172,6 @@
       Features(f),
       RBFactory(mgr.getAllocator()) {}
 
-  SubRegionMap *getSubRegionMap(Store store) {
-    return getRegionStoreSubRegionMap(store);
-  }
-
-  RegionStoreSubRegionMap *getRegionStoreSubRegionMap(Store store);
-
   Optional<SVal> getDirectBinding(RegionBindings B, const MemRegion *R);
   /// getDefaultBinding - Returns an SVal* representing an optional default
   ///  binding associated with a region and its subregions.
@@ -255,10 +224,12 @@
                              const CallEvent *Call,
                              InvalidatedRegions *Invalidated);
 
+  bool scanReachableSymbols(Store S, const MemRegion *R,
+                            ScanReachableSymbols &Callbacks);
+
 public:   // Made public for helper classes.
 
-  void RemoveSubRegionBindings(RegionBindings &B, const MemRegion *R,
-                               RegionStoreSubRegionMap &M);
+  RegionBindings removeSubRegionBindings(RegionBindings B, const SubRegion *R);
 
   RegionBindings addBinding(RegionBindings B, BindingKey K, SVal V);
 
@@ -443,28 +414,6 @@
 }
 
 
-RegionStoreSubRegionMap*
-RegionStoreManager::getRegionStoreSubRegionMap(Store store) {
-  RegionBindings B = GetRegionBindings(store);
-  RegionStoreSubRegionMap *M = new RegionStoreSubRegionMap();
-
-  SmallVector<const SubRegion*, 10> WL;
-
-  for (RegionBindings::iterator I=B.begin(), E=B.end(); I!=E; ++I)
-    if (const SubRegion *R = dyn_cast<SubRegion>(I.getKey().getRegion()))
-      M->process(WL, R);
-
-  // We also need to record in the subregion map "intermediate" regions that
-  // don't have direct bindings but are super regions of those that do.
-  while (!WL.empty()) {
-    const SubRegion *R = WL.back();
-    WL.pop_back();
-    M->process(WL, R);
-  }
-
-  return M;
-}
-
 //===----------------------------------------------------------------------===//
 // Region Cluster analysis.
 //===----------------------------------------------------------------------===//
@@ -584,16 +533,88 @@
 // Binding invalidation.
 //===----------------------------------------------------------------------===//
 
-void RegionStoreManager::RemoveSubRegionBindings(RegionBindings &B,
-                                                 const MemRegion *R,
-                                                 RegionStoreSubRegionMap &M) {
-
-  if (const RegionStoreSubRegionMap::Set *S = M.getSubRegions(R))
-    for (RegionStoreSubRegionMap::Set::iterator I = S->begin(), E = S->end();
-         I != E; ++I)
-      RemoveSubRegionBindings(B, *I, M);
+bool RegionStoreManager::scanReachableSymbols(Store S, const MemRegion *R,
+                                              ScanReachableSymbols &Callbacks) {
+  // FIXME: This linear scan through all bindings could possibly be optimized
+  // by changing the data structure used for RegionBindings.
+
+  RegionBindings B = GetRegionBindings(S);
+  for (RegionBindings::iterator RI = B.begin(), RE = B.end(); RI != RE; ++RI) {
+    BindingKey Key = RI.getKey();
+    if (Key.getRegion() == R) {
+      if (!Callbacks.scan(RI.getData()))
+        return false;
+    } else if (Key.hasSymbolicOffset()) {
+      if (const SubRegion *BaseSR = dyn_cast<SubRegion>(Key.getRegion()))
+        if (BaseSR->isSubRegionOf(R))
+          if (!Callbacks.scan(RI.getData()))
+            return false;
+    }
+  }
+
+  return true;
+}
+
+RegionBindings RegionStoreManager::removeSubRegionBindings(RegionBindings B,
+                                                           const SubRegion *R) {
+  // FIXME: This linear scan through all bindings could possibly be optimized
+  // by changing the data structure used for RegionBindings.
+
+  BindingKey SRKey = BindingKey::Make(R, BindingKey::Default);
+  assert(SRKey.isValid());
+  if (SRKey.hasSymbolicOffset()) {
+    const SubRegion *Base = cast<SubRegion>(SRKey.getConcreteOffsetRegion());
+    B = removeSubRegionBindings(B, Base);
+    return addBinding(B, Base, BindingKey::Default, UnknownVal());
+  }
+
+  // FIXME: This does the wrong thing for bitfields.
+  uint64_t Length = UINT64_MAX;
+
+  SVal Extent = R->getExtent(svalBuilder);
+  if (nonloc::ConcreteInt *ExtentCI = dyn_cast<nonloc::ConcreteInt>(&Extent)) {
+    const llvm::APSInt &ExtentInt = ExtentCI->getValue();
+    assert(ExtentInt.isNonNegative() || ExtentInt.isUnsigned());
+    // Extents are in bytes but region offsets are in bits. Be careful!
+    Length = ExtentInt.getLimitedValue() * Ctx.getCharWidth();
+  }
+
+  // It is safe to iterate over the bindings as they are being changed
+  // because they are in an ImmutableMap.
+  for (RegionBindings::iterator RI = B.begin(), RE = B.end(); RI != RE; ++RI) {
+    BindingKey NextKey = RI.getKey();
+    if (NextKey.getRegion() == SRKey.getRegion()) {
+      // Case 1: The next binding is inside the region we're invalidating.
+      // Remove it.
+      if (NextKey.getOffset() > SRKey.getOffset() &&
+          NextKey.getOffset() - SRKey.getOffset() < Length)
+        B = removeBinding(B, NextKey);
+      // Case 2: The next binding is at the same offset as the region we're
+      // invalidating. In this case, we need to leave default bindings alone,
+      // since they may be providing a default value for a regions beyond what
+      // we're invalidating.
+      // FIXME: This is probably incorrect; consider invalidating an outer
+      // struct whose first field is bound to a LazyCompoundVal.
+      else if (NextKey.getOffset() == SRKey.getOffset())
+        if (NextKey.isDirect())
+          B = removeBinding(B, NextKey);
+
+    } else if (NextKey.hasSymbolicOffset()) {
+      const MemRegion *Base = NextKey.getConcreteOffsetRegion();
+      // Case 3: The next key is symbolic and we just changed something within
+      // its concrete region. We don't know if the binding is still valid, so
+      // we'll be conservative and remove it.
+      if (R->isSubRegionOf(Base))
+        B = removeBinding(B, NextKey);
+      // Case 4: The next key is symbolic, but we changed a known super-region.
+      // In this case the binding is certainly no longer valid.
+      else if (const SubRegion *BaseSR = dyn_cast<SubRegion>(Base))
+        if (BaseSR->isSubRegionOf(R))
+          B = removeBinding(B, NextKey);
+    }
+  }
 
-  B = removeBinding(B, R);
+  return B;
 }
 
 namespace {
@@ -1542,20 +1563,7 @@
       return BindVector(store, TR, V);
   }
 
-  if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
-    if (ER->getIndex().isZeroConstant()) {
-      if (const TypedValueRegion *superR =
-            dyn_cast<TypedValueRegion>(ER->getSuperRegion())) {
-        QualType superTy = superR->getValueType();
-        // For now, just invalidate the fields of the struct/union/class.
-        // This is for test rdar_test_7185607 in misc-ps-region-store.m.
-        // FIXME: Precisely handle the fields of the record.
-        if (superTy->isStructureOrClassType())
-          return KillStruct(store, superR, UnknownVal());
-      }
-    }
-  }
-  else if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R)) {
+  if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R)) {
     // Binding directly to a symbolic region should be treated as binding
     // to element 0.
     QualType T = SR->getSymbol()->getType(Ctx);
@@ -1569,10 +1577,13 @@
     R = GetElementZeroRegion(SR, T);
   }
 
+  // Clear out bindings that may overlap with this binding.
+
   // Perform the binding.
   RegionBindings B = GetRegionBindings(store);
-  return StoreRef(addBinding(B, R, BindingKey::Direct,
-                             V).getRootWithoutRetain(), *this);
+  B = removeSubRegionBindings(B, cast<SubRegion>(R));
+  BindingKey Key = BindingKey::Make(R, BindingKey::Direct);
+  return StoreRef(addBinding(B, Key, V).getRootWithoutRetain(), *this);
 }
 
 StoreRef RegionStoreManager::BindDecl(Store store, const VarRegion *VR,
@@ -1792,30 +1803,11 @@
 StoreRef RegionStoreManager::KillStruct(Store store, const TypedRegion* R,
                                      SVal DefaultVal) {
   BindingKey key = BindingKey::Make(R, BindingKey::Default);
-  
-  // The BindingKey may be "invalid" if we cannot handle the region binding
-  // explicitly.  One example is something like array[index], where index
-  // is a symbolic value.  In such cases, we want to invalidate the entire
-  // array, as the index assignment could have been to any element.  In
-  // the case of nested symbolic indices, we need to march up the region
-  // hierarchy untile we reach a region whose binding we can reason about.
-  const SubRegion *subReg = R;
-
-  while (!key.isValid()) {
-    if (const SubRegion *tmp = dyn_cast<SubRegion>(subReg->getSuperRegion())) {
-      subReg = tmp;
-      key = BindingKey::Make(tmp, BindingKey::Default);
-    }
-    else
-      break;
-  }                                 
 
-  // Remove the old bindings, using 'subReg' as the root of all regions
+  // Remove the old bindings, using 'R' as the root of all regions
   // we will invalidate.
   RegionBindings B = GetRegionBindings(store);
-  OwningPtr<RegionStoreSubRegionMap>
-    SubRegions(getRegionStoreSubRegionMap(store));
-  RemoveSubRegionBindings(B, subReg, *SubRegions);
+  B = removeSubRegionBindings(B, R);
 
   // Set the default value of the struct region to "unknown".
   if (!key.isValid())
@@ -1830,12 +1822,7 @@
 
   // Nuke the old bindings stemming from R.
   RegionBindings B = GetRegionBindings(store);
-
-  OwningPtr<RegionStoreSubRegionMap>
-    SubRegions(getRegionStoreSubRegionMap(store));
-
-  // B and DVM are updated after the call to RemoveSubRegionBindings.
-  RemoveSubRegionBindings(B, R, *SubRegions.get());
+  B = removeSubRegionBindings(B, R);
 
   // Now copy the bindings.  This amounts to just binding 'V' to 'R'.  This
   // results in a zero-copy algorithm.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=161510&r1=161509&r2=161510&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Wed Aug  8 13:23:27 2012
@@ -369,6 +369,3 @@
 
   return true;
 }
-
-void SubRegionMap::anchor() { }
-void SubRegionMap::Visitor::anchor() { }

Modified: cfe/trunk/test/Analysis/array-struct-region.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/array-struct-region.c?rev=161510&r1=161509&r2=161510&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/array-struct-region.c (original)
+++ cfe/trunk/test/Analysis/array-struct-region.c Wed Aug  8 13:23:27 2012
@@ -92,3 +92,95 @@
   return c.r; // no-warning
 }
 
+
+int randomInt();
+
+int testSymbolicInvalidation(int index) {
+  int vals[10];
+
+  vals[0] = 42;
+  clang_analyzer_eval(vals[0] == 42); // expected-warning{{TRUE}}
+
+  vals[index] = randomInt();
+  clang_analyzer_eval(vals[0] == 42); // expected-warning{{UNKNOWN}}
+
+  return vals[index]; // no-warning
+}
+
+int testConcreteInvalidation(int index) {
+  int vals[10];
+
+  vals[index] = 42;
+  clang_analyzer_eval(vals[index] == 42); // expected-warning{{TRUE}}
+  vals[0] = randomInt();
+  clang_analyzer_eval(vals[index] == 42); // expected-warning{{UNKNOWN}}
+
+  return vals[0]; // no-warning
+}
+
+
+typedef struct {
+  int x, y, z;
+} S;
+
+S makeS();
+
+int testSymbolicInvalidationStruct(int index) {
+  S vals[10];
+
+  vals[0].x = 42;
+  clang_analyzer_eval(vals[0].x == 42); // expected-warning{{TRUE}}
+
+  vals[index] = makeS();
+  clang_analyzer_eval(vals[0].x == 42); // expected-warning{{UNKNOWN}}
+
+  return vals[index].x; // no-warning
+}
+
+int testConcreteInvalidationStruct(int index) {
+  S vals[10];
+
+  vals[index].x = 42;
+  clang_analyzer_eval(vals[index].x == 42); // expected-warning{{TRUE}}
+  vals[0] = makeS();
+  clang_analyzer_eval(vals[index].x == 42); // expected-warning{{UNKNOWN}}
+
+  return vals[0].x; // no-warning
+}
+
+typedef struct {
+  S a[5];
+  S b[5];
+} SS;
+
+int testSymbolicInvalidationDoubleStruct(int index) {
+  SS vals;
+
+  vals.a[0].x = 42;
+  vals.b[0].x = 42;
+  clang_analyzer_eval(vals.a[0].x == 42); // expected-warning{{TRUE}}
+  clang_analyzer_eval(vals.b[0].x == 42); // expected-warning{{TRUE}}
+
+  vals.a[index] = makeS();
+  clang_analyzer_eval(vals.a[0].x == 42); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(vals.b[0].x == 42); // expected-warning{{TRUE}}
+
+  return vals.b[index].x; // no-warning
+}
+
+int testConcreteInvalidationDoubleStruct(int index) {
+  SS vals;
+
+  vals.a[index].x = 42;
+  vals.b[index].x = 42;
+  clang_analyzer_eval(vals.a[index].x == 42); // expected-warning{{TRUE}}
+  clang_analyzer_eval(vals.b[index].x == 42); // expected-warning{{TRUE}}
+
+  vals.a[0] = makeS();
+  clang_analyzer_eval(vals.a[index].x == 42); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(vals.b[index].x == 42); // expected-warning{{TRUE}}
+
+  return vals.b[0].x; // no-warning
+}
+
+





More information about the cfe-commits mailing list