Add new check: -check-objc-methodsigs. This check scans methods in

ObjCImplementationDecls and sees if a ancestor class defines a method with the
same selector but with a different type signature. Right now it just compares
return types, and mainly looks at differences in primitive values. The checking
will be expanded in the future.

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

diff --git a/Driver/AnalysisConsumer.cpp b/Driver/AnalysisConsumer.cpp
index 44d96a00501b7638fa2d24f740514a055f1c609e..28ebe1d11b2d61c9e6d0b976e02d7f2a5101e524 100644 (file)
--- a/Driver/AnalysisConsumer.cpp
+++ b/Driver/AnalysisConsumer.cpp
@@ -390,6 +390,13 @@ static void ActionCheckObjCDealloc(AnalysisManager& mgr) {
                    mgr.getLangOptions(), BR);  
 }
 
+static void ActionCheckObjCInstMethSignature(AnalysisManager& mgr) {
+  BugReporter BR(mgr);
+  
+  CheckObjCInstMethSignature(cast<ObjCImplementationDecl>(mgr.getCodeDecl()),
+                             BR);
+}
+
 //===----------------------------------------------------------------------===//
 // AnalysisConsumer creation.
 //===----------------------------------------------------------------------===//
@@ -416,6 +423,10 @@ ASTConsumer* clang::CreateAnalysisConsumer(Analyses* Beg, Analyses* End,
       case WarnUninitVals:
         C->addCodeAction(&ActionUninitVals);
         break;
+        
+      case CheckObjCMethSigs:
+        C->addObjCImplementationAction(&ActionCheckObjCInstMethSignature);
+        break;
       
       case DisplayLiveVariables:
         C->addCodeAction(&ActionLiveness);
@@ -442,8 +453,8 @@ ASTConsumer* clang::CreateAnalysisConsumer(Analyses* Beg, Analyses* End,
   
   // Checks we always perform:
   if (lopts.getGCMode() != LangOptions::GCOnly)
-    C->addObjCImplementationAction(&ActionCheckObjCDealloc);  
-  
+    C->addObjCImplementationAction(&ActionCheckObjCDealloc);
+    
   return C.take();
 }
 

diff --git a/Driver/AnalysisConsumer.h b/Driver/AnalysisConsumer.h
index 83f3cf67eb2c0c79ffb213c9c51b875ee2bb7dd0..3a1278a3dae0309ef09478fee9403f4f61c72996 100644 (file)
--- a/Driver/AnalysisConsumer.h
+++ b/Driver/AnalysisConsumer.h
@@ -23,7 +23,8 @@ enum Analyses {
   WarnUninitVals,
   DisplayLiveVariables,
   CheckerCFRef,
-  CheckerSimple
+  CheckerSimple,
+  CheckObjCMethSigs
 };
   
 ASTConsumer* CreateAnalysisConsumer(Analyses* Beg, Analyses* End,

diff --git a/Driver/clang.cpp b/Driver/clang.cpp
index 6efab7486420aa77773f8ae483796ee2de19f706..d92d5d62e6310c415e891b36d7cabbf2bbe49435 100644 (file)
--- a/Driver/clang.cpp
+++ b/Driver/clang.cpp
@@ -170,6 +170,8 @@ clEnumValN(WarnDeadStores, "warn-dead-stores",
            "Flag warnings of stores to dead variables"),
 clEnumValN(WarnUninitVals, "warn-uninit-values",
            "Flag warnings of uses of unitialized variables"),
+clEnumValN(CheckObjCMethSigs, "check-objc-methodsigs",
+      "Check the Objective-C method signatures for type incompatibilities."),
 clEnumValN(CheckerSimple, "checker-simple",
            "Perform simple path-sensitive checks."),
 clEnumValN(CheckerCFRef, "checker-cfref",

diff --git a/include/clang/Analysis/LocalCheckers.h b/include/clang/Analysis/LocalCheckers.h
index acf789dd5b7d5c73ec56b7f3fcfdf6c511d3309a..a8669d30a7c43b41a3c090418327d21f027f3d73 100644 (file)
--- a/include/clang/Analysis/LocalCheckers.h
+++ b/include/clang/Analysis/LocalCheckers.h
@@ -43,7 +43,8 @@ GRTransferFuncs* MakeCFRefCountTF(ASTContext& Ctx, bool GCEnabled,
   
 void CheckObjCDealloc(ObjCImplementationDecl* D, const LangOptions& L,
                       BugReporter& BR);
-
+  
+void CheckObjCInstMethSignature(ObjCImplementationDecl* ID, BugReporter& BR);
   
 } // end namespace clang
 

diff --git a/lib/Analysis/CheckObjCInstMethSignature.cpp b/lib/Analysis/CheckObjCInstMethSignature.cpp
new file mode 100644 (file)
index 0000000..2572edb
--- /dev/null
+++ b/lib/Analysis/CheckObjCInstMethSignature.cpp
@@ -0,0 +1,125 @@
+//=- CheckObjCInstMethodRetTy.cpp - Check ObjC method signatures -*- C++ -*-==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines a CheckObjCInstMethSignature, a flow-insenstive check
+//  that determines if an Objective-C class interface incorrectly redefines
+//  the method signature in a subclass.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/LocalCheckers.h"
+#include "clang/Analysis/PathDiagnostic.h"
+#include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/ASTContext.h"
+
+#include "llvm/ADT/DenseMap.h"
+#include <sstream>
+
+using namespace clang;
+
+static bool AreTypesCompatible(QualType Derived, QualType Ancestor,
+                               ASTContext& C) {
+
+  // Right now don't compare the compatibility of pointers.  That involves
+  // looking at subtyping relationships.  FIXME: Future patch.
+  if ((Derived->isPointerType() || Derived->isObjCQualifiedIdType())  && 
+      (Ancestor->isPointerType() || Ancestor->isObjCQualifiedIdType()))
+    return true;
+
+  return C.typesAreCompatible(Derived, Ancestor);
+}
+
+static void CompareReturnTypes(ObjCMethodDecl* MethDerived,
+                               ObjCMethodDecl* MethAncestor,
+                               BugReporter& BR, ASTContext& Ctx,
+                               ObjCImplementationDecl* ID) {
+    
+  QualType ResDerived  = MethDerived->getResultType();
+  QualType ResAncestor = MethAncestor->getResultType(); 
+  
+  if (!AreTypesCompatible(ResDerived, ResAncestor, Ctx)) {
+    std::ostringstream os;
+    
+    os << "The Objective-C class '"
+       << MethDerived->getClassInterface()->getName()
+       << "', which is derived from class '"
+       << MethAncestor->getClassInterface()->getName()
+       << "', defines the instance method '"
+       << MethDerived->getSelector().getName()
+       << "' whose return type is '"
+       << ResDerived.getAsString()
+       << "'.  The same method (same selector) is defined in class 'B' and has "
+          "a return type of '"
+       << ResAncestor.getAsString()
+       << "'.  These two types are incompatible, and may result in undefined "
+          "behavior for clients of these classes.";
+    
+    // Refactor.
+    SimpleBugType BT("incompatible instance method return type");
+    DiagCollector C(BT);
+    Diagnostic& Diag = BR.getDiagnostic();    
+    Diag.Report(&C, Ctx.getFullLoc(MethDerived->getLocStart()),
+                Diag.getCustomDiagID(Diagnostic::Warning, os.str().c_str()),
+                NULL, 0, NULL, 0);
+    
+    for (DiagCollector::iterator I = C.begin(), E = C.end(); I != E; ++I)
+      BR.EmitWarning(*I);
+  }
+}
+
+void clang::CheckObjCInstMethSignature(ObjCImplementationDecl* ID,
+                                       BugReporter& BR) {
+  
+  ObjCInterfaceDecl* D = ID->getClassInterface();
+  ObjCInterfaceDecl* C = D->getSuperClass();
+
+  if (!C)
+    return;
+  
+  // Build a DenseMap of the methods for quick querying.
+  typedef llvm::DenseMap<Selector,ObjCMethodDecl*> MapTy;
+  MapTy IMeths;
+  unsigned NumMethods = 0;
+  
+  for (ObjCImplementationDecl::instmeth_iterator I=ID->instmeth_begin(),
+       E=ID->instmeth_end(); I!=E; ++I) {    
+    
+    ObjCMethodDecl* M = *I;
+    IMeths[M->getSelector()] = M;
+    ++NumMethods;
+  }
+
+  // Now recurse the class hierarchy chain looking for methods with the
+  // same signatures.
+  ASTContext& Ctx = BR.getContext();
+  
+  while (C && NumMethods) {
+    for (ObjCInterfaceDecl::instmeth_iterator I=C->instmeth_begin(),
+         E=C->instmeth_end(); I!=E; ++I) {
+
+      ObjCMethodDecl* M = *I;
+      Selector S = M->getSelector();
+      
+      MapTy::iterator MI = IMeths.find(S);
+
+      if (MI == IMeths.end() || MI->second == 0)
+        continue;
+      
+      --NumMethods;
+      ObjCMethodDecl* MethDerived = MI->second;
+      MI->second = 0;
+      
+      CompareReturnTypes(MethDerived, M, BR, Ctx, ID);
+    }
+    
+    C = C->getSuperClass();
+  }
+}

diff --git a/test/Analysis/ObjCRetSigs.m b/test/Analysis/ObjCRetSigs.m
new file mode 100644 (file)
index 0000000..24b0f3a
--- /dev/null
+++ b/test/Analysis/ObjCRetSigs.m
@@ -0,0 +1,25 @@
+// RUN: clang -check-objc-methodsigs -verify %s
+
+#include <stdio.h>
+
+@interface MyBase
+-(long long)length;
+@end
+
+@interface MySub : MyBase{}
+-(double)length;
+@end
+
+@implementation MyBase
+-(long long)length{
+   printf("Called MyBase -length;\n");
+   return 3;
+}
+@end
+
+@implementation MySub
+-(double)length{  // expected-warning{{types are incompatible}}
+   printf("Called MySub -length;\n");
+   return 3.3;
+}
+@end