Rework use of loc::SymbolVal in the retain/release checker to use the new method

SVal::getAsLocSymbol(). This simplifies the code and allows the retain/release
checker to (I believe) also correctly reason about location symbols wrapped in
SymbolicRegions.

Along the way I cleaned up SymbolRef a little, disallowing implicit casts to
'unsigned'.

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

diff --git a/include/clang/Analysis/PathSensitive/SVals.h b/include/clang/Analysis/PathSensitive/SVals.h
index a0bc3ed186b5a83d91a7402d2184a97c2a7274ca..42cd4225d34d0bf2e93d339d51486fe442248b08 100644 (file)
--- a/include/clang/Analysis/PathSensitive/SVals.h
+++ b/include/clang/Analysis/PathSensitive/SVals.h
@@ -66,8 +66,7 @@ public:
   inline bool operator==(const SVal& R) const {
     return getRawKind() == R.getRawKind() && Data == R.Data;
   }
-  
-  
+    
   inline bool operator!=(const SVal& R) const {
     return !(*this == R);
   }
@@ -75,7 +74,6 @@ public:
   /// GetRValueSymbolVal - make a unique symbol for value of R.
   static SVal GetRValueSymbolVal(SymbolManager& SymMgr, const MemRegion* R);
 
-  
   inline bool isUnknown() const {
     return getRawKind() == UnknownKind;
   }
@@ -94,6 +92,15 @@ public:
   
   bool isZeroConstant() const;
   
+  /// getAsLocSymbol - If this SVal is a location (subclasses Loc) and 
+  ///  wraps a symbol, return that SymbolRef.  Otherwise return a SymbolRef
+  ///  where 'isValid()' returns false.
+  SymbolRef getAsLocSymbol() const;
+  
+  /// getAsSymbol - If this Sval wraps a symbol return that SymbolRef.
+  ///  Otherwise return a SymbolRef where 'isValid()' returns false.
+  SymbolRef getAsSymbol() const;
+  
   void print(std::ostream& OS) const;
   void print(llvm::raw_ostream& OS) const;
   void printStdErr() const;
@@ -237,8 +244,9 @@ enum Kind { ConcreteIntKind, SymbolValKind, SymIntConstraintValKind,
 
 class SymbolVal : public NonLoc {
 public:
-  SymbolVal(unsigned SymID)
-    : NonLoc(SymbolValKind, reinterpret_cast<void*>((uintptr_t) SymID)) {}
+  SymbolVal(SymbolRef SymID)
+    : NonLoc(SymbolValKind,
+             reinterpret_cast<void*>((uintptr_t) SymID.getNumber())) {}
   
   SymbolRef getSymbol() const {
     return (SymbolRef) reinterpret_cast<uintptr_t>(Data);
@@ -370,8 +378,8 @@ enum Kind { SymbolValKind, GotoLabelKind, MemRegionKind, FuncValKind,
 
 class SymbolVal : public Loc {
 public:
-  SymbolVal(unsigned SymID)
-  : Loc(SymbolValKind, reinterpret_cast<void*>((uintptr_t) SymID)) {}
+  SymbolVal(SymbolRef SymID)
+  : Loc(SymbolValKind, reinterpret_cast<void*>((uintptr_t) SymID.getNumber())){}
   
   SymbolRef getSymbol() const {
     return (SymbolRef) reinterpret_cast<uintptr_t>(Data);

diff --git a/include/clang/Analysis/PathSensitive/SymbolManager.h b/include/clang/Analysis/PathSensitive/SymbolManager.h
index 18c177bdd6ecc6c03a8e29bf1e345b681a209813..91d3b348b9615f2f15e5e116d2e025425a514415 100644 (file)
--- a/include/clang/Analysis/PathSensitive/SymbolManager.h
+++ b/include/clang/Analysis/PathSensitive/SymbolManager.h
@@ -39,40 +39,45 @@ class SymbolRef {
 public:
   SymbolRef() : Data(~0U - 2) {}
   SymbolRef(unsigned x) : Data(x) {}
-    
-  bool isInitialized() const { return Data != (unsigned) (~0U - 2); }
-  operator unsigned() const { return getNumber(); }
-  unsigned getNumber() const { assert (isInitialized()); return Data; }
+
+  bool isValid() const { return Data != (unsigned) (~0U - 2);  }
+  unsigned getNumber() const { assert (isValid()); return Data; }
   
+  bool operator<(const SymbolRef& X) const { return Data < X.Data; }
+  bool operator>(const SymbolRef& X) const { return Data > X.Data; }
   bool operator==(const SymbolRef& X) const { return Data == X.Data; }
   bool operator!=(const SymbolRef& X) const { return Data != X.Data; }
-    
-  void print(llvm::raw_ostream& os) const;
-  
+      
   void Profile(llvm::FoldingSetNodeID& ID) const { 
-    assert (isInitialized());
+    assert (isValid());
     ID.AddInteger(Data);
   }
 };
-  
 } // end clang namespace
 
 namespace llvm {
-  template <> struct DenseMapInfo<clang::SymbolRef> {
-    static inline clang::SymbolRef getEmptyKey() {
-      return clang::SymbolRef(~0U);
-    }
-    static inline clang::SymbolRef getTombstoneKey() {
-      return clang::SymbolRef(~0U - 1);
-    }
-    static unsigned getHashValue(clang::SymbolRef X) {
-      return X.getNumber();
-    }
-    static bool isEqual(clang::SymbolRef X, clang::SymbolRef Y) {
-      return X.getNumber() == Y.getNumber();
-    }
-    static bool isPod() { return true; }
-  };
+  llvm::raw_ostream& operator<<(llvm::raw_ostream& Out, clang::SymbolRef Sym);
+}
+namespace std {
+  std::ostream& operator<<(std::ostream& Out, clang::SymbolRef Sym);
+}
+
+namespace llvm {
+template <> struct DenseMapInfo<clang::SymbolRef> {
+  static inline clang::SymbolRef getEmptyKey() {
+    return clang::SymbolRef(~0U);
+  }
+  static inline clang::SymbolRef getTombstoneKey() {
+    return clang::SymbolRef(~0U - 1);
+  }
+  static unsigned getHashValue(clang::SymbolRef X) {
+    return X.getNumber();
+  }
+  static bool isEqual(clang::SymbolRef X, clang::SymbolRef Y) {
+    return X == Y;
+  }
+  static bool isPod() { return true; }
+};
 }
 
 // SymbolData: Used to record meta data about symbols.

diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp
index 6f6b3c1566310c1e0f7ceb5b272d71f07baf26b9..e1265ded1710f507d35eeb0904c85247fc724c9a 100644 (file)
--- a/lib/Analysis/BugReporter.cpp
+++ b/lib/Analysis/BugReporter.cpp
@@ -531,7 +531,7 @@ public:
     else
       return true;
   
-    assert (ScanSym.isInitialized());
+    assert (ScanSym.isValid());
   
     if (!BR.isNotable(ScanSym))
       return true;

diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 59669976b1bd22cb5fcc69879800c59b4ff13d65..0f5e4269430bfe6049ffd402a005db55a5205a48 100644 (file)
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -1613,20 +1613,20 @@ void CFRefCount::EvalSummary(ExplodedNodeSet<GRState>& Dst,
   for (ExprIterator I = arg_beg; I != arg_end; ++I, ++idx) {    
     SVal V = state.GetSVal(*I);
     
-    if (isa<loc::SymbolVal>(V)) {
-      SymbolRef Sym = cast<loc::SymbolVal>(V).getSymbol();
+    SymbolRef Sym = V.getAsLocSymbol();
+    if (Sym.isValid())
       if (RefBindings::data_type* T = state.get<RefBindings>(Sym)) {
         state = Update(state, Sym, *T, GetArgE(Summ, idx), hasErr);
         if (hasErr) {
           ErrorExpr = *I;
           ErrorSym = Sym;
           break;
-        }
+        }        
+        continue;
       }
-    }  
-    else if (isa<Loc>(V)) {
-      if (loc::MemRegionVal* MR = dyn_cast<loc::MemRegionVal>(&V)) {
 
+    if (isa<Loc>(V)) {
+      if (loc::MemRegionVal* MR = dyn_cast<loc::MemRegionVal>(&V)) {
         if (GetArgE(Summ, idx) == DoNothingByRef)
           continue;
         
@@ -1650,15 +1650,11 @@ void CFRefCount::EvalSummary(ExplodedNodeSet<GRState>& Dst,
           R = dyn_cast<TypedRegion>(ATR->getSuperRegion());
         }
         
-        if (R) {
-          
+        if (R) {          
           // Is the invalidated variable something that we were tracking?
-          SVal X = state.GetSVal(Loc::MakeVal(R));
-          
-          if (isa<loc::SymbolVal>(X)) {
-            SymbolRef Sym = cast<loc::SymbolVal>(X).getSymbol();
+          SymbolRef Sym = state.GetSVal(Loc::MakeVal(R)).getAsLocSymbol();
+          if (Sym.isValid())
             state = state.remove<RefBindings>(Sym);
-          }
           
           // Set the value of the variable to be a conjured symbol.
           unsigned Count = Builder.getCurrentBlockCount();
@@ -1692,9 +1688,8 @@ void CFRefCount::EvalSummary(ExplodedNodeSet<GRState>& Dst,
   
   // Evaluate the effect on the message receiver.  
   if (!ErrorExpr && Receiver) {
-    SVal V = state.GetSVal(Receiver);
-    if (isa<loc::SymbolVal>(V)) {
-      SymbolRef Sym = cast<loc::SymbolVal>(V).getSymbol();
+    SymbolRef Sym = state.GetSVal(Receiver).getAsLocSymbol();
+    if (Sym.isValid()) {
       if (const RefVal* T = state.get<RefBindings>(Sym)) {
         state = Update(state, Sym, *T, GetReceiverE(Summ), hasErr);
         if (hasErr) {
@@ -1831,11 +1826,10 @@ void CFRefCount::EvalObjCMessageExpr(ExplodedNodeSet<GRState>& Dst,
     // FIXME: Wouldn't it be great if this code could be reduced?  It's just
     // a chain of lookups.
     const GRState* St = Builder.GetState(Pred);
-    SVal V = Eng.getStateManager().GetSVal(St, Receiver );
+    SVal V = Eng.getStateManager().GetSVal(St, Receiver);
 
-    if (isa<loc::SymbolVal>(V)) {
-      SymbolRef Sym = cast<loc::SymbolVal>(V).getSymbol();
-      
+    SymbolRef Sym = V.getAsLocSymbol();
+    if (Sym.isValid()) {
       if (const RefVal* T  = St->get<RefBindings>(Sym)) {
         QualType Ty = T->getType();
         
@@ -1979,16 +1973,16 @@ void CFRefCount::EvalReturn(ExplodedNodeSet<GRState>& Dst,
                             ExplodedNode<GRState>* Pred) {
   
   Expr* RetE = S->getRetValue();
-  if (!RetE) return;
+  if (!RetE)
+    return;
   
   GRStateRef state(Builder.GetState(Pred), Eng.getStateManager());
-  SVal V = state.GetSVal(RetE);
+  SymbolRef Sym = state.GetSVal(RetE).getAsLocSymbol();
   
-  if (!isa<loc::SymbolVal>(V))
+  if (!Sym.isValid())
     return;
-  
+
   // Get the reference count binding (if any).
-  SymbolRef Sym = cast<loc::SymbolVal>(V).getSymbol();
   const RefVal* T = state.get<RefBindings>(Sym);
   
   if (!T)
@@ -2461,27 +2455,21 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N,
       for (CallExpr::arg_iterator AI=CE->arg_begin(), AE=CE->arg_end();
            AI!=AE; ++AI, ++i) {
         
-        // Retrieve the value of the arugment.
-        SVal X = CurrSt.GetSVal(*AI);
-        
-        // Is it the symbol we're interested in?
-        if (!isa<loc::SymbolVal>(X) || 
-            Sym != cast<loc::SymbolVal>(X).getSymbol())
+        // Retrieve the value of the argument.  Is it the symbol
+        // we are interested in?
+        if (CurrSt.GetSVal(*AI).getAsLocSymbol() != Sym)
           continue;
-        
+
         // We have an argument.  Get the effect!
         AEffects.push_back(Summ->getArg(i));
       }
     }
     else if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) {      
-      if (Expr *receiver = ME->getReceiver()) {
-        SVal RetV = CurrSt.GetSVal(receiver);        
-        if (isa<loc::SymbolVal>(RetV) &&
-            Sym == cast<loc::SymbolVal>(RetV).getSymbol()) {
+      if (Expr *receiver = ME->getReceiver())
+        if (CurrSt.GetSVal(receiver).getAsLocSymbol() == Sym) {
           // The symbol we are tracking is the receiver.
           AEffects.push_back(Summ->getReceiverEffect());
         }
-      }
     }
   }
   
@@ -2596,14 +2584,11 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N,
   // Add the range by scanning the children of the statement for any bindings
   // to Sym.
   for (Stmt::child_iterator I = S->child_begin(), E = S->child_end(); I!=E; ++I)
-    if (Expr* Exp = dyn_cast_or_null<Expr>(*I)) {
-      SVal X = CurrSt.GetSVal(Exp);      
-      if (loc::SymbolVal* SV = dyn_cast<loc::SymbolVal>(&X))
-        if (SV->getSymbol() == Sym) {
-            P->addRange(Exp->getSourceRange());
-            break;
-        }
-    }
+    if (Expr* Exp = dyn_cast_or_null<Expr>(*I))
+      if (CurrSt.GetSVal(Exp).getAsLocSymbol() == Sym) {
+        P->addRange(Exp->getSourceRange());
+        break;
+      }
   
   return P;
 }
@@ -2619,17 +2604,11 @@ class VISIBILITY_HIDDEN FindUniqueBinding :
     FindUniqueBinding(SymbolRef sym) : Sym(sym), Binding(0), First(true) {}
     
   bool HandleBinding(StoreManager& SMgr, Store store, MemRegion* R, SVal val) {
-    if (const loc::SymbolVal* SV = dyn_cast<loc::SymbolVal>(&val)) {
-      if (SV->getSymbol() != Sym) 
-        return true;
-    }
-    else if (const nonloc::SymbolVal* SV=dyn_cast<nonloc::SymbolVal>(&val)) {
-      if (SV->getSymbol() != Sym)
-        return true;
-    }
-    else
+    SymbolRef SymV = val.getAsSymbol();
+    
+    if (!SymV.isValid() || SymV != Sym)
       return true;
-
+    
     if (Binding) {
       First = false;
       return false;
@@ -2731,20 +2710,16 @@ CFRefLeakReport::getEndPath(BugReporter& br, const ExplodedNode<GRState>* EndN){
       bool foundSymbol = false;
       
       // First check if 'S' itself binds to the symbol.
-      if (Expr *Ex = dyn_cast<Expr>(S)) {
-        SVal X = state.GetSVal(Ex);
-        if (isa<loc::SymbolVal>(X) && 
-            cast<loc::SymbolVal>(X).getSymbol() == Sym)
+      if (Expr *Ex = dyn_cast<Expr>(S))
+        if (state.GetSVal(Ex).getAsLocSymbol() == Sym)
           foundSymbol = true;
-      }
         
       if (!foundSymbol)
         for (Stmt::child_iterator I=S->child_begin(), E=S->child_end();
              I!=E; ++I)
           if (Expr *Ex = dyn_cast_or_null<Expr>(*I)) {
             SVal X = state.GetSVal(Ex);
-            if (isa<loc::SymbolVal>(X) && 
-                cast<loc::SymbolVal>(X).getSymbol() == Sym){
+            if (X.getAsLocSymbol() == Sym) {
               foundSymbol = true;        
               break;
             }
@@ -2848,8 +2823,8 @@ void CFRefCount::EvalEndPath(GRExprEngine& Eng,
     bool hasLeak = false;
     
     std::pair<GRStateRef, bool> X =
-    HandleSymbolDeath(Eng.getStateManager(), St, CodeDecl,
-                      (*I).first, (*I).second, hasLeak);
+      HandleSymbolDeath(Eng.getStateManager(), St, CodeDecl,
+                        (*I).first, (*I).second, hasLeak);
     
     St = X.first;
     if (hasLeak) Leaked.push_back(std::make_pair((*I).first, X.second));

diff --git a/lib/Analysis/MemRegion.cpp b/lib/Analysis/MemRegion.cpp
index b2d7ad2d8965a67128f817d81360f7e7c11163ab..f78d937df82ebde55802c4cc8657890213a418c4 100644 (file)
--- a/lib/Analysis/MemRegion.cpp
+++ b/lib/Analysis/MemRegion.cpp
@@ -171,8 +171,7 @@ void VarRegion::print(llvm::raw_ostream& os) const {
 }
 
 void SymbolicRegion::print(llvm::raw_ostream& os) const {
-  os << "SymRegion-";
-  sym.print(os);
+  os << "SymRegion-" << sym;
 }
 
 void FieldRegion::print(llvm::raw_ostream& os) const {

diff --git a/lib/Analysis/SVals.cpp b/lib/Analysis/SVals.cpp
index 2017c4bc6e8e6e6e63b1865ad2361beade01f56c..c12c2d277006176794efdbaf3eba42b642df2752 100644 (file)
--- a/lib/Analysis/SVals.cpp
+++ b/lib/Analysis/SVals.cpp
@@ -54,6 +54,42 @@ SVal::symbol_iterator SVal::symbol_end() const {
   return symbol_iterator();
 }
 
+/// getAsLocSymbol - If this SVal is a location (subclasses Loc) and 
+///  wraps a symbol, return that SymbolRef.  Otherwise return a SymbolRef
+///  where 'isValid()' returns false.
+SymbolRef SVal::getAsLocSymbol() const {
+  if (const loc::SymbolVal *X = dyn_cast<loc::SymbolVal>(this))
+    return X->getSymbol();
+
+  if (const loc::MemRegionVal *X = dyn_cast<loc::MemRegionVal>(this)) {
+    const MemRegion *R = X->getRegion();
+    
+    while (R) {
+      // Blast through region views.
+      if (const TypedViewRegion *View = dyn_cast<TypedViewRegion>(R)) {
+        R = View->getSuperRegion();
+        continue;
+      }
+      
+      if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(R))
+        return SymR->getSymbol();
+      
+      break;
+    }
+  }
+  
+  return SymbolRef();
+}
+
+/// getAsSymbol - If this Sval wraps a symbol return that SymbolRef.
+///  Otherwise return a SymbolRef where 'isValid()' returns false.
+SymbolRef SVal::getAsSymbol() const {
+  if (const nonloc::SymbolVal *X = dyn_cast<nonloc::SymbolVal>(this))
+    return X->getSymbol();
+  
+  return getAsLocSymbol();
+}
+
 //===----------------------------------------------------------------------===//
 // Other Iterators.
 //===----------------------------------------------------------------------===//

diff --git a/lib/Analysis/SymbolManager.cpp b/lib/Analysis/SymbolManager.cpp
index f1c1cc0a466e6d2679435b8f85ef3551299925cb..45e1aae23bc59614dc8b665ca849879f820b23e9 100644 (file)
--- a/lib/Analysis/SymbolManager.cpp
+++ b/lib/Analysis/SymbolManager.cpp
@@ -18,8 +18,23 @@
 
 using namespace clang;
 
-void SymbolRef::print(llvm::raw_ostream& os) const {
-  os << getNumber();
+llvm::raw_ostream& llvm::operator<<(llvm::raw_ostream& os,
+                                    clang::SymbolRef sym)  {
+  if (sym.isValid())
+    os << sym.getNumber();
+  else
+    os << "(Invalid)";
+  
+  return os;
+}
+
+std::ostream& std::operator<<(std::ostream& os, clang::SymbolRef sym) {
+  if (sym.isValid())
+    os << sym.getNumber();
+  else
+    os << "(Invalid)";
+  
+  return os;
 }
 
 SymbolRef SymbolManager::getRegionRValueSymbol(const MemRegion* R) {  
@@ -35,7 +50,6 @@ SymbolRef SymbolManager::getRegionRValueSymbol(const MemRegion* R) {
   DataSet.InsertNode(SD, InsertPos);
   DataMap[SymbolCounter] = SD;  
   return SymbolCounter++;
-
 }
 
 SymbolRef SymbolManager::getConjuredSymbol(Stmt* E, QualType T, unsigned Count){