From 04951d4fc3e496372abb13dcb2b95a1459574ced Mon Sep 17 00:00:00 2001 From: George Karpenkov Date: Wed, 16 May 2018 22:47:05 +0000 Subject: [PATCH] [analyzer] Extend ObjCAutoreleaseWriteChecker to catch block declarations with autoreleasing variables Differential Revision: https://reviews.llvm.org/D46984 llvm-svn: 332546 --- .../Checkers/ObjCAutoreleaseWriteChecker.cpp | 19 ++++++++++++------- .../Analysis/autoreleasewritechecker_test.m | 12 ++++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp index 5199959dee91..81bcda51b8f8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp @@ -115,14 +115,16 @@ static void emitDiagnostics(BoundNodes &Match, const Decl *D, BugReporter &BR, QualType Ty = PVD->getType(); if (Ty->getPointeeType().getObjCLifetime() != Qualifiers::OCL_Autoreleasing) return; - const char *WarningMsg = "Write to"; + const char *ActionMsg = "Write to"; const auto *MarkedStmt = Match.getNodeAs(ProblematicWriteBind); + bool IsCapture = false; // Prefer to warn on write, but if not available, warn on capture. if (!MarkedStmt) { MarkedStmt = Match.getNodeAs(CapturedBind); assert(MarkedStmt); - WarningMsg = "Capture of"; + ActionMsg = "Capture of"; + IsCapture = true; } SourceRange Range = MarkedStmt->getSourceRange(); @@ -130,12 +132,14 @@ static void emitDiagnostics(BoundNodes &Match, const Decl *D, BugReporter &BR, MarkedStmt, BR.getSourceManager(), ADC); bool IsMethod = Match.getNodeAs(IsMethodBind) != nullptr; const char *Name = IsMethod ? "method" : "function"; + BR.EmitBasicReport( ADC->getDecl(), Checker, - /*Name=*/(llvm::Twine(WarningMsg) + /*Name=*/(llvm::Twine(ActionMsg) + " autoreleasing out parameter inside autorelease pool").str(), /*Category=*/"Memory", - (llvm::Twine(WarningMsg) + " autoreleasing out parameter inside " + + (llvm::Twine(ActionMsg) + " autoreleasing out parameter " + + (IsCapture ? "'" + PVD->getName() + "'" + " " : "") + "inside " + "autorelease pool that may exit before " + Name + " returns; consider " "writing first to a strong local variable declared outside of the block") .str(), @@ -153,7 +157,7 @@ void ObjCAutoreleaseWriteChecker::checkASTCodeBody(const Decl *D, .bind(ParamBind); auto ReferencedParamM = - declRefExpr(to(parmVarDecl(DoublePointerParamM))); + declRefExpr(to(parmVarDecl(DoublePointerParamM))).bind(CapturedBind); // Write into a binded object, e.g. *ParamBind = X. auto WritesIntoM = binaryOperator( @@ -169,7 +173,7 @@ void ObjCAutoreleaseWriteChecker::checkASTCodeBody(const Decl *D, ignoringParenImpCasts(ReferencedParamM)); auto CapturedInParamM = stmt(anyOf( callExpr(ArgumentCaptureM), - objcMessageExpr(ArgumentCaptureM))).bind(CapturedBind); + objcMessageExpr(ArgumentCaptureM))); // WritesIntoM happens inside a block passed as an argument. auto WritesOrCapturesInBlockM = hasAnyArgument(allOf( @@ -192,7 +196,8 @@ void ObjCAutoreleaseWriteChecker::checkASTCodeBody(const Decl *D, auto MatcherM = decl(anyOf( objcMethodDecl(HasParamAndWritesInMarkedFuncM).bind(IsMethodBind), - functionDecl(HasParamAndWritesInMarkedFuncM))); + functionDecl(HasParamAndWritesInMarkedFuncM), + blockDecl(HasParamAndWritesInMarkedFuncM))); auto Matches = match(MatcherM, *D, AM.getASTContext()); for (BoundNodes Match : Matches) diff --git a/clang/test/Analysis/autoreleasewritechecker_test.m b/clang/test/Analysis/autoreleasewritechecker_test.m index eb7905200390..b3d34b9b11fa 100644 --- a/clang/test/Analysis/autoreleasewritechecker_test.m +++ b/clang/test/Analysis/autoreleasewritechecker_test.m @@ -265,5 +265,17 @@ void multipleErrors(NSError *__autoreleasing* error, NSDictionary *a) { }]; } +typedef void (^errBlock)(NSError *__autoreleasing *error); + +extern void expectError(errBlock); + +void captureAutoreleasingVarFromBlock(NSDictionary *dict) { + expectError(^(NSError *__autoreleasing *err) { + [dict enumerateKeysAndObjectsUsingBlock:^{ + writeIntoError(err); // expected-warning{{Capture of autoreleasing out parameter 'err'}} + }]; + }); +} + #endif -- 2.34.1