r332300 - [analyzer] Extend the ObjCAutoreleaseWriteChecker to warn on captures as well
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Mon May 14 14:39:54 PDT 2018
Author: george.karpenkov
Date: Mon May 14 14:39:54 2018
New Revision: 332300
URL: http://llvm.org/viewvc/llvm-project?rev=332300&view=rev
Log:
[analyzer] Extend the ObjCAutoreleaseWriteChecker to warn on captures as well
A common pattern is that the code in the block does not write into the
variable explicitly, but instead passes it to a helper function which
performs the write.
Differential Revision: https://reviews.llvm.org/D46772
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
cfe/trunk/test/Analysis/autoreleasewritechecker_test.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp?rev=332300&r1=332299&r2=332300&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp Mon May 14 14:39:54 2018
@@ -8,7 +8,7 @@
//===----------------------------------------------------------------------===//
//
// This file defines ObjCAutoreleaseWriteChecker which warns against writes
-// into autoreleased out parameters which are likely to cause crashes.
+// into autoreleased out parameters which cause crashes.
// An example of a problematic write is a write to {@code error} in the example
// below:
//
@@ -21,8 +21,9 @@
// return false;
// }
//
-// Such code is very likely to crash due to the other queue autorelease pool
-// begin able to free the error object.
+// Such code will crash on read from `*error` due to the autorelease pool
+// in `enumerateObjectsUsingBlock` implementation freeing the error object
+// on exit from the function.
//
//===----------------------------------------------------------------------===//
@@ -41,8 +42,9 @@ using namespace ast_matchers;
namespace {
const char *ProblematicWriteBind = "problematicwrite";
+const char *CapturedBind = "capturedbind";
const char *ParamBind = "parambind";
-const char *MethodBind = "methodbind";
+const char *IsMethodBind = "ismethodbind";
class ObjCAutoreleaseWriteChecker : public Checker<check::ASTCodeBody> {
public:
@@ -110,67 +112,87 @@ static void emitDiagnostics(BoundNodes &
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);
- bool IsMethod = Match.getNodeAs<ObjCMethodDecl>(MethodBind) != nullptr;
+ const char *WarningMsg = "Write to";
+ const auto *MarkedStmt = Match.getNodeAs<Expr>(ProblematicWriteBind);
+
+ // Prefer to warn on write, but if not available, warn on capture.
+ if (!MarkedStmt) {
+ MarkedStmt = Match.getNodeAs<Expr>(CapturedBind);
+ assert(MarkedStmt);
+ WarningMsg = "Capture of";
+ }
+
+ SourceRange Range = MarkedStmt->getSourceRange();
+ PathDiagnosticLocation Location = PathDiagnosticLocation::createBegin(
+ MarkedStmt, BR.getSourceManager(), ADC);
+ bool IsMethod = Match.getNodeAs<ObjCMethodDecl>(IsMethodBind) != nullptr;
const char *Name = IsMethod ? "method" : "function";
- assert(SW);
BR.EmitBasicReport(
ADC->getDecl(), Checker,
- /*Name=*/"Write to autoreleasing out parameter inside autorelease pool",
+ /*Name=*/(llvm::Twine(WarningMsg)
+ + " autoreleasing out parameter inside autorelease pool").str(),
/*Category=*/"Memory",
- (llvm::Twine("Write to autoreleasing out parameter inside ") +
+ (llvm::Twine(WarningMsg) + " autoreleasing 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());
+ Location,
+ Range);
}
void ObjCAutoreleaseWriteChecker::checkASTCodeBody(const Decl *D,
AnalysisManager &AM,
BugReporter &BR) const {
+ auto DoublePointerParamM =
+ parmVarDecl(hasType(hasCanonicalType(pointerType(
+ pointee(hasCanonicalType(objcObjectPointerType()))))))
+ .bind(ParamBind);
+
+ auto ReferencedParamM =
+ declRefExpr(to(parmVarDecl(DoublePointerParamM)));
+
// Write into a binded object, e.g. *ParamBind = X.
auto WritesIntoM = binaryOperator(
hasLHS(unaryOperator(
hasOperatorName("*"),
hasUnaryOperand(
- ignoringParenImpCasts(
- declRefExpr(to(parmVarDecl(equalsBoundNode(ParamBind))))))
+ ignoringParenImpCasts(ReferencedParamM))
)),
hasOperatorName("=")
).bind(ProblematicWriteBind);
+ auto ArgumentCaptureM = hasAnyArgument(
+ ignoringParenImpCasts(ReferencedParamM));
+ auto CapturedInParamM = stmt(anyOf(
+ callExpr(ArgumentCaptureM),
+ objcMessageExpr(ArgumentCaptureM))).bind(CapturedBind);
+
// WritesIntoM happens inside a block passed as an argument.
- auto WritesInBlockM = hasAnyArgument(allOf(
+ auto WritesOrCapturesInBlockM = hasAnyArgument(allOf(
hasType(hasCanonicalType(blockPointerType())),
- forEachDescendant(WritesIntoM)
- ));
+ forEachDescendant(
+ stmt(anyOf(WritesIntoM, CapturedInParamM))
+ )));
- auto CallsAsyncM = stmt(anyOf(
+ auto BlockPassedToMarkedFuncM = stmt(anyOf(
callExpr(allOf(
- callsNames(FunctionsWithAutoreleasingPool), WritesInBlockM)),
+ callsNames(FunctionsWithAutoreleasingPool), WritesOrCapturesInBlockM)),
objcMessageExpr(allOf(
hasAnySelector(toRefs(SelectorsWithAutoreleasingPool)),
- WritesInBlockM))
+ WritesOrCapturesInBlockM))
));
- auto DoublePointerParamM =
- parmVarDecl(hasType(hasCanonicalType(pointerType(
- pointee(hasCanonicalType(objcObjectPointerType()))))))
- .bind(ParamBind);
-
- auto HasParamAndWritesAsyncM = allOf(
+ auto HasParamAndWritesInMarkedFuncM = allOf(
hasAnyParameter(DoublePointerParamM),
- forEachDescendant(CallsAsyncM));
+ forEachDescendant(BlockPassedToMarkedFuncM));
auto MatcherM = decl(anyOf(
- objcMethodDecl(HasParamAndWritesAsyncM).bind(MethodBind),
- functionDecl(HasParamAndWritesAsyncM)));
+ objcMethodDecl(HasParamAndWritesInMarkedFuncM).bind(IsMethodBind),
+ functionDecl(HasParamAndWritesInMarkedFuncM)));
auto Matches = match(MatcherM, *D, AM.getASTContext());
for (BoundNodes Match : Matches)
Modified: cfe/trunk/test/Analysis/autoreleasewritechecker_test.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/autoreleasewritechecker_test.m?rev=332300&r1=332299&r2=332300&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/autoreleasewritechecker_test.m (original)
+++ cfe/trunk/test/Analysis/autoreleasewritechecker_test.m Mon May 14 14:39:54 2018
@@ -205,10 +205,65 @@ BOOL writeToErrorWithIterator(NSError *_
return 0;
}
+void writeIntoError(NSError **error) {
+ *error = [NSError errorWithDomain:1];
+}
+
+extern void readError(NSError *error);
+
void writeToErrorWithIteratorNonnull(NSError *__autoreleasing* _Nonnull error, NSDictionary *a) {
[a enumerateKeysAndObjectsUsingBlock:^{
*error = [NSError errorWithDomain:1]; // expected-warning{{Write to autoreleasing out parameter}}
}];
}
+
+
+void escapeErrorFromIterator(NSError *__autoreleasing* _Nonnull error, NSDictionary *a) {
+ [a enumerateKeysAndObjectsUsingBlock:^{
+ writeIntoError(error); // expected-warning{{Capture of autoreleasing out parameter}}
+ }];
+}
+
+void noWarningOnRead(NSError *__autoreleasing* error, NSDictionary *a) {
+ [a enumerateKeysAndObjectsUsingBlock:^{
+ NSError* local = *error; // no-warning
+ }];
+}
+
+void noWarningOnEscapeRead(NSError *__autoreleasing* error, NSDictionary *a) {
+ [a enumerateKeysAndObjectsUsingBlock:^{
+ readError(*error); // no-warning
+ }];
+}
+
+ at interface ErrorCapture
+- (void) captureErrorOut:(NSError**) error;
+- (void) captureError:(NSError*) error;
+ at end
+
+void escapeErrorFromIteratorMethod(NSError *__autoreleasing* _Nonnull error,
+ NSDictionary *a,
+ ErrorCapture *capturer) {
+ [a enumerateKeysAndObjectsUsingBlock:^{
+ [capturer captureErrorOut:error]; // expected-warning{{Capture of autoreleasing out parameter}}
+ }];
+}
+
+void noWarningOnEscapeReadMethod(NSError *__autoreleasing* error,
+ NSDictionary *a,
+ ErrorCapture *capturer) {
+ [a enumerateKeysAndObjectsUsingBlock:^{
+ [capturer captureError:*error]; // no-warning
+ }];
+}
+
+void multipleErrors(NSError *__autoreleasing* error, NSDictionary *a) {
+ [a enumerateKeysAndObjectsUsingBlock:^{
+ writeIntoError(error); // expected-warning{{Capture of autoreleasing out parameter}}
+ *error = [NSError errorWithDomain:1]; // expected-warning{{Write to autoreleasing out parameter}}
+ writeIntoError(error); // expected-warning{{Capture of autoreleasing out parameter}}
+ }];
+}
+
#endif
More information about the cfe-commits
mailing list