Refactor argument checking in CallAndMessageChecker to be the same

for both CallExprs and ObjCMessageExprs.

llvm-svn: 98800

clang/lib/Checker/CallAndMessageChecker.cpp

@@ -44,6 +44,9 @@ public:
   bool EvalNilReceiver(CheckerContext &C, const ObjCMessageExpr *ME);
 
 private:
+  bool PreVisitProcessArg(CheckerContext &C, const Expr *Ex,
+                          const char *BT_desc, BugType *&BT);
+
   void EmitBadCall(BugType *BT, CheckerContext &C, const CallExpr *CE);
   void EmitNilReceiverBug(CheckerContext &C, const ObjCMessageExpr *ME,
                           ExplodedNode *N);
@@ -51,10 +54,9 @@ private:
   void HandleNilReceiver(CheckerContext &C, const GRState *state,
                          const ObjCMessageExpr *ME);
 
-  void LazyInit_BT_call_arg() {
+  void LazyInit_BT(const char *desc, BugType *&BT) {
-    if (!BT_call_arg)
+    if (!BT)
-      BT_call_arg = new BuiltinBug("Pass-by-value argument in function call"
+      BT = new BuiltinBug(desc);
-                                   " is undefined");
   }
 };
 } // end anonymous namespace
@@ -75,46 +77,28 @@ void CallAndMessageChecker::EmitBadCall(BugType *BT, CheckerContext &C,
   C.EmitReport(R);
 }
 
-void CallAndMessageChecker::PreVisitCallExpr(CheckerContext &C,
+bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
-                                             const CallExpr *CE){
+                                               const Expr *Ex,
+                                               const char *BT_desc,
+                                               BugType *&BT) {
 
-  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  const SVal &V = C.getState()->getSVal(Ex);
-  SVal L = C.getState()->getSVal(Callee);
 
-  if (L.isUndef()) {
-    if (!BT_call_undef)
-      BT_call_undef =
-        new BuiltinBug("Called function pointer is an undefined pointer value");
-    EmitBadCall(BT_call_undef, C, CE);
-    return;
-  }
-
-  if (isa<loc::ConcreteInt>(L)) {
-    if (!BT_call_null)
-      BT_call_null =
-        new BuiltinBug("Called function pointer is null (null dereference)");
-    EmitBadCall(BT_call_null, C, CE);
-  }
-
-  for (CallExpr::const_arg_iterator I = CE->arg_begin(), E = CE->arg_end();
-       I != E; ++I) {
-    const SVal &V = C.getState()->getSVal(*I);
   if (V.isUndef()) {
     if (ExplodedNode *N = C.GenerateSink()) {
-        LazyInit_BT_call_arg();
+      LazyInit_BT(BT_desc, BT);
 
       // Generate a report for this bug.
-        EnhancedBugReport *R = new EnhancedBugReport(*BT_call_arg,
+      EnhancedBugReport *R = new EnhancedBugReport(*BT, BT->getName(), N);
-                                                     BT_call_arg->getName(), N);
+      R->addRange(Ex->getSourceRange());
-        R->addRange((*I)->getSourceRange());
+      R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, Ex);
-        R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, *I);
       C.EmitReport(R);
-        return;
     }
+    return true;
   }
+
   if (const nonloc::LazyCompoundVal *LV =
         dyn_cast<nonloc::LazyCompoundVal>(&V)) {
-      const LazyCompoundValData *D = LV->getCVData();
 
     class FindUninitializedField {
     public:
@@ -156,6 +140,7 @@ void CallAndMessageChecker::PreVisitCallExpr(CheckerContext &C,
       }
     };
 
+    const LazyCompoundValData *D = LV->getCVData();
     FindUninitializedField F(C.getASTContext(),
                              C.getState()->getStateManager().getStoreManager(),
                              C.getValueManager().getRegionManager(),
@@ -163,13 +148,14 @@ void CallAndMessageChecker::PreVisitCallExpr(CheckerContext &C,
 
     if (F.Find(D->getRegion())) {
       if (ExplodedNode *N = C.GenerateSink()) {
-          LazyInit_BT_call_arg();
+        LazyInit_BT(BT_desc, BT);
         llvm::SmallString<512> Str;
         llvm::raw_svector_ostream os(Str);
         os << "Passed-by-value struct argument contains uninitialized data";
 
         if (F.FieldChain.size() == 1)
-            os << " (e.g., field: '" << F.FieldChain[0]->getNameAsString() << "')";
+          os << " (e.g., field: '" << F.FieldChain[0]->getNameAsString()
+             << "')";
         else {
           os << " (e.g., via the field chain: '";
           bool first = true;
@@ -185,17 +171,47 @@ void CallAndMessageChecker::PreVisitCallExpr(CheckerContext &C,
         }
 
         // Generate a report for this bug.
-          EnhancedBugReport *R =
+        EnhancedBugReport *R = new EnhancedBugReport(*BT, os.str(), N);
-            new EnhancedBugReport(*BT_call_arg, os.str(), N);
+        R->addRange(Ex->getSourceRange());
-          R->addRange((*I)->getSourceRange());
 
         // FIXME: enhance track back for uninitialized value for arbitrary
         // memregions
         C.EmitReport(R);
       }
+      return true;
     }
   }
+
+  return false;
+}
+
+void CallAndMessageChecker::PreVisitCallExpr(CheckerContext &C,
+                                             const CallExpr *CE){
+
+  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  SVal L = C.getState()->getSVal(Callee);
+
+  if (L.isUndef()) {
+    if (!BT_call_undef)
+      BT_call_undef =
+        new BuiltinBug("Called function pointer is an undefined pointer value");
+    EmitBadCall(BT_call_undef, C, CE);
+    return;
   }
+
+  if (isa<loc::ConcreteInt>(L)) {
+    if (!BT_call_null)
+      BT_call_null =
+        new BuiltinBug("Called function pointer is null (null dereference)");
+    EmitBadCall(BT_call_null, C, CE);
+  }
+
+  for (CallExpr::const_arg_iterator I = CE->arg_begin(), E = CE->arg_end();
+       I != E; ++I)
+    if (PreVisitProcessArg(C, *I,
+                           "Pass-by-value argument in function call is"
+                           " undefined", BT_call_arg))
+      return;
 }
 
 void CallAndMessageChecker::PreVisitObjCMessageExpr(CheckerContext &C,
@@ -221,23 +237,11 @@ void CallAndMessageChecker::PreVisitObjCMessageExpr(CheckerContext &C,
 
   // Check for any arguments that are uninitialized/undefined.
   for (ObjCMessageExpr::const_arg_iterator I = ME->arg_begin(),
-         E = ME->arg_end(); I != E; ++I) {
+         E = ME->arg_end(); I != E; ++I)
-    if (state->getSVal(*I).isUndef()) {
+    if (PreVisitProcessArg(C, *I,
-      if (ExplodedNode *N = C.GenerateSink()) {
+                           "Pass-by-value argument in message expression "
-        if (!BT_msg_arg)
+                           "is undefined", BT_msg_arg))
-          BT_msg_arg =
-            new BuiltinBug("Pass-by-value argument in message expression"
-                           " is undefined");
-        // Generate a report for this bug.
-        EnhancedBugReport *R = new EnhancedBugReport(*BT_msg_arg,
-                                                     BT_msg_arg->getName(), N);
-        R->addRange((*I)->getSourceRange());
-        R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, *I);
-        C.EmitReport(R);
         return;
-      }
-    }
-  }
 }
 
 bool CallAndMessageChecker::EvalNilReceiver(CheckerContext &C,

clang/test/Analysis/uninit-vals-ps-region.m

@@ -51,3 +51,12 @@ void test_uninit_struct_arg() {
   test_uninit_struct_arg_aux(x); // expected-warning{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'x')}}
 }
 
+@interface Foo
+- (void) passVal:(struct TestUninit)arg;
+@end
+void testFoo(Foo *o) {
+  struct TestUninit x;
+  [o passVal:x]; // expected-warning{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'x')}}
+}
+
+