1 files changed, 88 insertions, 14 deletions

diff --git a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
index dc90b67..6d3dd1e 100644
--- a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
+++ b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
@@ -7,18 +7,27 @@
 //
 //===----------------------------------------------------------------------===//
 //
-//  Check that Objective C properties follow the following rules:
-//    - The property should be set with the setter, not though a direct
-//      assignment.
+//  Check that Objective C properties are set with the setter, not though a
+//      direct assignment.
+//
+//  Two versions of a checker exist: one that checks all methods and the other
+//      that only checks the methods annotated with
+//      __attribute__((annotate("objc_no_direct_instance_variable_assignment")))
+//
+//  The checker does not warn about assignments to Ivars, annotated with
+//       __attribute__((objc_allow_direct_instance_variable_assignment"))). This
+//      annotation serves as a false positive suppression mechanism for the
+//      checker. The annotation is allowed on properties and Ivars.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
-#include "clang/StaticAnalyzer/Core/Checker.h"
-#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "llvm/ADT/DenseMap.h"
 
 using namespace clang;
@@ -26,6 +35,27 @@ using namespace ento;
 
 namespace {
 
+/// The default method filter, which is used to filter out the methods on which
+/// the check should not be performed.
+///
+/// Checks for the init, dealloc, and any other functions that might be allowed
+/// to perform direct instance variable assignment based on their name.
+struct MethodFilter {
+  virtual ~MethodFilter() {}
+  virtual bool operator()(ObjCMethodDecl *M) {
+    if (M->getMethodFamily() == OMF_init ||
+        M->getMethodFamily() == OMF_dealloc ||
+        M->getMethodFamily() == OMF_copy ||
+        M->getMethodFamily() == OMF_mutableCopy ||
+        M->getSelector().getNameForSlot(0).find("init") != StringRef::npos ||
+        M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos)
+      return true;
+    return false;
+  }
+};
+
+static MethodFilter DefaultMethodFilter;
+
 class DirectIvarAssignment :
   public Checker<check::ASTDecl<ObjCImplementationDecl> > {
 
@@ -59,6 +89,10 @@ class DirectIvarAssignment :
   };
 
 public:
+  MethodFilter *ShouldSkipMethod;
+
+  DirectIvarAssignment() : ShouldSkipMethod(&DefaultMethodFilter) {}
+
   void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr,
                     BugReporter &BR) const;
 };
@@ -118,14 +152,7 @@ void DirectIvarAssignment::checkASTDecl(const ObjCImplementationDecl *D,
     ObjCMethodDecl *M = *I;
     AnalysisDeclContext *DCtx = Mgr.getAnalysisDeclContext(M);
 
-    // Skip the init, dealloc functions and any functions that might be doing
-    // initialization based on their name.
-    if (M->getMethodFamily() == OMF_init ||
-        M->getMethodFamily() == OMF_dealloc ||
-        M->getMethodFamily() == OMF_copy ||
-        M->getMethodFamily() == OMF_mutableCopy ||
-        M->getSelector().getNameForSlot(0).find("init") != StringRef::npos ||
-        M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos)
+    if ((*ShouldSkipMethod)(M))
       continue;
 
     const Stmt *Body = M->getBody();
@@ -136,6 +163,18 @@ void DirectIvarAssignment::checkASTDecl(const ObjCImplementationDecl *D,
   }
 }
 
+static bool isAnnotatedToAllowDirectAssignment(const Decl *D) {
+  for (specific_attr_iterator<AnnotateAttr>
+       AI = D->specific_attr_begin<AnnotateAttr>(),
+       AE = D->specific_attr_end<AnnotateAttr>(); AI != AE; ++AI) {
+    const AnnotateAttr *Ann = *AI;
+    if (Ann->getAnnotation() ==
+        "objc_allow_direct_instance_variable_assignment")
+      return true;
+  }
+  return false;
+}
+
 void DirectIvarAssignment::MethodCrawler::VisitBinaryOperator(
                                                     const BinaryOperator *BO) {
   if (!BO->isAssignmentOp())
@@ -149,8 +188,16 @@ void DirectIvarAssignment::MethodCrawler::VisitBinaryOperator(
 
   if (const ObjCIvarDecl *D = IvarRef->getDecl()) {
     IvarToPropertyMapTy::const_iterator I = IvarToPropMap.find(D);
+
     if (I != IvarToPropMap.end()) {
       const ObjCPropertyDecl *PD = I->second;
+      // Skip warnings on Ivars, annotated with
+      // objc_allow_direct_instance_variable_assignment. This annotation serves
+      // as a false positive suppression mechanism for the checker. The
+      // annotation is allowed on properties and ivars.
+      if (isAnnotatedToAllowDirectAssignment(PD) ||
+          isAnnotatedToAllowDirectAssignment(D))
+        return;
 
       ObjCMethodDecl *GetterMethod =
           InterfD->getInstanceMethod(PD->getGetterName());
@@ -175,6 +222,33 @@ void DirectIvarAssignment::MethodCrawler::VisitBinaryOperator(
 }
 }
 
+// Register the checker that checks for direct accesses in all functions,
+// except for the initialization and copy routines.
 void ento::registerDirectIvarAssignment(CheckerManager &mgr) {
   mgr.registerChecker<DirectIvarAssignment>();
 }
+
+// Register the checker that checks for direct accesses in functions annotated
+// with __attribute__((annotate("objc_no_direct_instance_variable_assignment"))).
+namespace {
+struct InvalidatorMethodFilter : MethodFilter {
+  virtual ~InvalidatorMethodFilter() {}
+  virtual bool operator()(ObjCMethodDecl *M) {
+    for (specific_attr_iterator<AnnotateAttr>
+         AI = M->specific_attr_begin<AnnotateAttr>(),
+         AE = M->specific_attr_end<AnnotateAttr>(); AI != AE; ++AI) {
+      const AnnotateAttr *Ann = *AI;
+      if (Ann->getAnnotation() == "objc_no_direct_instance_variable_assignment")
+        return false;
+    }
+    return true;
+  }
+};
+
+InvalidatorMethodFilter AttrFilter;
+}
+
+void ento::registerDirectIvarAssignmentForAnnotatedFunctions(
+    CheckerManager &mgr) {
+  mgr.registerChecker<DirectIvarAssignment>()->ShouldSkipMethod = &AttrFilter;
+}