[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

diff --git a/lib/ARCMigrate/ARCMT.cpp b/lib/ARCMigrate/ARCMT.cpp
index f1d947da677e17ab66b653b3c964b4eeb786b20c..1f21dda2a098c9701c0c229d323e3baf1accf3a2 100644 (file)
--- 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 5f2711e36f26a78232650fae1453387bc6c00e51..4433161ed2b64e7a35a8d9ec7cf83410cefd86d8 100644 (file)
--- 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 4f9b138a06ce59c8653a01cb76106cc5df906c77..5fdfca9ef8ec6d974f15e788caa7e91ed5871a40 100644 (file)
--- 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 (file)
index 0000000..58fd3d0
--- /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<APIChecker> {
+  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 c99940b494b97f69cfb1c20d2a8ea9e322fa732c..5f3605d73405867e573e6b07981b91025e83a76a 100644 (file)
--- 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 7bd95e54bc27081f0d681f205c9e8f3e7703ab18..22c34befe2fdc967719f46199784478d7598591e 100644 (file)
--- 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<TransformFn> arcmt::getAllTransformations() {

diff --git a/lib/ARCMigrate/Transforms.h b/lib/ARCMigrate/Transforms.h
index b47d6d8e9b8b64e6a9f39d66d131ef40792589b3..150893e6434bbb9fd7b19f4f08d8019286dec70e 100644 (file)
--- 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 (file)
index 0000000..47adb79
--- /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