[cfe-commits] r65003 - in /cfe/trunk: include/clang/Analysis/PathSensitive/GRExprEngine.h lib/Analysis/GRExprEngine.cpp lib/Analysis/GRExprEngineInternalChecks.cpp

[Ted Kremenek]

Author: kremenek
Date: Wed Feb 18 22:06:22 2009
New Revision: 65003

URL: http://llvm.org/viewvc/llvm-project?rev=65003&view=rev
Log:
Implemented simple check in <rdar://problem/6600344>: When the receiver of a
message expression is nil and the return type is struct then the returned value
is undefined or potentially garbage.

Modified:
    cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
    cfe/trunk/lib/Analysis/GRExprEngine.cpp
    cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp

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=65003&r1=65002&r2=65003&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Wed Feb 18 22:06:22 2009
@@ -94,6 +94,16 @@
   typedef llvm::SmallPtrSet<NodeTy*,2> ErrorNodes;  
   typedef llvm::DenseMap<NodeTy*, Expr*> UndefArgsTy;
   
+  /// NilReceiverStructRetExplicit - Nodes in the ExplodedGraph that resulted
+  ///  from [x ...] with 'x' definitely being nil and the result was a 'struct'
+  //  (an undefined value).
+  ErrorNodes NilReceiverStructRetExplicit;
+  
+  /// NilReceiverStructRetImplicit - Nodes in the ExplodedGraph that resulted
+  ///  from [x ...] with 'x' possibly being nil and the result was a 'struct'
+  //  (an undefined value).
+  ErrorNodes NilReceiverStructRetImplicit;
+  
   /// RetsStackAddr - Nodes in the ExplodedGraph that result from returning
   ///  the address of a stack variable.
   ErrorNodes RetsStackAddr;
@@ -300,6 +310,16 @@
     return ImplicitNullDeref.end();
   }
   
+  typedef ErrorNodes::iterator nil_receiver_struct_ret_iterator;
+  
+  nil_receiver_struct_ret_iterator nil_receiver_struct_ret_begin() {
+    return NilReceiverStructRetExplicit.begin();
+  }
+
+  nil_receiver_struct_ret_iterator nil_receiver_struct_ret_end() {
+    return NilReceiverStructRetExplicit.end();
+  }
+  
   typedef ErrorNodes::iterator undef_deref_iterator;
   undef_deref_iterator undef_derefs_begin() { return UndefDeref.begin(); }
   undef_deref_iterator undef_derefs_end() { return UndefDeref.end(); }

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

==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Wed Feb 18 22:06:22 2009
@@ -1514,8 +1514,7 @@
     
     SVal L = GetSVal(state, Receiver);
     
-    // Check for undefined control-flow or calls to NULL.
-    
+    // Check for undefined control-flow.    
     if (L.isUndef()) {
       NodeTy* N = Builder->generateNode(ME, state, Pred);
       
@@ -1527,6 +1526,33 @@
       return;
     }
     
+    // "Assume" that the receiver is not NULL.    
+    bool isFeasibleNotNull = false;
+    Assume(state, L, true, isFeasibleNotNull);
+    
+    // "Assume" that the receiver is NULL.    
+    bool isFeasibleNull = false;
+    const GRState *StNull = Assume(state, L, false, isFeasibleNull);
+    
+    if (isFeasibleNull) {
+      // Check if the receiver was nil and the return value a struct.
+      if (ME->getType()->isRecordType()) {
+        // The [0 ...] expressions will return garbage.  Flag either an
+        // explicit or implicit error.  Because of the structure of this
+        // function we currently do not bifurfacte the state graph at
+        // this point.
+        // FIXME: We should bifurcate and fill the returned struct with
+        //  garbage.                
+        if (NodeTy* N = Builder->generateNode(ME, StNull, Pred)) {
+          N->markAsSink();
+          if (isFeasibleNotNull)
+            NilReceiverStructRetImplicit.insert(N);
+          else
+            NilReceiverStructRetExplicit.insert(N);
+        }
+      }
+    }
+    
     // Check if the "raise" message was sent.
     if (ME->getSelector() == RaiseSel)
       RaisesException = true;

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

==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp Wed Feb 18 22:06:22 2009
@@ -71,6 +71,34 @@
   }
 };
   
+class VISIBILITY_HIDDEN NilReceiverStructRet : public BugType {
+  GRExprEngine &Eng;
+public:
+  NilReceiverStructRet(GRExprEngine* eng) :
+    BugType("nil receiver with struct return type", "Logic Errors"),  
+    Eng(*eng) {}
+
+  void FlushReports(BugReporter& BR) {
+    for (GRExprEngine::nil_receiver_struct_ret_iterator
+          I=Eng.nil_receiver_struct_ret_begin(),
+          E=Eng.nil_receiver_struct_ret_end(); I!=E; ++I) {
+
+      std::string sbuf;
+      llvm::raw_string_ostream os(sbuf);
+      PostStmt P = cast<PostStmt>((*I)->getLocation());
+      ObjCMessageExpr *ME = cast<ObjCMessageExpr>(P.getStmt());
+      os << "The receiver in the message expression is 'nil' and results in the"
+            " returned value (of type '"
+         << ME->getType().getAsString()
+         << "') to be garbage or otherwise undefined.";
+
+      RangedBugReport *R = new RangedBugReport(*this, os.str().c_str(), *I);
+      R->addRange(ME->getReceiver()->getSourceRange());
+      BR.EmitReport(R);
+    }
+  }
+};
+  
 class VISIBILITY_HIDDEN UndefinedDeref : public BuiltinBug {
 public:
   UndefinedDeref(GRExprEngine* eng)
@@ -142,7 +170,8 @@
     for (GRExprEngine::UndefArgsTy::iterator I=Eng.msg_expr_undef_arg_begin(),
          E = Eng.msg_expr_undef_arg_end(); I!=E; ++I) {      
       // Generate a report for this bug.
-      RangedBugReport *report = new RangedBugReport(*this, desc.c_str(), I->first);
+      RangedBugReport *report = new RangedBugReport(*this, desc.c_str(),
+                                                    I->first);
       report->addRange(I->second->getSourceRange());
       BR.EmitReport(report);
     }    
@@ -431,6 +460,7 @@
   BR.Register(new BadReceiver(this));
   BR.Register(new OutOfBoundMemoryAccess(this));
   BR.Register(new BadSizeVLA(this));
+  BR.Register(new NilReceiverStructRet(this));
   
   // The following checks do not need to have their associated BugTypes
   // explicitly registered with the BugReporter.  If they issue any BugReports,