From f55a869ba4c651943715d13d9b9c50a2e752a6ac Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Fri, 15 Jul 2011 21:11:23 +0000 Subject: [PATCH] [arcmt] For: id x = ... @try { ... } @finally { [x release]; } Migrator will drop the release. It's better to change it to "x = 0" in a @finally to avoid leak when exception is thrown. rdar://9398256 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@135301 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/ARCMigrate/TransRetainReleaseDealloc.cpp | 39 ++++++++++++++++---- test/ARCMT/releases.m | 9 ++++- test/ARCMT/releases.m.result | 9 ++++- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/lib/ARCMigrate/TransRetainReleaseDealloc.cpp b/lib/ARCMigrate/TransRetainReleaseDealloc.cpp index c72312b260..8e6c533991 100644 --- a/lib/ARCMigrate/TransRetainReleaseDealloc.cpp +++ b/lib/ARCMigrate/TransRetainReleaseDealloc.cpp @@ -97,10 +97,7 @@ public: return true; case ObjCMessageExpr::SuperInstance: { Transaction Trans(Pass.TA); - Pass.TA.clearDiagnostic(diag::err_arc_illegal_explicit_message, - diag::err_unavailable, - diag::err_unavailable_message, - E->getSuperLoc()); + clearDiagnostics(E->getSuperLoc()); if (tryRemoving(E)) return true; Pass.TA.replace(E->getSourceRange(), "self"); @@ -114,10 +111,17 @@ public: if (!rec) return true; Transaction Trans(Pass.TA); - Pass.TA.clearDiagnostic(diag::err_arc_illegal_explicit_message, - diag::err_unavailable, - diag::err_unavailable_message, - rec->getExprLoc()); + clearDiagnostics(rec->getExprLoc()); + + if (E->getMethodFamily() == OMF_release && + isRemovable(E) && isInAtFinally(E)) { + // Change the -release to "receiver = 0" in a finally to avoid a leak + // when an exception is thrown. + Pass.TA.replace(E->getSourceRange(), rec->getSourceRange()); + Pass.TA.insertAfterToken(rec->getLocEnd(), " = 0"); + return true; + } + if (!hasSideEffects(E, Pass.Ctx)) { if (tryRemoving(E)) return true; @@ -128,6 +132,25 @@ public: } private: + void clearDiagnostics(SourceLocation loc) const { + Pass.TA.clearDiagnostic(diag::err_arc_illegal_explicit_message, + diag::err_unavailable, + diag::err_unavailable_message, + loc); + } + + bool isInAtFinally(Expr *E) const { + assert(E); + Stmt *S = E; + while (S) { + if (isa(S)) + return true; + S = StmtMap->getParent(S); + } + + return false; + } + bool isRemovable(Expr *E) const { return Removables.count(E); } diff --git a/test/ARCMT/releases.m b/test/ARCMT/releases.m index f49cb7e7b9..74e3f47aa7 100644 --- a/test/ARCMT/releases.m +++ b/test/ARCMT/releases.m @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fobjc-nonfragile-abi -fblocks -fsyntax-only -fobjc-arc -x objective-c %s.result -// RUN: arcmt-test --args -triple x86_64-apple-darwin10 -fblocks -fobjc-nonfragile-abi -fsyntax-only -x objective-c %s > %t +// RUN: %clang_cc1 -fobjc-nonfragile-abi -fobjc-exceptions -fblocks -fsyntax-only -fobjc-arc -x objective-c %s.result +// RUN: arcmt-test --args -triple x86_64-apple-darwin10 -fobjc-exceptions -fblocks -fobjc-nonfragile-abi -fsyntax-only -x objective-c %s > %t // RUN: diff %t %s.result typedef int BOOL; @@ -36,6 +36,11 @@ id IhaveSideEffect(); [IhaveSideEffect() release]; [x release], x = 0; + + @try { + } @finally { + [x release]; + } } @end diff --git a/test/ARCMT/releases.m.result b/test/ARCMT/releases.m.result index 886ce6095a..f074fb7cf7 100644 --- a/test/ARCMT/releases.m.result +++ b/test/ARCMT/releases.m.result @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fobjc-nonfragile-abi -fblocks -fsyntax-only -fobjc-arc -x objective-c %s.result -// RUN: arcmt-test --args -triple x86_64-apple-darwin10 -fblocks -fobjc-nonfragile-abi -fsyntax-only -x objective-c %s > %t +// RUN: %clang_cc1 -fobjc-nonfragile-abi -fobjc-exceptions -fblocks -fsyntax-only -fobjc-arc -x objective-c %s.result +// RUN: arcmt-test --args -triple x86_64-apple-darwin10 -fobjc-exceptions -fblocks -fobjc-nonfragile-abi -fsyntax-only -x objective-c %s > %t // RUN: diff %t %s.result typedef int BOOL; @@ -34,6 +34,11 @@ id IhaveSideEffect(); IhaveSideEffect(); x = 0; + + @try { + } @finally { + x = 0; + } } @end -- 2.40.0