[cfe-commits] r160768 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/Calls.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Core/ExprEngineObjC.cpp test/Analysis/inlining/ test/Analysis/inlining/InlineObjCClassMethod.m

Anna Zaks ganna at apple.com
Wed Jul 25 17:27:51 PDT 2012


Author: zaks
Date: Wed Jul 25 19:27:51 2012
New Revision: 160768

URL: http://llvm.org/viewvc/llvm-project?rev=160768&view=rev
Log:
[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.

Added:
    cfe/trunk/test/Analysis/inlining/
    cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h?rev=160768&r1=160767&r2=160768&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h Wed Jul 25 19:27:51 2012
@@ -121,15 +121,9 @@
   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 @@
     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 @@
     : 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 @@
     return BR->getDecl();
   }
 
-  const Decl *getDefinition(bool &IsDynamicDispatch) const {
-    IsDynamicDispatch = false;
+  const Decl *getRuntimeDefinition() const {
     return getBlockDecl();
   }
 
@@ -551,7 +543,7 @@
   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 @@
   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 @@
     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 @@
   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 {

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=160768&r1=160767&r2=160768&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Wed Jul 25 19:27:51 2012
@@ -471,7 +471,7 @@
   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:

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp?rev=160768&r1=160767&r2=160768&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp Wed Jul 25 19:27:51 2012
@@ -201,8 +201,7 @@
 
 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 @@
 
 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 @@
 }
 
 
-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 @@
   if (const CXXMethodDecl *Devirtualized = devirtualize(MD, getCXXThisVal()))
     return Devirtualized;
 
-  IsDynamicDispatch = true;
-  return MD;
+  return 0;
 }
 
 
@@ -458,8 +455,8 @@
     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 @@
   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 @@
 
 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 @@
     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;
+}
+

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=160768&r1=160767&r2=160768&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed Jul 25 19:27:51 2012
@@ -271,9 +271,8 @@
   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 @@
     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 @@
     case OMF_self: {
       // These methods return their receivers.
       return State->BindExpr(E, LCtx, Msg->getReceiverSVal());
-      break;
     }
     }
   }
@@ -457,15 +453,13 @@
   // 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.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp?rev=160768&r1=160767&r2=160768&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp Wed Jul 25 19:27:51 2012
@@ -190,9 +190,6 @@
         // 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 @@
 
         }
       }
-
-      // Evaluate the call.
-      defaultEvalCall(Bldr, Pred, msg);
     }
+    // Evaluate the call.
+    defaultEvalCall(Bldr, Pred, msg);
+
   }
   
   ExplodedNodeSet dstPostvisit;

Added: cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m?rev=160768&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m (added)
+++ cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m Wed Jul 25 19:27:51 2012
@@ -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;
+ at protocol NSObject  - (BOOL)isEqual:(id)object; @end
+ at interface NSObject <NSObject> {}
++(id)alloc;
+-(id)init;
+-(id)autorelease;
+-(id)copy;
+- (Class)class;
+-(id)retain;
+ at end
+
+// Vanila: ObjC class method is called by name.
+ at interface MyParent : NSObject
++ (int)getInt;
+ at end
+ at interface MyClass : MyParent
++ (int)getInt;
+ at end
+ at implementation MyClass
++ (int)testClassMethodByName {
+    int y = [MyClass getInt];
+    return 5/y; // expected-warning {{Division by zero}}
+}
++ (int)getInt {
+  return 0;
+}
+ at end
+
+// The definition is defined by the parent. Make sure we find it and inline.
+ at interface MyParentDIP : NSObject
++ (int)getInt;
+ at end
+ at interface MyClassDIP : MyParentDIP
+ at end
+ at implementation MyClassDIP
++ (int)testClassMethodByName {
+    int y = [MyClassDIP getInt];
+    return 5/y; // expected-warning {{Division by zero}}
+}
+ at end
+ at implementation MyParentDIP
++ (int)getInt {
+    return 0;
+}
+ at end
+
+// ObjC class method is called by name. Definition is in the category.
+ at interface AAA : NSObject
+ at end
+ at interface AAA (MyCat)
++ (int)getInt;
+ at end
+int foo() {
+    int y = [AAA getInt];
+    return 5/y; // expected-warning {{Division by zero}}
+}
+ at implementation AAA
+ at end
+ at implementation AAA (MyCat)
++ (int)getInt {
+    return 0;
+}
+ at 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.
+ at interface MyParentTricky : NSObject
++ (int)getInt;
+ at end
+ at interface MyClassTricky : MyParentTricky
+ at end
+ at implementation MyParentTricky
++ (int)getInt {
+    return 0;
+}
+ at end
+ at implementation MyClassTricky
++ (int)getInt {
+  return 1;
+}
++ (int)testClassMethodByName {
+    int y = [MyClassTricky getInt];
+    return 5/y; // no-warning
+}
+ at end
+
+// ObjC class method is called by unknown class declaration (passed in as a 
+// parameter). We should not inline in such case.
+ at interface MyParentUnknown : NSObject
++ (int)getInt;
+ at end
+ at interface MyClassUnknown : MyParentUnknown
++ (int)getInt;
+ at end
+ at implementation MyClassUnknown
++ (int)testClassVariableByUnknownVarDecl: (Class)cl  {
+  int y = [cl getInt];
+  return 3/y; // no-warning
+}
++ (int)getInt {
+  return 0;
+}
+ at 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.
+ at interface MyClassKT : NSObject
+ at end
+ at interface MyClassKT (MyCatKT)
++ (int)getInt;
+ at end
+ at implementation MyClassKT (MyCatKT)
++ (int)getInt {
+    return 0;
+}
+ at end
+ at implementation MyClassKT
+- (int)testClassMethodByKnownVarDecl {
+  Class currentClass = [self class];
+  int y = [currentClass getInt];
+  return 5/y; // Would be great to get a warning here.
+}
+ at end





More information about the cfe-commits mailing list