From d676ba0f28ee263bc68f6992ca237cdf9395be21 Mon Sep 17 00:00:00 2001 From: George Karpenkov Date: Thu, 29 Mar 2018 20:55:34 +0000 Subject: [PATCH] [analyzer] Path-insensitive checker for writes into an auto-releasing pointer from the wrong auto-releasing pool, as such writes may crash. rdar://25301111 Differential Revision: https://reviews.llvm.org/D44722 llvm-svn: 328827 --- .../clang/StaticAnalyzer/Checkers/Checkers.td | 3 + clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt | 1 + .../Checkers/ObjCAutoreleaseWriteChecker.cpp | 157 +++++++++++++++++++ clang/test/Analysis/autoreleasewritechecker_test.m | 171 +++++++++++++++++++++ 4 files changed, 332 insertions(+) create mode 100644 clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp create mode 100644 clang/test/Analysis/autoreleasewritechecker_test.m diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index eeec0c8..e26cc6f 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -616,6 +616,9 @@ def ObjCSuperDeallocChecker : Checker<"SuperDealloc">, HelpText<"Warn about improper use of '[super dealloc]' in Objective-C">, DescFile<"ObjCSuperDeallocChecker.cpp">; +def AutoreleaseWriteChecker : Checker<"AutoreleaseWrite">, + HelpText<"Warn about potentially crashing writes to autoreleasing objects from different autoreleasing pools in Objective-C">, + DescFile<"ObjCAutoreleaseWriteChecker.cpp">; } // end "osx.cocoa" let ParentPackage = Performance in { diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index d472ae4..c1c1b99 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -63,6 +63,7 @@ add_clang_library(clangStaticAnalyzerCheckers NullabilityChecker.cpp NumberObjectConversionChecker.cpp ObjCAtSyncChecker.cpp + ObjCAutoreleaseWriteChecker.cpp ObjCContainersASTChecker.cpp ObjCContainersChecker.cpp ObjCMissingSuperCallChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp new file mode 100644 index 0000000..0433a8a --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp @@ -0,0 +1,157 @@ +//===- 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 out parameters which are likely to cause crashes. +// An example of a problematic write is a write to {@code error} in the example +// below: +// +// - (BOOL) mymethod:(NSError *__autoreleasing *)error list:(NSArray*) list { +// [list enumerateObjectsUsingBlock:^(id obj, NSUInteger idx, BOOL *stop) { +// NSString *myString = obj; +// if ([myString isEqualToString:@"error"] && error) +// *error = [NSError errorWithDomain:@"MyDomain" code:-1]; +// }]; +// 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 "llvm/ADT/Twine.h" + +using namespace clang; +using namespace ento; +using namespace ast_matchers; + +namespace { + +const char *ProblematicWriteBind = "problematicwrite"; +const char *ParamBind = "parambind"; +const char *MethodBind = "methodbind"; + +class ObjCAutoreleaseWriteChecker : public Checker { +public: + void checkASTCodeBody(const Decl *D, + AnalysisManager &AM, + BugReporter &BR) const; +private: + std::vector 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 FunctionsWithAutoreleasingPool = { + "dispatch_async", "dispatch_group_async", "dispatch_barrier_async"}; +}; +} + +static inline std::vector toRefs(std::vector V) { + return std::vector(V.begin(), V.end()); +} + +static auto callsNames(std::vector FunctionNames) + -> decltype(callee(functionDecl())) { + return callee(functionDecl(hasAnyName(toRefs(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(ParamBind); + assert(PVD); + QualType Ty = PVD->getType(); + if (Ty->getPointeeType().getObjCLifetime() != Qualifiers::OCL_Autoreleasing) + return; + const auto *SW = Match.getNodeAs(ProblematicWriteBind); + bool IsMethod = Match.getNodeAs(MethodBind) != nullptr; + const char *Name = IsMethod ? "method" : "function"; + assert(SW); + BR.EmitBasicReport( + ADC->getDecl(), Checker, + /*Name=*/"Writing into auto-releasing variable from a different queue", + /*Category=*/"Memory", + (llvm::Twine("Writing into an auto-releasing out parameter inside ") + + "autorelease pool that may exit before " + Name + " returns; consider " + "writing first to a strong local variable declared outside of the block") + .str(), + 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)))))) + )), + hasOperatorName("=") + ).bind(ProblematicWriteBind); + + // WritesIntoM happens inside a block passed as an argument. + auto WritesInBlockM = hasAnyArgument(allOf( + hasType(hasCanonicalType(blockPointerType())), + forEachDescendant(WritesIntoM) + )); + + auto CallsAsyncM = stmt(anyOf( + callExpr(allOf( + callsNames(FunctionsWithAutoreleasingPool), WritesInBlockM)), + objcMessageExpr(allOf( + hasAnySelector(toRefs(SelectorsWithAutoreleasingPool)), + WritesInBlockM)) + )); + + auto DoublePointerParamM = + parmVarDecl(hasType(pointerType( + pointee(hasCanonicalType(objcObjectPointerType()))))) + .bind(ParamBind); + + auto HasParamAndWritesAsyncM = allOf( + hasAnyParameter(DoublePointerParamM), + forEachDescendant(CallsAsyncM)); + + auto MatcherM = decl(anyOf( + objcMethodDecl(HasParamAndWritesAsyncM).bind(MethodBind), + functionDecl(HasParamAndWritesAsyncM))); + + auto Matches = match(MatcherM, *D, AM.getASTContext()); + for (BoundNodes Match : Matches) + emitDiagnostics(Match, D, BR, AM, this); +} + +void ento::registerAutoreleaseWriteChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} diff --git a/clang/test/Analysis/autoreleasewritechecker_test.m b/clang/test/Analysis/autoreleasewritechecker_test.m new file mode 100644 index 0000000..ff839f4 --- /dev/null +++ b/clang/test/Analysis/autoreleasewritechecker_test.m @@ -0,0 +1,171 @@ +// RUN: %clang_analyze_cc1 -DARC -fobjc-arc -analyzer-checker=core,osx.cocoa.AutoreleaseWrite %s -fblocks -verify + + +typedef signed char BOOL; +@protocol NSObject - (BOOL)isEqual:(id)object; @end +@interface 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 {} ++ (id)errorWithDomain:(int)domain; +@end + +typedef int dispatch_semaphore_t; +typedef void (^block_t)(); + +@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); + +// No warnings without ARC. +#ifdef NOARC + +// expected-no-diagnostics +BOOL writeToErrorWithIterator(NSError ** error, NSArray *a) { + [a enumerateObjectsUsingBlock:^{ + *error = [NSError errorWithDomain:1]; // no-warning + }]; + return 0; +} +#endif + +#ifdef ARC +@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 out parameter inside autorelease pool that may exit before method returns}} + } + 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 out}} + } + 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 + } + dispatch_semaphore_signal(sem); + }); + + dispatch_semaphore_wait(sem, 100); + return 0; +} + +- (BOOL) writeToStrongErrorInBlock:(NSError *__strong *)error { + dispatch_semaphore_t sem = dispatch_semaphore_create(0l); + dispatch_async(queue, ^{ + if (error) { + *error = [NSError errorWithDomain:2]; // no-warning + } + dispatch_semaphore_signal(sem); + }); + + dispatch_semaphore_wait(sem, 100); + return 0; +} + +- (BOOL) writeToErrorInBlockMultipleTimes:(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 out}} + } + dispatch_semaphore_signal(sem); + }); + dispatch_async(queue, ^{ + if (error) { + *error = [NSError errorWithDomain:1]; // expected-warning{{Writing into an auto-releasing out}} + *error = [NSError errorWithDomain:1]; // expected-warning{{Writing into an auto-releasing out}} + } + dispatch_semaphore_signal(sem); + }); + *error = [NSError errorWithDomain:1]; // no-warning + + dispatch_semaphore_wait(sem, 100); + return 0; +} + +- (BOOL) writeToError:(NSError *__autoreleasing *)error { + *error = [NSError errorWithDomain:1]; // no-warning + return 0; +} +@end + +BOOL writeToErrorInBlockFromCFunc(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 out}} + } + dispatch_semaphore_signal(sem); + }); + + 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 out parameter inside autorelease pool that may exit before function returns; consider writing first to a strong local variable declared outside of the block}} + }]; + return 0; +} +#endif -- 2.7.4