[analyzer] Path-insensitive checker for writes into an auto-releasing pointer from the wrong pool

[george.karpenkov]

Summary

rdar://25301111

[dcoughlin]

Looks good with updated diagnostic text and a test for non-ARC behavior.

lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp

11

I would say "warns about writes to autoreleasing out parameters that are likely to cause crashes".

16

I would use the -enumerateObjectsUsingBlock: as the canonical example. The queue/threading aspect is incidental to the problem here.

102

The queue aspect is incidental here. I suggest:

"write to autoreleasing out parameter inside autorelease pool that may exit before function returns; consider writing first to a strong local variable declared outside of the block"

and

"write to autoreleasing out parameter inside autorelease pool that may exit before method returns; consider writing first to a strong local variable declared outside of the block"

for the method case.

test/Analysis/autoreleasewritechecker_test.m

1

Can you add a test that makes sure we don't warn on a write to an out parameter when ARC is disabled.

lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp

 //===- ObjCAutoreleaseWriteChecker.cpp ----------------------------*- 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 ObjCAutoreleaseWriteChecker which warns against writes
 // into autoreleased objects from different queues which are likely to cause
 // crashes.
 // An example of a problematic write is a write to {@code error} in the example
 // below:
 //
 // - (BOOL) doSomethingWithError:(NSError *__autoreleasing *)error {
 //     dispatch_semaphore_t sem = dispatch_semaphore_create(0l);
 //     dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0l), ^{
 //         if (error) {
 //             *error = [NSError errorWithDomain:@"MyDomain" code:-1];
 //         }
 //         dispatch_semaphore_signal(sem);
 //     });
 //
 //     dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);
 //     return false;
 // }
 //
 // Such code is very likely to crash due to the other queue autorelease pool
 // begin able to free the error object.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
-#include <vector>
-#include <string>
+#include "llvm/ADT/StringSet.h"
 
 using namespace clang;
 using namespace ento;
 using namespace ast_matchers;
 
 namespace {
 
 const char *ProblematicWriteBind = "problematicwrite";
 const char *ParamBind = "parambind";
 
 class ObjCAutoreleaseWriteChecker : public Checker<check::ASTCodeBody> {
 public:
   void checkASTCodeBody(const Decl *D,
                         AnalysisManager &AM,
                         BugReporter &BR) const;
+private:
+  std::vector<std::string> SelectorsWithAutoreleasingPool = {
+      "enumerateObjectsUsingBlock:",
+      "enumerateKeysAndObjectsUsingBlock:",
+      "enumerateKeysAndObjectsWithOptions:usingBlock:",
+      "enumerateObjectsWithOptions:usingBlock:",
+      "enumerateObjectsAtIndexes:options:usingBlock:",
+      "enumerateIndexesWithOptions:usingBlock:",
+      "enumerateIndexesUsingBlock:",
+      "enumerateIndexesInRange:options:usingBlock:",
+      "enumerateRangesUsingBlock:",
+      "enumerateRangesWithOptions:usingBlock:",
+      "enumerateRangesInRange:options:usingBlock:"
+      "objectWithOptions:passingTest:",
+  };
+
+  std::vector<std::string> FunctionsWithAutoreleasingPool = {
+      "dispatch_async", "dispatch_group_async", "dispatch_barrier_async"};
 };
 
 }
 
-static auto callsName(const char *FunctionName)
+static auto callsNames(std::vector<std::string> FunctionNames)
     -> decltype(callee(functionDecl())) {
-  return callee(functionDecl(hasName(FunctionName)));
+  return callee(functionDecl(hasAnyNameIn(FunctionNames)));
 }
 
 static void emitDiagnostics(BoundNodes &Match,
                             const Decl *D,
                             BugReporter &BR,
                             AnalysisManager &AM,
                             const ObjCAutoreleaseWriteChecker *Checker) {
   AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
 
   const auto *PVD = Match.getNodeAs<ParmVarDecl>(ParamBind);
   assert(PVD);
   QualType Ty = PVD->getType();
   if (Ty->getPointeeType().getObjCLifetime() != Qualifiers::OCL_Autoreleasing)
     return;
   const auto *SW = Match.getNodeAs<Expr>(ProblematicWriteBind);
   assert(SW);
   BR.EmitBasicReport(
     ADC->getDecl(),
     Checker,
     /*Name=*/"Writing into auto-releasing variable from a different queue",
     /*Category=*/"Memory",
     "Writing into an auto-releasing variable from a different queue "
     "to which it belongs to is very likely to race and crash",
     PathDiagnosticLocation::createBegin(SW, BR.getSourceManager(), ADC),
     SW->getSourceRange()
   );
 }
 
 void ObjCAutoreleaseWriteChecker::checkASTCodeBody(const Decl *D,
                                                   AnalysisManager &AM,
                                                   BugReporter &BR) const {
 
   // Write into a binded object, e.g. *ParamBind = X.
   auto WritesIntoM = binaryOperator(
     hasLHS(unaryOperator(
         hasOperatorName("*"),
         hasUnaryOperand(
           ignoringParenImpCasts(
-            declRefExpr(to(parmVarDecl(equalsBoundNode(ParamBind)))
-                               )))
+            declRefExpr(to(parmVarDecl(equalsBoundNode(ParamBind))))))
     )),
     hasOperatorName("=")
   ).bind(ProblematicWriteBind);
 
   // WritesIntoM happens inside a block passed as an argument.
   auto WritesInBlockM = hasAnyArgument(allOf(
       hasType(hasCanonicalType(blockPointerType())),
       forEachDescendant(WritesIntoM)
       ));
 
-  // FIXME: use a list of APIs which can dispatch in asynchronous manner.
-  auto CallsAsyncM = callExpr(allOf(callsName("dispatch_async"),
-                                    WritesInBlockM));
+  auto CallsAsyncM = stmt(anyOf(callExpr(allOf(
+      callsNames(FunctionsWithAutoreleasingPool), WritesInBlockM)),
+    objcMessageExpr(allOf(
+       hasAnySelectorIn(SelectorsWithAutoreleasingPool),  WritesInBlockM))));
 
   auto DoublePointerParamM =
       parmVarDecl(hasType(pointerType(
                       pointee(hasCanonicalType(objcObjectPointerType())))))
           .bind(ParamBind);
 
   auto HasParamAndWritesAsyncM = allOf(
       hasAnyParameter(DoublePointerParamM),

test/Analysis/autoreleasewritechecker_test.m

 // RUN: %clang_analyze_cc1 -fobjc-arc -analyzer-checker=core,osx.cocoa.AutoreleaseWrite %s -fblocks -verify
 typedef signed char BOOL;
 @protocol NSObject  - (BOOL)isEqual:(id)object; @end
 @interface NSObject <NSObject> {}
 +(id)alloc;
 -(id)init;
 -(id)autorelease;
 -(id)copy;
 -(id)retain;
 @end
 typedef int NSZone;
 typedef int NSCoder;
 @protocol NSCopying  - (id)copyWithZone:(NSZone *)zone; @end
 @protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end
 @interface NSError : NSObject <NSCopying, NSCoding> {}
 + (id)errorWithDomain:(int)domain;
 @end
 
 typedef int dispatch_semaphore_t;
 typedef void (^block_t)();
-typedef NSError *__autoreleasing* MYERROR;
 
+@interface NSArray
+- (void) enumerateObjectsUsingBlock:(block_t)block;
+@end
+
+typedef int group_t;
 typedef struct dispatch_queue_s *dispatch_queue_t;
 typedef void (^dispatch_block_t)(void);
 extern dispatch_queue_t queue;
+
+void dispatch_group_async(dispatch_queue_t queue,
+                          group_t group,
+                          dispatch_block_t block);
 void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
 dispatch_semaphore_t dispatch_semaphore_create(int);
 
 void dispatch_semaphore_wait(dispatch_semaphore_t, int);
 void dispatch_semaphore_signal(dispatch_semaphore_t);
 
 @interface I : NSObject
 - (BOOL) writeToStrongErrorInBlock:(NSError *__strong *)error;
 - (BOOL) writeToErrorInBlock:(NSError *__autoreleasing *)error;
 - (BOOL) writeToLocalErrorInBlock:(NSError **)error;
 - (BOOL) writeToErrorInBlockMultipleTimes:(NSError *__autoreleasing *)error;
 - (BOOL) writeToError:(NSError *__autoreleasing *)error;
+- (BOOL) writeToErrorWithDispatchGroup:(NSError *__autoreleasing *)error;
 @end
 
 @implementation I
 
 - (BOOL) writeToErrorInBlock:(NSError *__autoreleasing *)error {
     dispatch_semaphore_t sem = dispatch_semaphore_create(0l);
     dispatch_async(queue, ^{
         if (error) {
             *error = [NSError errorWithDomain:1]; // expected-warning{{Writing into an auto-releasing variable}}
         }
         dispatch_semaphore_signal(sem);
     });
 
     dispatch_semaphore_wait(sem, 100);
     return 0;
 }
 
+- (BOOL) writeToErrorWithDispatchGroup:(NSError *__autoreleasing *)error {
+    dispatch_semaphore_t sem = dispatch_semaphore_create(0l);
+    dispatch_group_async(queue, 0, ^{
+        if (error) {
+            *error = [NSError errorWithDomain:1]; // expected-warning{{Writing into an auto-releasing variable}}
+        }
+        dispatch_semaphore_signal(sem);
+    });
+
+    dispatch_semaphore_wait(sem, 100);
+    return 0;
+}
+
 - (BOOL) writeToLocalErrorInBlock:(NSError *__autoreleasing *)error {
     dispatch_semaphore_t sem = dispatch_semaphore_create(0l);
     dispatch_async(queue, ^{
         NSError* error2;
         NSError*__strong* error3 = &error2;
         if (error) {
             *error3 = [NSError errorWithDomain:1]; // no-warning
         }
BOOL writeToErrorInBlockFromCFunc(NSError *__autoreleasing* error) {
     dispatch_semaphore_wait(sem, 100);
   return 0;
 }
 
 BOOL writeToErrorNoWarning(NSError *__autoreleasing* error) {
   *error = [NSError errorWithDomain:1]; // no-warning
   return 0;
 }
+
+BOOL writeToErrorWithIterator(NSError *__autoreleasing* error, NSArray *a) {
+  [a enumerateObjectsUsingBlock:^{
+    *error = [NSError errorWithDomain:1]; // expected-warning{{Writing into an auto-releasing variable}}
+    }];
+  return 0;
+}