[analyzer] Move handling of hardcoded noreturn ("panic") methods from CFRefCount to NoReturnFunctionChecker. No functionality change intended.

llvm-svn: 138210

clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp

@@ -17,15 +17,18 @@
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/DataTypes.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 
-class NoReturnFunctionChecker : public Checker< check::PostStmt<CallExpr> > {
+class NoReturnFunctionChecker : public Checker< check::PostStmt<CallExpr>,
+                                                check::PostObjCMessage > {
 public:
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
+  void checkPostObjCMessage(const ObjCMessage &msg, CheckerContext &C) const;
 };
 
 }
@@ -76,6 +79,67 @@ void NoReturnFunctionChecker::checkPostStmt(const CallExpr *CE,
     C.generateSink(CE);
 }
 
+static bool END_WITH_NULL isMultiArgSelector(Selector Sel, ...) {
+  va_list argp;
+  va_start(argp, Sel);
+
+  unsigned Slot = 0;
+  const char *Arg;
+  while ((Arg = va_arg(argp, const char *))) {
+    if (!Sel.getNameForSlot(Slot).equals(Arg))
+      break; // still need to va_end!
+    ++Slot;
+  }
+
+  va_end(argp);
+
+  // We only succeeded if we made it to the end of the argument list.
+  return (Arg == NULL);
+}
+
+void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMessage &Msg,
+                                                   CheckerContext &C) const {
+  // HACK: This entire check is to handle two messages in the Cocoa frameworks:
+  // -[NSAssertionHandler
+  //    handleFailureInMethod:object:file:lineNumber:description:]
+  // -[NSAssertionHandler
+  //    handleFailureInFunction:file:lineNumber:description:]
+  // Eventually these should be annotated with __attribute__((noreturn)).
+  // Because ObjC messages use dynamic dispatch, it is not generally safe to
+  // assume certain methods can't return. In cases where it is definitely valid,
+  // see if you can mark the methods noreturn or analyzer_noreturn instead of
+  // adding more explicit checks to this method.
+
+  if (!Msg.isInstanceMessage())
+    return;
+
+  const ObjCInterfaceDecl *Receiver = Msg.getReceiverInterface();
+  if (!Receiver)
+    return;
+  if (!Receiver->getIdentifier()->isStr("NSAssertionHandler"))
+    return;
+
+  Selector Sel = Msg.getSelector();
+  switch (Sel.getNumArgs()) {
+  default:
+    return;
+  case 4:
+    if (!isMultiArgSelector(Sel, "handleFailureInFunction", "file",
+                            "lineNumber", "description", NULL))
+      return;
+    break;
+  case 5:
+    if (!isMultiArgSelector(Sel, "handleFailureInMethod", "object", "file",
+                            "lineNumber", "description", NULL))
+      return;
+    break;
+  }
+
+  // If we got here, it's one of the messages we care about.
+  C.generateSink();
+}
+
+
 void ento::registerNoReturnFunctionChecker(CheckerManager &mgr) {
   mgr.registerChecker<NoReturnFunctionChecker>();
 }

clang/lib/StaticAnalyzer/Core/CFRefCount.cpp

@@ -431,15 +431,10 @@ class RetainSummary {
   ///  alias of one of the arguments in the call, and so on.
   RetEffect   Ret;
 
-  /// EndPath - Indicates that execution of this method/function should
-  ///  terminate the simulation of a path.
-  bool EndPath;
-
 public:
   RetainSummary(ArgEffects A, RetEffect R, ArgEffect defaultEff,
-                ArgEffect ReceiverEff, bool endpath = false)
-    : Args(A), DefaultArgEffect(defaultEff), Receiver(ReceiverEff), Ret(R),
-      EndPath(endpath) {}
+                ArgEffect ReceiverEff)
+    : Args(A), DefaultArgEffect(defaultEff), Receiver(ReceiverEff), Ret(R) {}
 
   /// getArg - Return the argument effect on the argument specified by
   ///  idx (starting from 0).
@@ -465,10 +460,6 @@ public:
   /// setRetEffect - Set the effect of the return value of the call.
   void setRetEffect(RetEffect E) { Ret = E; }
 
-  /// isEndPath - Returns true if executing the given method/function should
-  ///  terminate the path.
-  bool isEndPath() const { return EndPath; }
-
   
   /// Sets the effect on the receiver of the message.
   void setReceiverEffect(ArgEffect e) { Receiver = e; }
@@ -692,8 +683,7 @@ public:
 
   RetainSummary* getPersistentSummary(ArgEffects AE, RetEffect RetEff,
                                       ArgEffect ReceiverEff = DoNothing,
-                                      ArgEffect DefaultEff = MayEscape,
-                                      bool isEndPath = false);
+                                      ArgEffect DefaultEff = MayEscape);
 
   RetainSummary* getPersistentSummary(RetEffect RE,
                                       ArgEffect ReceiverEff = DoNothing,
@@ -774,16 +764,6 @@ private:
     va_end(argp);
   }
 
-  void addPanicSummary(const char* Cls, ...) {
-    RetainSummary* Summ = getPersistentSummary(AF.getEmptyMap(),
-                                               RetEffect::MakeNoRet(),
-                                               DoNothing,  DoNothing, true);
-    va_list argp;
-    va_start (argp, Cls);
-    addMethodSummary(&Ctx.Idents.get(Cls), ObjCMethodSummaries, Summ, argp);
-    va_end(argp);
-  }
-
 public:
 
   RetainSummaryManager(ASTContext &ctx, bool gcenabled, bool usesARC)
@@ -899,11 +879,10 @@ ArgEffects RetainSummaryManager::getArgEffects() {
 RetainSummary*
 RetainSummaryManager::getPersistentSummary(ArgEffects AE, RetEffect RetEff,
                                            ArgEffect ReceiverEff,
-                                           ArgEffect DefaultEff,
-                                           bool isEndPath) {
+                                           ArgEffect DefaultEff) {
   // Create the summary and return it.
   RetainSummary *Summ = (RetainSummary*) BPAlloc.Allocate<RetainSummary>();
-  new (Summ) RetainSummary(AE, RetEff, DefaultEff, ReceiverEff, isEndPath);
+  new (Summ) RetainSummary(AE, RetEff, DefaultEff, ReceiverEff);
   return Summ;
 }
 
@@ -1569,13 +1548,6 @@ void RetainSummaryManager::InitializeMethodSummaries() {
   // exit a method.
   addClassMethSummary("NSAutoreleasePool", "alloc", NoTrackYet);
 
-  // Create NSAssertionHandler summaries.
-  addPanicSummary("NSAssertionHandler", "handleFailureInFunction", "file",
-                  "lineNumber", "description", NULL);
-
-  addPanicSummary("NSAssertionHandler", "handleFailureInMethod", "object",
-                  "file", "lineNumber", "description", NULL);
-
   // Create summaries QCRenderer/QCView -createSnapShotImageOfType:
   addInstMethSummary("QCRenderer", AllocSumm,
                      "createSnapshotImageOfType", NULL);
@@ -2846,10 +2818,7 @@ void CFRefCount::evalSummary(ExplodedNodeSet &Dst,
     }
   }
 
-  // Generate a sink node if we are at the end of a path.
-  ExplodedNode *NewNode =
-    Summ.isEndPath() ? Builder.MakeSinkNode(Dst, Ex, Pred, state)
-                     : Builder.MakeNode(Dst, Ex, Pred, state);
+  ExplodedNode *NewNode = Builder.MakeNode(Dst, Ex, Pred, state);
 
   // Annotate the edge with summary we used.
   if (NewNode) SummaryLog[NewNode] = &Summ;

clang/test/Analysis/CFRetainRelease_NSAssertionHandler.m

@@ -20,6 +20,7 @@ extern id NSAllocateObject(Class aClass, NSUInteger extraBytes, NSZone *zone);
 @interface NSAssertionHandler : NSObject {}
 + (NSAssertionHandler *)currentHandler;
 - (void)handleFailureInMethod:(SEL)selector object:(id)object file:(NSString *)fileName lineNumber:(NSInteger)line description:(NSString *)format,...;
+- (void)handleFailureInFunction:(NSString *)functionName file:(NSString *)fileName lineNumber:(NSInteger)line description:(NSString *)format,...;
 @end
 extern NSString * const NSConnectionReplyMode;
 
@@ -63,3 +64,10 @@ extern NSString * const NSConnectionReplyMode;
 }
 
 @end
+
+void pointerFunction (int *x) {
+  // Manual expansion of NSCAssert( x != 0, @"")
+  do { if (!((x != 0))) { [[NSAssertionHandler currentHandler] handleFailureInFunction:[NSString stringWithUTF8String:__PRETTY_FUNCTION__] file:[NSString stringWithUTF8String:__FILE__] lineNumber:__LINE__ description:((@""))]; } } while(0);  
+
+  *x = 1; // no-warning
+}