[analyzer] Support inlining of '[self classMethod]' and '[[self class] classMethod]'

[dcoughlin]

lib/StaticAnalyzer/Core/CallEvent.cpp

972 ↗

(On Diff #83720)

Here is a case where dispatching via the compile-time type of E is not safe:

#import <Foundation/Foundation.h>

void clang_analyzer_eval(int);

@interface Parent : NSObject
+ (int)a;
+ (int)b;
@end

@interface Child : Parent
@end

@interface Other : NSObject
+(void)run;
@end

int main(int argc, const char * argv[]) {
  @autoreleasepool {
    [Other run];
  }
  return 0;
}

@implementation Other
+(void)run {
  int result = [Child a];

  clang_analyzer_eval(result == 12);
  printf("result is %d\n", result);
}
@end

@implementation Parent
+ (int)a; {
  return [self b];
}
+ (int)b; {
  return 12;
}
@end

@implementation Child
+ (int)b; {
  return 100;
}
@end

Running this code will print 'result is 100' but the clang_analyzer_eval() will incorrectly yield 'TRUE'.

What do you think about adding a new SVal for ObjC 'Class' values that know what interface declaration they come from? Then 'self' in a class method would be filled in with something meaningful in ObjCMethodCall::getReceiverSVal() and we could do proper dynamic dispatch for class methods just like we do for instance methods now.

977 ↗

(On Diff #83720)

accosiate-->associate

test/Analysis/inlining/InlineObjCClassMethod.m

269 ↗

(On Diff #83720)

I think it would be good to duplicate some of these tests in -(void)instanceMethod since calling [self class] is most-commonly used in instance methods:

unsigned result2 = [[self class] returns30];
clang_analyzer_eval(result2 == 30); // expected-warning{{TRUE}}
unsigned result3 = [[super class] returns30];
clang_analyzer_eval(result3 == 100); // expected-warning{{UNKNOWN}}

cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp

while (true) {
     }
 
     return false;
   };
 
   llvm_unreachable("The while loop should always terminate.");
 }
 
+static const ObjCMethodDecl *findDefiningRedecl(const ObjCMethodDecl *MD) {
+  if (!MD)
+    return MD;
+
+  // Find the redeclaration that defines the method.
+  if (!MD->hasBody()) {
+    for (auto I : MD->redecls())
+      if (I->hasBody())
+        MD = cast<ObjCMethodDecl>(I);
+  }
+  return MD;
+}
+
+static bool isCallToSelfClass(const ObjCMessageExpr *ME) {
+  const Expr* InstRec = ME->getInstanceReceiver();
+  if (!InstRec)
+    return false;
+  const auto *InstRecIg = dyn_cast<DeclRefExpr>(InstRec->IgnoreParenImpCasts());
+
+  // Check that receiver is called 'self'.
+  if (!InstRecIg || !InstRecIg->getFoundDecl() ||
+      !InstRecIg->getFoundDecl()->getName().equals("self"))
+    return false;
+
+  // Check that the method name is 'class'.
+  if (ME->getSelector().getNumArgs() != 0 ||
+      !ME->getSelector().getNameForSlot(0).equals("class"))
+    return false;
+
+  return true;
+}
+
 RuntimeDefinition ObjCMethodCall::getRuntimeDefinition() const {
   const ObjCMessageExpr *E = getOriginExpr();
   assert(E);
   Selector Sel = E->getSelector();
 
   if (E->isInstanceMessage()) {
 
     // Find the receiver type.
     const ObjCObjectPointerType *ReceiverT = nullptr;
     bool CanBeSubClassed = false;
     QualType SupersType = E->getSuperType();
     const MemRegion *Receiver = nullptr;
 
     if (!SupersType.isNull()) {
+      // The receiver is guaranteed to be 'super' in this case.
       // Super always means the type of immediate predecessor to the method
       // where the call occurs.
       ReceiverT = cast<ObjCObjectPointerType>(SupersType);
     } else {
       Receiver = getReceiverSVal().getAsRegion();
       if (!Receiver)
         return RuntimeDefinition();
 
       DynamicTypeInfo DTI = getDynamicTypeInfo(getState(), Receiver);
       QualType DynType = DTI.getType();
       CanBeSubClassed = DTI.canBeASubClass();
-      ReceiverT = dyn_cast<ObjCObjectPointerType>(DynType);
+      ReceiverT = dyn_cast<ObjCObjectPointerType>(DynType.getCanonicalType());
 
       if (ReceiverT && CanBeSubClassed)
         if (ObjCInterfaceDecl *IDecl = ReceiverT->getInterfaceDecl())
           if (!canBeOverridenInSubclass(IDecl, Sel))
             CanBeSubClassed = false;
     }
 
-    // Lookup the method implementation.
+    // Handle special cases of '[self classMethod]' and
+    // '[[self class] classMethod]', which are treated by the compiler as
+    // instance (not class) messages. We will statically dispatch to those.
+    if (auto *PT = dyn_cast_or_null<ObjCObjectPointerType>(ReceiverT)) {
+      // For [self classMethod], return the compiler visible declaration.
+      if (PT->getObjectType()->isObjCClass() &&
+          Receiver == getSelfSVal().getAsRegion())
+        return RuntimeDefinition(findDefiningRedecl(E->getMethodDecl()));
+
+      // Similarly, handle [[self class] classMethod].
+      // TODO: We are currently doing a syntactic match for this pattern with is
+      // limiting as the test cases in Analysis/inlining/InlineObjCClassMethod.m
+      // shows. A better way would be to associate the meta type with the symbol
+      // using the dynamic type info tracking and use it here. We can add a new
+      // SVal for ObjC 'Class' values that know what interface declaration they
+      // come from. Then 'self' in a class method would be filled in with
+      // something meaningful in ObjCMethodCall::getReceiverSVal() and we could
+      // do proper dynamic dispatch for class methods just like we do for
+      // instance methods now.
+      if (E->getInstanceReceiver())
+        if (const auto *M = dyn_cast<ObjCMessageExpr>(E->getInstanceReceiver()))
+          if (isCallToSelfClass(M))
+            return RuntimeDefinition(findDefiningRedecl(E->getMethodDecl()));
+    }
+
+    // Lookup the instance method implementation.
     if (ReceiverT)
       if (ObjCInterfaceDecl *IDecl = ReceiverT->getInterfaceDecl()) {
         // Repeatedly calling lookupPrivateMethod() is expensive, especially
         // when in many cases it returns null.  We cache the results so
         // that repeated queries on the same ObjCIntefaceDecl and Selector
         // don't incur the same cost.  On some test cases, we can see the
         // same query being issued thousands of times.
         //

cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m

 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config ipa=dynamic-bifurcate -verify %s
 
 void clang_analyzer_checkInlined(int);
+void clang_analyzer_eval(int);
 
 // Test inlining of ObjC class methods.
 
 typedef signed char BOOL;
 typedef struct objc_class *Class;
 typedef struct objc_object {
     Class isa;
 } *id;

 // should keep it live. 
 @interface SelfUsedInParent : NSObject
 + (int)getNum;
 + (int)foo;
 @end
 @implementation SelfUsedInParent
 + (int)getNum {return 5;}
 + (int)foo {
-  return [self getNum];
+  int r = [self getNum];
+  clang_analyzer_eval(r == 5); // expected-warning{{TRUE}}
+  return r;
 }
 @end
 @interface SelfUsedInParentChild : SelfUsedInParent
 + (int)getNum;
 + (int)fooA;
 @end
 @implementation SelfUsedInParentChild
 + (int)getNum {return 0;}

 
 + (void)forwardDeclaredMethod {
   clang_analyzer_checkInlined(1); // expected-warning{{TRUE}}
 }
 
 + (void)forwardDeclaredVariadicMethod:(int)x, ... {
   clang_analyzer_checkInlined(0); // no-warning
 }
-
 @end
 
+@interface SelfClassTestParent : NSObject
+-(unsigned)returns10;
++(unsigned)returns20;
++(unsigned)returns30;
+@end
+
+@implementation SelfClassTestParent
+-(unsigned)returns10 { return 100; }
++(unsigned)returns20 { return 100; }
++(unsigned)returns30 { return 100; }
+@end
+
+@interface SelfClassTest : SelfClassTestParent
+-(unsigned)returns10;
++(unsigned)returns20;
++(unsigned)returns30;
+@end
+
+@implementation SelfClassTest
+-(unsigned)returns10 { return 10; }
++(unsigned)returns20 { return 20; }
++(unsigned)returns30 { return 30; }
++(void)classMethod {
+  unsigned result1 = [self returns20];
+  clang_analyzer_eval(result1 == 20); // expected-warning{{TRUE}}
+  unsigned result2 = [[self class] returns30];
+  clang_analyzer_eval(result2 == 30); // expected-warning{{TRUE}}
+  unsigned result3 = [[super class] returns30];
+  clang_analyzer_eval(result3 == 100); // expected-warning{{UNKNOWN}}
+}
+-(void)instanceMethod {
+  unsigned result0 = [self returns10];
+  clang_analyzer_eval(result0 == 10); // expected-warning{{TRUE}}
+  unsigned result2 = [[self class] returns30];
+  clang_analyzer_eval(result2 == 30); // expected-warning{{TRUE}}
+  unsigned result3 = [[super class] returns30];
+  clang_analyzer_eval(result3 == 100); // expected-warning{{UNKNOWN}}
+}
+@end
 
-
+@interface Parent : NSObject
++ (int)a;
++ (int)b;
+@end
+@interface Child : Parent
+@end
+@interface Other : NSObject
++(void)run;
+@end
+int main(int argc, const char * argv[]) {
+  @autoreleasepool {
+    [Other run];
+  }
+  return 0;
+}
+@implementation Other
++(void)run {
+  int result = [Child a];
+  // TODO: This should return 100.
+  clang_analyzer_eval(result == 12); // expected-warning{{TRUE}}
+}
+@end
+@implementation Parent
++ (int)a; {
+  return [self b];
+}
++ (int)b; {
+  return 12;
+}
+@end
+@implementation Child
++ (int)b; {
+  return 100;
+}
+@end