New static analyzer check by Nikita Zhuk!

"The attached patch generates warnings of cases where an ObjC message is sent to
a nil object and the size of return type of that message is larger than the size
of void pointer. This may result in undefined return values as described in PR
2718.  The patch also includes test cases."

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

diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index f53a9abaad8cc5b76c0737ee9ec1cc012217294b..56e0e1e531131012cfda2ef9d3e341f51e064a59 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -113,6 +113,16 @@ public:
   //  (an undefined value).
   ErrorNodes NilReceiverStructRetImplicit;
   
+  /// NilReceiverLargerThanVoidPtrRetExplicit - Nodes in the ExplodedGraph that
+  /// resulted from [x ...] with 'x' definitely being nil and the result's size
+  // was larger than sizeof(void *) (an undefined value).
+  ErrorNodes NilReceiverLargerThanVoidPtrRetExplicit;
+
+  /// NilReceiverLargerThanVoidPtrRetImplicit - Nodes in the ExplodedGraph that
+  /// resulted from [x ...] with 'x' possibly being nil and the result's size
+  // was larger than sizeof(void *) (an undefined value).
+  ErrorNodes NilReceiverLargerThanVoidPtrRetImplicit;
+  
   /// RetsStackAddr - Nodes in the ExplodedGraph that result from returning
   ///  the address of a stack variable.
   ErrorNodes RetsStackAddr;
@@ -329,6 +339,18 @@ public:
     return NilReceiverStructRetExplicit.end();
   }
   
+  typedef ErrorNodes::iterator nil_receiver_larger_than_voidptr_ret_iterator;
+  
+  nil_receiver_larger_than_voidptr_ret_iterator
+  nil_receiver_larger_than_voidptr_ret_begin() {
+    return NilReceiverLargerThanVoidPtrRetExplicit.begin();
+  }
+
+  nil_receiver_larger_than_voidptr_ret_iterator
+  nil_receiver_larger_than_voidptr_ret_end() {
+    return NilReceiverLargerThanVoidPtrRetExplicit.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(); }

diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 487aac06c5823634962c70050092628c0daa216c..06d61cc4b010b2943ad6118b51acdb8baf78bc58 100644 (file)
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -1690,20 +1690,40 @@ void GRExprEngine::VisitObjCMessageExprDispatchHelper(ObjCMessageExpr* ME,
     
     if (isFeasibleNull) {
       // Check if the receiver was nil and the return value a struct.
-      if (ME->getType()->isRecordType() &&
-          BR.getParentMap().isConsumedExpr(ME)) {
-        // 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);
+      if (BR.getParentMap().isConsumedExpr(ME)) {
+        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);
+          }
+        }
+        else {
+          ASTContext& Ctx = getContext();
+          
+          // sizeof(void *)
+          const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy);
+          
+          // sizeof(return type)
+          const uint64_t returnTypeSize = Ctx.getTypeSize(ME->getType());
+          
+          if(voidPtrSize < returnTypeSize) {
+            if (NodeTy* N = Builder->generateNode(ME, StNull, Pred)) {
+              N->markAsSink();
+              if(isFeasibleNotNull)
+                NilReceiverLargerThanVoidPtrRetImplicit.insert(N);
+              else
+                NilReceiverLargerThanVoidPtrRetExplicit.insert(N);
+            }
+          }
         }
       }
     }

diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp
index 45baebd32bb69f795840a973a6c7cae73e4347ab..f4efdb1250f59b3c5a57857cea27908a7ba34860 100644 (file)
--- a/lib/Analysis/GRExprEngineInternalChecks.cpp
+++ b/lib/Analysis/GRExprEngineInternalChecks.cpp
@@ -98,6 +98,37 @@ public:
     }
   }
 };
+
+class VISIBILITY_HIDDEN NilReceiverLargerThanVoidPtrRet : public BugType {
+  GRExprEngine &Eng;
+public:
+  NilReceiverLargerThanVoidPtrRet(GRExprEngine* eng) :
+  BugType("'nil' receiver with return type larger than sizeof(void *)", 
+          "Logic Errors"),  
+  Eng(*eng) {}
+  
+  void FlushReports(BugReporter& BR) {
+    for (GRExprEngine::nil_receiver_larger_than_voidptr_ret_iterator
+         I=Eng.nil_receiver_larger_than_voidptr_ret_begin(),
+         E=Eng.nil_receiver_larger_than_voidptr_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()
+      << "' and of size "
+      << Eng.getContext().getTypeSize(ME->getType()) / 8
+      << " bytes) 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:
@@ -465,6 +496,7 @@ void GRExprEngine::RegisterInternalChecks() {
   BR.Register(new OutOfBoundMemoryAccess(this));
   BR.Register(new BadSizeVLA(this));
   BR.Register(new NilReceiverStructRet(this));
+  BR.Register(new NilReceiverLargerThanVoidPtrRet(this));
   
   // The following checks do not need to have their associated BugTypes
   // explicitly registered with the BugReporter.  If they issue any BugReports,

diff --git a/test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m b/test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m
new file mode 100644 (file)
index 0000000..a5e9c06
--- /dev/null
+++ b/test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m
@@ -0,0 +1,48 @@
+// RUN: clang-cc -analyze -checker-cfref -analyzer-constraints=basic -analyzer-store=basic %s -verify
+
+@interface MyClass {}
+- (void *)voidPtrM;
+- (int)intM;
+- (long long)longlongM;
+- (double)doubleM;
+- (long double)longDoubleM;
+@end
+@implementation MyClass
+- (void *)voidPtrM { return (void *)0; }
+- (int)intM { return 0; }
+- (long long)longlongM { return 0; }
+- (double)doubleM { return 0.0; }
+- (long double)longDoubleM { return 0.0; }
+@end
+
+void createFoo() {
+  MyClass *obj = 0;  
+  
+  void *v = [obj voidPtrM]; // no-warning
+  int i = [obj intM]; // no-warning
+}
+
+void createFoo2() {
+  MyClass *obj = 0;  
+  
+  long double ld = [obj longDoubleM]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value}}
+}
+
+void createFoo3() {
+  MyClass *obj = 0;  
+  
+  long long ll = [obj longlongM]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value}}
+}
+
+void createFoo4() {
+  MyClass *obj = 0;  
+  
+  double d = [obj doubleM]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value}}
+}
+
+void createFoo5() {
+  MyClass *obj = @"";  
+  
+  double d = [obj doubleM]; // no-warning
+}
+