[cfe-commits] r50176 - in /cfe/trunk: include/clang/Analysis/PathSensitive/GRExprEngine.h include/clang/Analysis/PathSensitive/GRTransferFuncs.h lib/Analysis/CFRefCount.cpp lib/Analysis/GRExprEngine.cpp lib/Analysis/GRSimpleVals.cpp lib/Analysis/GRSimpleVals.h

Ted Kremenek kremenek at apple.com
Wed Apr 23 13:12:28 PDT 2008


Author: kremenek
Date: Wed Apr 23 15:12:28 2008
New Revision: 50176

URL: http://llvm.org/viewvc/llvm-project?rev=50176&view=rev
Log:
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.


Modified:
    cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
    cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
    cfe/trunk/lib/Analysis/CFRefCount.cpp
    cfe/trunk/lib/Analysis/GRExprEngine.cpp
    cfe/trunk/lib/Analysis/GRSimpleVals.cpp
    cfe/trunk/lib/Analysis/GRSimpleVals.h

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h?rev=50176&r1=50175&r2=50176&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Wed Apr 23 15:12:28 2008
@@ -626,7 +626,7 @@
       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);
   }

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h?rev=50176&r1=50175&r2=50176&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h Wed Apr 23 15:12:28 2008
@@ -63,7 +63,7 @@
   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,

Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=50176&r1=50175&r2=50176&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Wed Apr 23 15:12:28 2008
@@ -659,7 +659,7 @@
   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::EvalCall(ExplodedNodeSet<ValueState>& Dst,
                           GRExprEngine& Eng,
                           GRStmtNodeBuilder<ValueState>& Builder,
-                          CallExpr* CE, LVal L,
+                          CallExpr* CE, RVal L,
                           ExplodedNode<ValueState>* Pred) {
   
   ValueStateManager& StateMgr = Eng.getStateManager();

Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=50176&r1=50175&r2=50176&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Wed Apr 23 15:12:28 2008
@@ -78,6 +78,18 @@
   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 @@
   
   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 @@
   // 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 @@
     }
     
     // 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 @@
           }
             
           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;
+      if (GetRVal(GetState(*DI), *I).isUndef()) {        
+        NodeTy* N = Builder->generateNode(CE, GetState(*DI), *DI);
       
-      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 (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 @@
   
   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 @@
     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 @@
   
   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);
   

Modified: cfe/trunk/lib/Analysis/GRSimpleVals.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRSimpleVals.cpp?rev=50176&r1=50175&r2=50176&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/GRSimpleVals.cpp (original)
+++ cfe/trunk/lib/Analysis/GRSimpleVals.cpp Wed Apr 23 15:12:28 2008
@@ -557,7 +557,7 @@
 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();

Modified: cfe/trunk/lib/Analysis/GRSimpleVals.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRSimpleVals.h?rev=50176&r1=50175&r2=50176&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/GRSimpleVals.h (original)
+++ cfe/trunk/lib/Analysis/GRSimpleVals.h Wed Apr 23 15:12:28 2008
@@ -60,7 +60,7 @@
   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,





More information about the cfe-commits mailing list