Fixed: <rdar://problem/5881148>

Problem:

In the recently refactored VisitDeref (which processes dereferences), we
were incorrectly skipping the node just generated for the subexpression
of the dereference.  This was a horrible regression.

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

diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index 8d949f8c041ca3c7e449c53df9fbe55ee177b4c8..2d088c788d63cad85bbd4573dcfe3ead112a1d49 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -626,7 +626,7 @@ protected:
       return TF->EvalBinOp(*this, Op, cast<NonLVal>(L), cast<NonLVal>(R));
   }
   
-  void EvalCall(NodeSet& Dst, CallExpr* CE, LVal L, NodeTy* Pred) {
+  void EvalCall(NodeSet& Dst, CallExpr* CE, RVal L, NodeTy* Pred) {
     assert (Builder && "GRStmtNodeBuilder must be defined.");    
     TF->EvalCall(Dst, *this, *Builder, CE, L, Pred);
   }

diff --git a/include/clang/Analysis/PathSensitive/GRTransferFuncs.h b/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
index 33199a574c4c40959ba7cac5ad3fc8faa7ee92a7..47f5d2a59d07292209bee02298bc96e946b6a0e2 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
+++ b/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
@@ -63,7 +63,7 @@ public:
   virtual void EvalCall(ExplodedNodeSet<ValueState>& Dst,
                         GRExprEngine& Engine,
                         GRStmtNodeBuilder<ValueState>& Builder,
-                        CallExpr* CE, LVal L,
+                        CallExpr* CE, RVal L,
                         ExplodedNode<ValueState>* Pred) {}
   
   virtual void EvalObjCMessageExpr(ExplodedNodeSet<ValueState>& Dst,

diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 499da2f85f273c5998a38d83c6f5a01444f9d7f3..4b23ff47b9beb6b15bfe57d2a9beb41aaf2a96e7 100644 (file)
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -659,7 +659,7 @@ public:
   virtual void EvalCall(ExplodedNodeSet<ValueState>& Dst,
                         GRExprEngine& Eng,
                         GRStmtNodeBuilder<ValueState>& Builder,
-                        CallExpr* CE, LVal L,
+                        CallExpr* CE, RVal L,
                         ExplodedNode<ValueState>* Pred);  
   
   virtual void EvalObjCMessageExpr(ExplodedNodeSet<ValueState>& Dst,
@@ -769,7 +769,7 @@ void CFRefCount::ProcessNonLeakError(ExplodedNodeSet<ValueState>& Dst,
 void CFRefCount::EvalCall(ExplodedNodeSet<ValueState>& Dst,
                           GRExprEngine& Eng,
                           GRStmtNodeBuilder<ValueState>& Builder,
-                          CallExpr* CE, LVal L,
+                          CallExpr* CE, RVal L,
                           ExplodedNode<ValueState>* Pred) {
   
   ValueStateManager& StateMgr = Eng.getStateManager();

diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 6072094d057bac12565b97212cc03b3b52fd77c0..a6d8ef9051714e6f0297b1e1bffcf3058094db44 100644 (file)
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -78,6 +78,18 @@ struct VISIBILITY_HIDDEN SaveAndRestore {
   T old_value;
 };
 
+// SaveOr - Similar to SaveAndRestore.  Operates only on bools; the old
+//  value of a variable is saved, and during the dstor the old value is
+//  or'ed with the new value.
+struct VISIBILITY_HIDDEN SaveOr {
+  SaveOr(bool& x) : X(x), old_value(x) { x = false; }
+  ~SaveOr() { X |= old_value; }
+  
+  bool& X;
+  bool old_value;
+};
+
+
 void GRExprEngine::EmitWarnings(Diagnostic& Diag, PathDiagnosticClient* PD) {
   for (bug_type_iterator I = bug_types_begin(), E = bug_types_end(); I!=E; ++I){
     BugReporter BR(Diag, PD, getContext(), *this);
@@ -784,11 +796,9 @@ void GRExprEngine::EvalStore(NodeSet& Dst, Expr* E, NodeTy* Pred,
   
   unsigned size = Dst.size();  
 
-  SaveAndRestore<bool> OldSink(Builder->BuildSinks),
-                       OldHasGen(Builder->HasGeneratedNode);
+  SaveAndRestore<bool> OldSink(Builder->BuildSinks);
+  SaveOr OldHasGen(Builder->HasGeneratedNode);
 
-  Builder->HasGeneratedNode = false;
-  
   assert (!TargetLV.isUndef());
   
   TF->EvalStore(Dst, *this, *Builder, E, Pred, St, TargetLV, Val);
@@ -828,13 +838,10 @@ void GRExprEngine::VisitCall(CallExpr* CE, NodeTy* Pred,
   // the callee expression.
   
   NodeSet DstTmp;    
-  Expr* Callee = CE->getCallee()->IgnoreParenCasts();
+  Expr* Callee = CE->getCallee()->IgnoreParens();
 
   VisitLVal(Callee, Pred, DstTmp);
   
-  if (DstTmp.empty())
-    DstTmp.Add(Pred);
-  
   // Finally, evaluate the function call.
   for (NodeSet::iterator DI = DstTmp.begin(), DE = DstTmp.end(); DI!=DE; ++DI) {
 
@@ -903,19 +910,12 @@ void GRExprEngine::VisitCall(CallExpr* CE, NodeTy* Pred,
     }
     
     // Evaluate the call.
-    
-    
-    bool invalidateArgs = false;
-    
-    if (L.isUnknown()) {
-      // Check for an "unknown" callee.      
-      invalidateArgs = true;
-    }
-    else if (isa<lval::FuncVal>(L)) {
+
+    if (isa<lval::FuncVal>(L)) {
       
       IdentifierInfo* Info = cast<lval::FuncVal>(L).getDecl()->getIdentifier();
       
-      if (unsigned id = Info->getBuiltinID()) {
+      if (unsigned id = Info->getBuiltinID())
         switch (id) {
           case Builtin::BI__builtin_expect: {
             // For __builtin_expect, just return the value of the subexpression.
@@ -926,67 +926,45 @@ void GRExprEngine::VisitCall(CallExpr* CE, NodeTy* Pred,
           }
             
           default:
-            invalidateArgs = true;
             break;
         }
-      }
-    }
-        
-    if (invalidateArgs) {
-      // Invalidate all arguments passed in by reference (LVals).
-      for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end();
-                                                       I != E; ++I) {
-        RVal V = GetRVal(St, *I);
-
-        if (isa<LVal>(V))
-          St = SetRVal(St, cast<LVal>(V), UnknownVal());
-      }
-      
-      MakeNode(Dst, CE, *DI, St);
     }
-    else {
 
-      // Check any arguments passed-by-value against being undefined.
+    // Check any arguments passed-by-value against being undefined.
 
-      bool badArg = false;
-      
-      for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end();
-           I != E; ++I) {
+    bool badArg = false;
+    
+    for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end();
+         I != E; ++I) {
 
-        if (GetRVal(GetState(*DI), *I).isUndef()) {        
-          NodeTy* N = Builder->generateNode(CE, GetState(*DI), *DI);
-        
-          if (N) {
-            N->markAsSink();
-            UndefArgs[N] = *I;
-          }
-          
-          badArg = true;
-          break;
+      if (GetRVal(GetState(*DI), *I).isUndef()) {        
+        NodeTy* N = Builder->generateNode(CE, GetState(*DI), *DI);
+      
+        if (N) {
+          N->markAsSink();
+          UndefArgs[N] = *I;
         }
-      }
         
-      if (badArg)
-        continue;        
-      
-      // Dispatch to the plug-in transfer function.      
-      
-      unsigned size = Dst.size();
-      
-      SaveAndRestore<bool> OldSink(Builder->BuildSinks),
-                           OldHasGen(Builder->HasGeneratedNode);
-      
-      Builder->HasGeneratedNode = false;
-
-      EvalCall(Dst, CE, cast<LVal>(L), *DI);
-      
-      // Handle the case where no nodes where generated.  Auto-generate that
-      // contains the updated state if we aren't generating sinks.
-      
-      if (!Builder->BuildSinks && Dst.size() == size &&
-          !Builder->HasGeneratedNode)
-        MakeNode(Dst, CE, *DI, St);
+        badArg = true;
+        break;
+      }
     }
+    
+    if (badArg)
+      continue;        
+
+    // Dispatch to the plug-in transfer function.      
+    
+    unsigned size = Dst.size();
+    SaveOr OldHasGen(Builder->HasGeneratedNode);
+    EvalCall(Dst, CE, L, *DI);
+    
+    // Handle the case where no nodes where generated.  Auto-generate that
+    // contains the updated state if we aren't generating sinks.
+    
+    if (!Builder->BuildSinks && Dst.size() == size &&
+        !Builder->HasGeneratedNode)
+      MakeNode(Dst, CE, *DI, St);
   }
 }
 
@@ -1081,10 +1059,8 @@ void GRExprEngine::VisitObjCMessageExprDispatchHelper(ObjCMessageExpr* ME,
   
   unsigned size = Dst.size();
 
-  SaveAndRestore<bool> OldSink(Builder->BuildSinks),
-                       OldHasGen(Builder->HasGeneratedNode);
-  
-  Builder->HasGeneratedNode = false;
+  SaveAndRestore<bool> OldSink(Builder->BuildSinks);
+  SaveOr OldHasGen(Builder->HasGeneratedNode);
  
   EvalObjCMessageExpr(Dst, ME, Pred);
   
@@ -1312,9 +1288,9 @@ void GRExprEngine::VisitDeref(UnaryOperator* U, NodeTy* Pred,
     Visit(Ex, Pred, DstTmp);
   
   for (NodeSet::iterator I = DstTmp.begin(), DE = DstTmp.end(); I != DE; ++I) {
-    ValueState* St = GetState(Pred);
+    ValueState* St = GetState(*I);
     RVal V = GetRVal(St, Ex);
-    VisitDeref(U, V, St, Pred, Dst, GetLVal);
+    VisitDeref(U, V, St, *I, Dst, GetLVal);
   }
 }
 
@@ -1641,10 +1617,8 @@ void GRExprEngine::EvalReturn(NodeSet& Dst, ReturnStmt* S, NodeTy* Pred) {
   
   unsigned size = Dst.size();  
 
-  SaveAndRestore<bool> OldSink(Builder->BuildSinks),
-                       OldHasGen(Builder->HasGeneratedNode);
-  
-  Builder->HasGeneratedNode = false;
+  SaveAndRestore<bool> OldSink(Builder->BuildSinks);
+  SaveOr OldHasGen(Builder->HasGeneratedNode);
 
   TF->EvalReturn(Dst, *this, *Builder, S, Pred);
   

diff --git a/lib/Analysis/GRSimpleVals.cpp b/lib/Analysis/GRSimpleVals.cpp
index e41df7662c212c9114d05102b348fa420cece0ab..42cce94c9b788a8f65b7094d9b8778ef3e8b9f5a 100644 (file)
--- a/lib/Analysis/GRSimpleVals.cpp
+++ b/lib/Analysis/GRSimpleVals.cpp
@@ -557,7 +557,7 @@ RVal GRSimpleVals::EvalNE(GRExprEngine& Eng, LVal L, LVal R) {
 void GRSimpleVals::EvalCall(ExplodedNodeSet<ValueState>& Dst,
                             GRExprEngine& Eng,
                             GRStmtNodeBuilder<ValueState>& Builder,
-                            CallExpr* CE, LVal L,
+                            CallExpr* CE, RVal L,
                             ExplodedNode<ValueState>* Pred) {
   
   ValueStateManager& StateMgr = Eng.getStateManager();

diff --git a/lib/Analysis/GRSimpleVals.h b/lib/Analysis/GRSimpleVals.h
index 5adf895af885f931521e2f67f03b57a6bf625609..d86db51511cdcb3b4b1ab39c45b4dd995c6d0a9e 100644 (file)
--- a/lib/Analysis/GRSimpleVals.h
+++ b/lib/Analysis/GRSimpleVals.h
@@ -60,7 +60,7 @@ public:
   virtual void EvalCall(ExplodedNodeSet<ValueState>& Dst,
                         GRExprEngine& Engine,
                         GRStmtNodeBuilder<ValueState>& Builder,
-                        CallExpr* CE, LVal L,
+                        CallExpr* CE, RVal L,
                         ExplodedNode<ValueState>* Pred);
   
   virtual void EvalObjCMessageExpr(ExplodedNodeSet<ValueState>& Dst,