From fd10398c10ffdcbdeb1e3e299c74d70e689f503c Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Mon, 18 Jul 2011 07:44:45 +0000 Subject: [PATCH] [arcmt] NSInvocation's [get/set]ReturnValue and [get/set]Argument are only safe with __unsafe_unretained parameters. Emit error for strong/weak ones. rdar://9206226 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@135381 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/ARCMigrate/ARCMT.cpp | 2 +- lib/ARCMigrate/CMakeLists.txt | 1 + lib/ARCMigrate/Internals.h | 3 + lib/ARCMigrate/TransAPIUses.cpp | 89 +++++++++++++++++++++++++++++ lib/ARCMigrate/TransformActions.cpp | 3 +- lib/ARCMigrate/Transforms.cpp | 1 + lib/ARCMigrate/Transforms.h | 4 +- test/ARCMT/check-api.m | 43 ++++++++++++++ 8 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 lib/ARCMigrate/TransAPIUses.cpp create mode 100644 test/ARCMT/check-api.m diff --git a/lib/ARCMigrate/ARCMT.cpp b/lib/ARCMigrate/ARCMT.cpp index f1d947da67..1f21dda2a0 100644 --- a/lib/ARCMigrate/ARCMT.cpp +++ b/lib/ARCMigrate/ARCMT.cpp @@ -266,7 +266,7 @@ bool arcmt::checkForManualIssues(CompilerInvocation &origCI, // to remove it so that we don't get errors from normal compilation. origCI.getLangOpts().ObjCAutoRefCount = false; - return capturedDiags.hasErrors(); + return capturedDiags.hasErrors() || testAct.hasReportedErrors(); } //===----------------------------------------------------------------------===// diff --git a/lib/ARCMigrate/CMakeLists.txt b/lib/ARCMigrate/CMakeLists.txt index 5f2711e36f..4433161ed2 100644 --- a/lib/ARCMigrate/CMakeLists.txt +++ b/lib/ARCMigrate/CMakeLists.txt @@ -4,6 +4,7 @@ add_clang_library(clangARCMigrate ARCMT.cpp ARCMTActions.cpp FileRemapper.cpp + TransAPIUses.cpp TransARCAssign.cpp TransAutoreleasePool.cpp TransBlockObjCVariable.cpp diff --git a/lib/ARCMigrate/Internals.h b/lib/ARCMigrate/Internals.h index 4f9b138a06..5fdfca9ef8 100644 --- a/lib/ARCMigrate/Internals.h +++ b/lib/ARCMigrate/Internals.h @@ -37,6 +37,7 @@ public: class TransformActions { Diagnostic &Diags; CapturedDiagList &CapturedDiags; + bool ReportedErrors; void *Impl; // TransformActionsImpl. public: @@ -88,6 +89,8 @@ public: void reportNote(llvm::StringRef note, SourceLocation loc, SourceRange range = SourceRange()); + bool hasReportedErrors() const { return ReportedErrors; } + class RewriteReceiver { public: virtual ~RewriteReceiver(); diff --git a/lib/ARCMigrate/TransAPIUses.cpp b/lib/ARCMigrate/TransAPIUses.cpp new file mode 100644 index 0000000000..58fd3d07d0 --- /dev/null +++ b/lib/ARCMigrate/TransAPIUses.cpp @@ -0,0 +1,89 @@ +//===--- TransAPIUses.cpp - Tranformations to ARC mode --------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// checkAPIUses: +// +// Emits error with some API uses that are not safe in ARC mode: +// +// - NSInvocation's [get/set]ReturnValue and [get/set]Argument are only safe +// with __unsafe_unretained objects. +// +//===----------------------------------------------------------------------===// + +#include "Transforms.h" +#include "Internals.h" + +using namespace clang; +using namespace arcmt; +using namespace trans; +using llvm::StringRef; + +namespace { + +class APIChecker : public RecursiveASTVisitor { + MigrationPass &Pass; + Selector getReturnValueSel, setReturnValueSel; + Selector getArgumentSel, setArgumentSel; + +public: + APIChecker(MigrationPass &pass) : Pass(pass) { + SelectorTable &sels = Pass.Ctx.Selectors; + IdentifierTable &ids = Pass.Ctx.Idents; + getReturnValueSel = sels.getUnarySelector(&ids.get("getReturnValue")); + setReturnValueSel = sels.getUnarySelector(&ids.get("setReturnValue")); + + IdentifierInfo *selIds[2]; + selIds[0] = &ids.get("getArgument"); + selIds[1] = &ids.get("atIndex"); + getArgumentSel = sels.getSelector(2, selIds); + selIds[0] = &ids.get("setArgument"); + setArgumentSel = sels.getSelector(2, selIds); + } + + bool VisitObjCMessageExpr(ObjCMessageExpr *E) { + if (E->isInstanceMessage() && + E->getReceiverInterface() && + E->getReceiverInterface()->getName() == "NSInvocation") { + StringRef selName; + if (E->getSelector() == getReturnValueSel) + selName = "getReturnValue"; + else if (E->getSelector() == setReturnValueSel) + selName = "setReturnValue"; + else if (E->getSelector() == getArgumentSel) + selName = "getArgument"; + else if (E->getSelector() == setArgumentSel) + selName = "setArgument"; + + if (selName.empty()) + return true; + + Expr *parm = E->getArg(0)->IgnoreParenCasts(); + QualType pointee = parm->getType()->getPointeeType(); + if (pointee.isNull()) + return true; + + if (pointee.getObjCLifetime() > Qualifiers::OCL_ExplicitNone) { + std::string err = "NSInvocation's "; + err += selName; + err += " is not safe to be used with an object with ownership other " + "than __unsafe_unretained"; + Pass.TA.reportError(err, parm->getLocStart(), parm->getSourceRange()); + } + return true; + } + + return true; + } +}; + +} // anonymous namespace + +void trans::checkAPIUses(MigrationPass &pass) { + APIChecker(pass).TraverseDecl(pass.Ctx.getTranslationUnitDecl()); +} diff --git a/lib/ARCMigrate/TransformActions.cpp b/lib/ARCMigrate/TransformActions.cpp index c99940b494..5f3605d734 100644 --- a/lib/ARCMigrate/TransformActions.cpp +++ b/lib/ARCMigrate/TransformActions.cpp @@ -600,7 +600,7 @@ TransformActions::RewriteReceiver::~RewriteReceiver() { } TransformActions::TransformActions(Diagnostic &diag, CapturedDiagList &capturedDiags, ASTContext &ctx, Preprocessor &PP) - : Diags(diag), CapturedDiags(capturedDiags) { + : Diags(diag), CapturedDiags(capturedDiags), ReportedErrors(false) { Impl = new TransformActionsImpl(capturedDiags, ctx, PP); } @@ -683,6 +683,7 @@ void TransformActions::reportError(llvm::StringRef error, SourceLocation loc, = Diags.getDiagnosticIDs()->getCustomDiagID(DiagnosticIDs::Error, rewriteErr); Diags.Report(loc, diagID) << range; + ReportedErrors = true; } void TransformActions::reportNote(llvm::StringRef note, SourceLocation loc, diff --git a/lib/ARCMigrate/Transforms.cpp b/lib/ARCMigrate/Transforms.cpp index 7bd95e54bc..22c34befe2 100644 --- a/lib/ARCMigrate/Transforms.cpp +++ b/lib/ARCMigrate/Transforms.cpp @@ -283,6 +283,7 @@ static void independentTransforms(MigrationPass &pass) { makeAssignARCSafe(pass); rewriteUnbridgedCasts(pass); rewriteBlockObjCVariable(pass); + checkAPIUses(pass); } std::vector arcmt::getAllTransformations() { diff --git a/lib/ARCMigrate/Transforms.h b/lib/ARCMigrate/Transforms.h index b47d6d8e9b..150893e643 100644 --- a/lib/ARCMigrate/Transforms.h +++ b/lib/ARCMigrate/Transforms.h @@ -37,6 +37,7 @@ void removeZeroOutPropsInDealloc(MigrationPass &pass); void rewriteProperties(MigrationPass &pass); void rewriteBlockObjCVariable(MigrationPass &pass); void rewriteUnusedInitDelegate(MigrationPass &pass); +void checkAPIUses(MigrationPass &pass); void removeEmptyStatementsAndDealloc(MigrationPass &pass); @@ -65,7 +66,8 @@ public: BodyTransform(MigrationPass &pass) : Pass(pass) { } bool TraverseStmt(Stmt *rootS) { - BODY_TRANS(Pass).transformBody(rootS); + if (rootS) + BODY_TRANS(Pass).transformBody(rootS); return true; } }; diff --git a/test/ARCMT/check-api.m b/test/ARCMT/check-api.m new file mode 100644 index 0000000000..47adb79711 --- /dev/null +++ b/test/ARCMT/check-api.m @@ -0,0 +1,43 @@ +// RUN: %clang_cc1 -arcmt-check -verify -triple x86_64-apple-macosx10.7 -fobjc-nonfragile-abi %s + +#include "Common.h" + +@interface NSInvocation : NSObject +- (void)getReturnValue:(void *)retLoc; +- (void)setReturnValue:(void *)retLoc; + +- (void)getArgument:(void *)argumentLocation atIndex:(int)idx; +- (void)setArgument:(void *)argumentLocation atIndex:(int)idx; +@end + +@interface Test +@end + +@implementation Test { + id strong_id; + __weak id weak_id; + __unsafe_unretained id unsafe_id; + int arg; +} +- (void) test:(NSInvocation *)invok { + [invok getReturnValue:&strong_id]; // expected-error {{NSInvocation's getReturnValue is not safe to be used with an object with ownership other than __unsafe_unretained}} + [invok getReturnValue:&weak_id]; // expected-error {{NSInvocation's getReturnValue is not safe to be used with an object with ownership other than __unsafe_unretained}} + [invok getReturnValue:&unsafe_id]; + [invok getReturnValue:&arg]; + + [invok setReturnValue:&strong_id]; // expected-error {{NSInvocation's setReturnValue is not safe to be used with an object with ownership other than __unsafe_unretained}} + [invok setReturnValue:&weak_id]; // expected-error {{NSInvocation's setReturnValue is not safe to be used with an object with ownership other than __unsafe_unretained}} + [invok setReturnValue:&unsafe_id]; + [invok setReturnValue:&arg]; + + [invok getArgument:&strong_id atIndex:0]; // expected-error {{NSInvocation's getArgument is not safe to be used with an object with ownership other than __unsafe_unretained}} + [invok getArgument:&weak_id atIndex:0]; // expected-error {{NSInvocation's getArgument is not safe to be used with an object with ownership other than __unsafe_unretained}} + [invok getArgument:&unsafe_id atIndex:0]; + [invok getArgument:&arg atIndex:0]; + + [invok setArgument:&strong_id atIndex:0]; // expected-error {{NSInvocation's setArgument is not safe to be used with an object with ownership other than __unsafe_unretained}} + [invok setArgument:&weak_id atIndex:0]; // expected-error {{NSInvocation's setArgument is not safe to be used with an object with ownership other than __unsafe_unretained}} + [invok setArgument:&unsafe_id atIndex:0]; + [invok setArgument:&arg atIndex:0]; +} +@end -- 2.50.1