[analyzer] Inline ObjC class methods.

- Some cleanup(the TODOs) will be done after ObjC method inlining is
complete.
- Simplified CallEvent::getDefinition not to require ISDynamicDispatch
parameter.
- Also addressed Jordan's comments from r160530.

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

6 files changed, 214 insertions, 66 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h b/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h
index d4068275ee..adbf85957b 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h
@@ -121,15 +121,9 @@ public:
   const Decl *getDecl() const;
 
   /// \brief Returns the definition of the function or method that will be
-  /// called. May be null.
-  ///
-  /// This is used when deciding how to inline the call.
-  ///
-  /// \param IsDynamicDispatch True if the definition returned may not be the
-  ///   definition that is actually invoked at runtime. Note that if we have
-  ///   sufficient type information to devirtualize a dynamic method call,
-  ///   we will (and \p IsDynamicDispatch will be set to \c false).
-  const Decl *getDefinition(bool &IsDynamicDispatch) const;
+  /// called. Returns NULL if the definition cannot be found; ex: due to
+  /// dynamic dispatch in ObjC methods.
+  const Decl *getRuntimeDefinition() const;
 
   /// \brief Returns the expression whose value will be the result of this call.
   /// May be null.
@@ -289,8 +283,7 @@ public:
     return cast_or_null<FunctionDecl>(CallEvent::getDecl());
   }
 
-  const Decl *getDefinition(bool &IsDynamicDispatch) const {
-    IsDynamicDispatch = false;
+  const Decl *getRuntimeDefinition() const {
     const FunctionDecl *FD = getDecl();
     // Note that hasBody() will fill FD with the definition FunctionDecl.
     if (FD && FD->hasBody(FD))
@@ -371,7 +364,7 @@ protected:
     : SimpleCall(CE, St, LCtx, K) {}
 
 public:
-  const Decl *getDefinition(bool &IsDynamicDispatch) const;
+  const Decl *getRuntimeDefinition() const;
 
   static bool classof(const CallEvent *CA) {
     return CA->getKind() >= CE_BEG_CXX_INSTANCE_CALLS &&
@@ -457,8 +450,7 @@ public:
     return BR->getDecl();
   }
 
-  const Decl *getDefinition(bool &IsDynamicDispatch) const {
-    IsDynamicDispatch = false;
+  const Decl *getRuntimeDefinition() const {
     return getBlockDecl();
   }
 
@@ -551,7 +543,7 @@ public:
   unsigned getNumArgs() const { return 0; }
 
   SVal getCXXThisVal() const;
-  const Decl *getDefinition(bool &IsDynamicDispatch) const;
+  const Decl *getRuntimeDefinition() const;
 
   static bool classof(const CallEvent *CA) {
     return CA->getKind() == CE_CXXDestructor;
@@ -620,6 +612,8 @@ protected:
   void getExtraInvalidatedRegions(RegionList &Regions) const;
 
   QualType getDeclaredResultType() const;
+  ObjCMethodDecl *LookupClassMethodDefinition(Selector Sel,
+                                           ObjCInterfaceDecl *ClassDecl) const;
 
 public:
   ObjCMethodCall(const ObjCMessageExpr *Msg, ProgramStateRef St,
@@ -678,18 +672,23 @@ public:
     llvm_unreachable("Unknown message kind");
   }
 
-  const Decl *getDefinition(bool &IsDynamicDispatch) const {
-    IsDynamicDispatch = true;
-    
-    const ObjCMethodDecl *MD = getDecl();
-    if (!MD)
-      return 0;
+  // TODO: We might want to only compute this once (or change the API for 
+  // getting the parameters). Currently, this gets called 3 times during 
+  // inlining.
+  const Decl *getRuntimeDefinition() const {
 
-    for (Decl::redecl_iterator I = MD->redecls_begin(), E = MD->redecls_end();
-         I != E; ++I) {
-      if (cast<ObjCMethodDecl>(*I)->isThisDeclarationADefinition())
-        return *I;
+    const ObjCMessageExpr *E = getOriginExpr();
+    if (E->isInstanceMessage()) {
+      return 0;
+    } else {
+      // This is a calss method.
+      // If we have type info for the receiver class, we are calling via
+      // class name.
+      if (ObjCInterfaceDecl *IDecl = E->getReceiverInterface()) {
+        return LookupClassMethodDefinition(E->getSelector(), IDecl);
+      }
     }
+
     return 0;
   }
 
@@ -770,8 +769,8 @@ inline const Decl *CallEvent::getDecl() const {
   DISPATCH(getDecl);
 }
 
-inline const Decl *CallEvent::getDefinition(bool &IsDynamicDispatch) const {
-  DISPATCH_ARG(getDefinition, IsDynamicDispatch);
+inline const Decl *CallEvent::getRuntimeDefinition() const {
+  DISPATCH(getRuntimeDefinition);
 }
 
 inline unsigned CallEvent::getNumArgs() const {
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 4d0eedb9e2..3b9acd627e 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -471,7 +471,7 @@ public:
   void evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
                 const SimpleCall &Call);
 
-  /// \bried Default implementation of call evaluation.
+  /// \brief Default implementation of call evaluation.
   void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred,
                        const CallEvent &Call);
 private:
diff --git a/lib/StaticAnalyzer/Core/Calls.cpp b/lib/StaticAnalyzer/Core/Calls.cpp
index 22fea3298d..e71f91e604 100644
--- a/lib/StaticAnalyzer/Core/Calls.cpp
+++ b/lib/StaticAnalyzer/Core/Calls.cpp
@@ -201,8 +201,7 @@ bool CallEvent::mayBeInlined(const Stmt *S) {
 
 CallEvent::param_iterator
 AnyFunctionCall::param_begin(bool UseDefinitionParams) const {
-  bool IgnoredDynamicDispatch;
-  const Decl *D = UseDefinitionParams ? getDefinition(IgnoredDynamicDispatch)
+  const Decl *D = UseDefinitionParams ? getRuntimeDefinition()
                                       : getDecl();
   if (!D)
     return 0;
@@ -212,8 +211,7 @@ AnyFunctionCall::param_begin(bool UseDefinitionParams) const {
 
 CallEvent::param_iterator
 AnyFunctionCall::param_end(bool UseDefinitionParams) const {
-  bool IgnoredDynamicDispatch;
-  const Decl *D = UseDefinitionParams ? getDefinition(IgnoredDynamicDispatch)
+  const Decl *D = UseDefinitionParams ? getRuntimeDefinition()
                                       : getDecl();
   if (!D)
     return 0;
@@ -354,8 +352,8 @@ static const CXXMethodDecl *devirtualize(const CXXMethodDecl *MD, SVal ThisVal){
 }
 
 
-const Decl *CXXInstanceCall::getDefinition(bool &IsDynamicDispatch) const {
-  const Decl *D = SimpleCall::getDefinition(IsDynamicDispatch);
+const Decl *CXXInstanceCall::getRuntimeDefinition() const {
+  const Decl *D = SimpleCall::getRuntimeDefinition();
   if (!D)
     return 0;
 
@@ -368,8 +366,7 @@ const Decl *CXXInstanceCall::getDefinition(bool &IsDynamicDispatch) const {
   if (const CXXMethodDecl *Devirtualized = devirtualize(MD, getCXXThisVal()))
     return Devirtualized;
 
-  IsDynamicDispatch = true;
-  return MD;
+  return 0;
 }
 
 
@@ -458,8 +455,8 @@ void CXXDestructorCall::getExtraInvalidatedRegions(RegionList &Regions) const {
     Regions.push_back(static_cast<const MemRegion *>(Data));
 }
 
-const Decl *CXXDestructorCall::getDefinition(bool &IsDynamicDispatch) const {
-  const Decl *D = AnyFunctionCall::getDefinition(IsDynamicDispatch);
+const Decl *CXXDestructorCall::getRuntimeDefinition() const {
+  const Decl *D = AnyFunctionCall::getRuntimeDefinition();
   if (!D)
     return 0;
 
@@ -472,15 +469,13 @@ const Decl *CXXDestructorCall::getDefinition(bool &IsDynamicDispatch) const {
   if (const CXXMethodDecl *Devirtualized = devirtualize(MD, getCXXThisVal()))
     return Devirtualized;
 
-  IsDynamicDispatch = true;
-  return MD;
+  return 0;
 }
 
 
 CallEvent::param_iterator
 ObjCMethodCall::param_begin(bool UseDefinitionParams) const {
-  bool IgnoredDynamicDispatch;
-  const Decl *D = UseDefinitionParams ? getDefinition(IgnoredDynamicDispatch)
+  const Decl *D = UseDefinitionParams ? getRuntimeDefinition()
                                       : getDecl();
   if (!D)
     return 0;
@@ -490,8 +485,7 @@ ObjCMethodCall::param_begin(bool UseDefinitionParams) const {
 
 CallEvent::param_iterator
 ObjCMethodCall::param_end(bool UseDefinitionParams) const {
-  bool IgnoredDynamicDispatch;
-  const Decl *D = UseDefinitionParams ? getDefinition(IgnoredDynamicDispatch)
+  const Decl *D = UseDefinitionParams ? getRuntimeDefinition()
                                       : getDecl();
   if (!D)
     return 0;
@@ -593,3 +587,34 @@ ObjCMessageKind ObjCMethodCall::getMessageKind() const {
     return OCM_Message;
   return static_cast<ObjCMessageKind>(Info.getInt());
 }
+
+// TODO: This implementation is copied from SemaExprObjC.cpp, needs to be
+// factored into the ObjCInterfaceDecl.
+ObjCMethodDecl *ObjCMethodCall::LookupClassMethodDefinition(Selector Sel,
+                                           ObjCInterfaceDecl *ClassDecl) const {
+  ObjCMethodDecl *Method = 0;
+  // Lookup in class and all superclasses.
+  while (ClassDecl && !Method) {
+    if (ObjCImplementationDecl *ImpDecl = ClassDecl->getImplementation())
+      Method = ImpDecl->getClassMethod(Sel);
+
+    // Look through local category implementations associated with the class.
+    if (!Method)
+      Method = ClassDecl->getCategoryClassMethod(Sel);
+
+    // Before we give up, check if the selector is an instance method.
+    // But only in the root. This matches gcc's behavior and what the
+    // runtime expects.
+    if (!Method && !ClassDecl->getSuperClass()) {
+      Method = ClassDecl->lookupInstanceMethod(Sel);
+      // Look through local category implementations associated
+      // with the root class.
+      //if (!Method)
+      //  Method = LookupPrivateInstanceMethod(Sel, ClassDecl);
+    }
+
+    ClassDecl = ClassDecl->getSuperClass();
+  }
+  return Method;
+}
+
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index cc257eb38b..62c288dad4 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -271,9 +271,8 @@ bool ExprEngine::inlineCall(const CallEvent &Call,
   if (!getAnalysisManager().shouldInlineCall())
     return false;
 
-  bool IsDynamicDispatch;
-  const Decl *D = Call.getDefinition(IsDynamicDispatch);
-  if (!D || IsDynamicDispatch)
+  const Decl *D = Call.getRuntimeDefinition();
+  if (!D)
     return false;
 
   const LocationContext *CurLC = Pred->getLocationContext();
@@ -305,9 +304,7 @@ bool ExprEngine::inlineCall(const CallEvent &Call,
     break;
   }
   case CE_ObjCMessage:
-    // These always use dynamic dispatch; enabling inlining means assuming
-    // that a particular method will be called at runtime.
-    llvm_unreachable("Dynamic dispatch should be handled above.");
+    break;
   }
 
   if (!shouldInlineDecl(D, Pred))
@@ -435,7 +432,6 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
     case OMF_self: {
       // These methods return their receivers.
       return State->BindExpr(E, LCtx, Msg->getReceiverSVal());
-      break;
     }
     }
   }
@@ -457,15 +453,13 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
   // The origin expression here is just used as a kind of checksum;
   // for CallEvents that do not have origin expressions, this should still be
   // safe.
-  if (!isa<ObjCMethodCall>(Call)) {
-    State = getInlineFailedState(Pred->getState(), E);
-    if (State == 0 && inlineCall(Call, Pred)) {
-      // If we inlined the call, the successor has been manually added onto
-      // the work list and we should not consider it for subsequent call
-      // handling steps.
-      Bldr.takeNodes(Pred);
-      return;
-    }
+  State = getInlineFailedState(Pred->getState(), E);
+  if (State == 0 && inlineCall(Call, Pred)) {
+    // If we inlined the call, the successor has been manually added onto
+    // the work list and we should not consider it for subsequent call
+    // handling steps.
+    Bldr.takeNodes(Pred);
+    return;
   }
 
   // If we can't inline it, handle the return value and invalidate the regions.
diff --git a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
index a8e0b3b809..1e59fb117b 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -190,9 +190,6 @@ void ExprEngine::VisitObjCMessage(const ObjCMethodCall &msg,
         // Generate a transition to non-Nil state.
         if (notNilState != state)
           Pred = Bldr.generateNode(currentStmt, Pred, notNilState);
-
-        // Evaluate the call.
-        defaultEvalCall(Bldr, Pred, msg);
       }
     } else {
       // Check for special class methods.
@@ -242,10 +239,10 @@ void ExprEngine::VisitObjCMessage(const ObjCMethodCall &msg,
 
         }
       }
-
-      // Evaluate the call.
-      defaultEvalCall(Bldr, Pred, msg);
     }
+    // Evaluate the call.
+    defaultEvalCall(Bldr, Pred, msg);
+
   }
   
   ExplodedNodeSet dstPostvisit;
diff --git a/test/Analysis/inlining/InlineObjCClassMethod.m b/test/Analysis/inlining/InlineObjCClassMethod.m
new file mode 100644
index 0000000000..0765251550
--- /dev/null
+++ b/test/Analysis/inlining/InlineObjCClassMethod.m
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
+
+// Test inlining of ObjC class methods.
+
+typedef signed char BOOL;
+typedef struct objc_class *Class;
+typedef struct objc_object {
+    Class isa;
+} *id;
+@protocol NSObject  - (BOOL)isEqual:(id)object; @end
+@interface NSObject <NSObject> {}
++(id)alloc;
+-(id)init;
+-(id)autorelease;
+-(id)copy;
+- (Class)class;
+-(id)retain;
+@end
+
+// Vanila: ObjC class method is called by name.
+@interface MyParent : NSObject
++ (int)getInt;
+@end
+@interface MyClass : MyParent
++ (int)getInt;
+@end
+@implementation MyClass
++ (int)testClassMethodByName {
+    int y = [MyClass getInt];
+    return 5/y; // expected-warning {{Division by zero}}
+}
++ (int)getInt {
+  return 0;
+}
+@end
+
+// The definition is defined by the parent. Make sure we find it and inline.
+@interface MyParentDIP : NSObject
++ (int)getInt;
+@end
+@interface MyClassDIP : MyParentDIP
+@end
+@implementation MyClassDIP
++ (int)testClassMethodByName {
+    int y = [MyClassDIP getInt];
+    return 5/y; // expected-warning {{Division by zero}}
+}
+@end
+@implementation MyParentDIP
++ (int)getInt {
+    return 0;
+}
+@end
+
+// ObjC class method is called by name. Definition is in the category.
+@interface AAA : NSObject
+@end
+@interface AAA (MyCat)
++ (int)getInt;
+@end
+int foo() {
+    int y = [AAA getInt];
+    return 5/y; // expected-warning {{Division by zero}}
+}
+@implementation AAA
+@end
+@implementation AAA (MyCat)
++ (int)getInt {
+    return 0;
+}
+@end
+
+// There is no declaration in the class but there is one in the parent. Make 
+// sure we pick the definition from the class and not the parent.
+@interface MyParentTricky : NSObject
++ (int)getInt;
+@end
+@interface MyClassTricky : MyParentTricky
+@end
+@implementation MyParentTricky
++ (int)getInt {
+    return 0;
+}
+@end
+@implementation MyClassTricky
++ (int)getInt {
+  return 1;
+}
++ (int)testClassMethodByName {
+    int y = [MyClassTricky getInt];
+    return 5/y; // no-warning
+}
+@end
+
+// ObjC class method is called by unknown class declaration (passed in as a 
+// parameter). We should not inline in such case.
+@interface MyParentUnknown : NSObject
++ (int)getInt;
+@end
+@interface MyClassUnknown : MyParentUnknown
++ (int)getInt;
+@end
+@implementation MyClassUnknown
++ (int)testClassVariableByUnknownVarDecl: (Class)cl  {
+  int y = [cl getInt];
+  return 3/y; // no-warning
+}
++ (int)getInt {
+  return 0;
+}
+@end
+
+
+// False negative.
+// ObjC class method call through a decl with a known type.
+// We should be able to track the type of currentClass and inline this call.
+@interface MyClassKT : NSObject
+@end
+@interface MyClassKT (MyCatKT)
++ (int)getInt;
+@end
+@implementation MyClassKT (MyCatKT)
++ (int)getInt {
+    return 0;
+}
+@end
+@implementation MyClassKT
+- (int)testClassMethodByKnownVarDecl {
+  Class currentClass = [self class];
+  int y = [currentClass getInt];
+  return 5/y; // Would be great to get a warning here.
+}
+@end