ahatanak updated this revision to Diff 142792.
ahatanak added a comment.

Deactivate the cleanups for callee-destructed parameters that are being 
forwarded to a delegated constructor.

I also added a new member (CurrentCleanupStackDepth) to CodeGenFunction that 
tracks the stack depth of the most recent RunCleanupsScope. This is needed to 
prevent popping the cleanup at the top of the stack in DeactivateCleanupBlock 
if the cleanup doesn't belong to the current RunCleanupsScope. Without this, 
the current RunCleanupsScope's CleanupStackDepth can point to a cleanup that 
doesn't exist on the stack anymore 
(CleanupStackDepth.encloses(EHStack.stable_begin()) doesn't hold true anymore), 
which can cause CodeGenFunction::PopCleanupBlocks to pop all the cleanups 
before it hits the bottom of the stack and crash.


Repository:
  rC Clang

https://reviews.llvm.org/D45382

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGCleanup.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
  test/CodeGenObjCXX/arc-special-member-functions.mm
  test/CodeGenObjCXX/lambda-expressions.mm

Index: test/CodeGenObjCXX/lambda-expressions.mm
===================================================================
--- test/CodeGenObjCXX/lambda-expressions.mm
+++ test/CodeGenObjCXX/lambda-expressions.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks -fobjc-arc | FileCheck -check-prefix=ARC %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks -fobjc-arc -fobjc-runtime-has-weak -DWEAK_SUPPORTED | FileCheck -check-prefix=ARC %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks | FileCheck -check-prefix=MRC %s
 
 typedef int (^fp)();
@@ -138,5 +138,31 @@
 }
 @end
 
+// Check that the delegating invoke function doesn't destruct the Weak object
+// that is passed.
+
+// ARC-LABEL: define internal void @"_ZZN14LambdaDelegate4testEvEN3$_58__invokeENS_4WeakE"(
+// ARC: call void @"_ZZN14LambdaDelegate4testEvENK3$_5clENS_4WeakE"(
+// ARC-NEXT: ret void
+
+// ARC-LABEL: define internal void @"_ZZN14LambdaDelegate4testEvENK3$_5clENS_4WeakE"(
+// ARC: call void @_ZN14LambdaDelegate4WeakD1Ev(
+
+#ifdef WEAK_SUPPORTED
+
+namespace LambdaDelegate {
+
+struct Weak {
+  __weak id x;
+};
+
+void test() {
+  void (*p)(Weak) = [](Weak a) { };
+}
+
+};
+
+#endif
+
 // ARC: attributes [[NUW]] = { noinline nounwind{{.*}} }
 // MRC: attributes [[NUW]] = { noinline nounwind{{.*}} }
Index: test/CodeGenObjCXX/arc-special-member-functions.mm
===================================================================
--- test/CodeGenObjCXX/arc-special-member-functions.mm
+++ test/CodeGenObjCXX/arc-special-member-functions.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fobjc-arc -fblocks -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fobjc-arc -fblocks -triple x86_64-apple-darwin10.0.0 -fobjc-runtime-has-weak -emit-llvm -o - %s | FileCheck %s
 
 struct ObjCMember {
   id member;
@@ -12,6 +12,59 @@
   int (^bp)(int);
 };
 
+// CHECK: %[[STRUCT_CONTAINSWEAK:.*]] = type { %[[STRUCT_WEAK:.*]] }
+// CHECK: %[[STRUCT_WEAK]] = type { i8* }
+
+// The Weak object that is passed is destructed in this constructor.
+
+// CHECK: define void @_ZN12ContainsWeakC2E4Weak(
+// CHECK: call void @_ZN4WeakC1ERKS_(
+// CHECK: call void @_ZN4WeakD1Ev(
+
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define void @_ZN12ContainsWeakC1E4Weak(
+// CHECK: call void @_ZN12ContainsWeakC2E4Weak(
+// CHECK-NEXT: ret void
+
+struct Weak {
+  Weak(id);
+  __weak id x;
+};
+
+struct ContainsWeak {
+  ContainsWeak(Weak);
+  Weak w;
+};
+
+ContainsWeak::ContainsWeak(Weak a) : w(a) {}
+
+// The Weak object that is passed is destructed in this constructor.
+
+// CHECK: define void @_ZN4BaseC2E4Weak(
+// CHECK: call void @_ZN4WeakD1Ev(
+// CHECK: ret void
+
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define linkonce_odr void @_ZN7DerivedCI14BaseE4Weak(
+// CHECK: call void @_ZN7DerivedCI24BaseE4Weak(
+// CHECK-NEXT: ret void
+
+struct Base {
+  Base(Weak);
+};
+
+Base::Base(Weak a) {}
+
+struct Derived : Base {
+  using Base::Base;
+};
+
+Derived d(Weak(0));
+
 // CHECK-LABEL: define void @_Z42test_ObjCMember_default_construct_destructv(
 void test_ObjCMember_default_construct_destruct() {
   // CHECK: call void @_ZN10ObjCMemberC1Ev
@@ -111,6 +164,13 @@
 // CHECK-NEXT: call void @objc_release(i8* [[T7]])
 // CHECK-NEXT: ret
 
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define linkonce_odr void @_ZN7DerivedCI24BaseE4Weak(
+// CHECK: call void @_ZN4BaseC2E4Weak(
+// CHECK-NEXT: ret void
+
 // Implicitly-generated default constructor for ObjCMember
 // CHECK-LABEL: define linkonce_odr void @_ZN10ObjCMemberC2Ev
 // CHECK-NOT: objc_release
Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===================================================================
--- test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -10,6 +10,17 @@
   // CHECK-NEXT: ret i8* [[T2]]
 }
 
+// Check that the delegating block invoke function doesn't destruct the Weak
+// object that is passed.
+
+// CHECK-LABEL: define internal void @___Z8testWeakv_block_invoke(
+// CHECK: call void @"_ZZ8testWeakvENK3$_2clE4Weak"(
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @"_ZZ8testWeakvENK3$_2clE4Weak"(
+// CHECK: call void @_ZN4WeakD1Ev(
+// CHECK-NEXT: ret void
+
 id test1_rv;
 
 void test1() {
@@ -21,3 +32,12 @@
   // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]])
   // CHECK-NEXT: ret i8* [[T2]]
 }
+
+struct Weak {
+  __weak id x;
+};
+
+void testWeak() {
+  extern void testWeak_helper(void (^)(Weak));
+  testWeak_helper([](Weak){});
+}
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -587,7 +587,7 @@
   /// \brief Enters a new scope for capturing cleanups, all of which
   /// will be executed once the scope is exited.
   class RunCleanupsScope {
-    EHScopeStack::stable_iterator CleanupStackDepth;
+    EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupStackDepth;
     size_t LifetimeExtendedCleanupStackSize;
     bool OldDidCallStackSave;
   protected:
@@ -610,6 +610,8 @@
           CGF.LifetimeExtendedCleanupStack.size();
       OldDidCallStackSave = CGF.DidCallStackSave;
       CGF.DidCallStackSave = false;
+      OldCleanupStackDepth = CGF.getCurrentCleanupStackDepth();
+      CGF.setCurrentCleanupStackDepth(CleanupStackDepth);
     }
 
     /// \brief Exit this cleanup scope, emitting any accumulated cleanups.
@@ -635,6 +637,7 @@
       CGF.PopCleanupBlocks(CleanupStackDepth, LifetimeExtendedCleanupStackSize,
                            ValuesToReload);
       PerformCleanup = false;
+      CGF.setCurrentCleanupStackDepth(OldCleanupStackDepth);
     }
   };
 
@@ -835,6 +838,14 @@
                    size_t OldLifetimeExtendedStackSize,
                    std::initializer_list<llvm::Value **> ValuesToReload = {});
 
+  EHScopeStack::stable_iterator getCurrentCleanupStackDepth() const {
+    return CurrentCleanupStackDepth;
+  }
+
+  void setCurrentCleanupStackDepth(EHScopeStack::stable_iterator C) {
+    CurrentCleanupStackDepth = C;
+  }
+
   void ResolveBranchFixups(llvm::BasicBlock *Target);
 
   /// The given basic block lies in the current EH scope, but may be a
@@ -1095,6 +1106,15 @@
   /// decls.
   DeclMapTy LocalDeclMap;
 
+  // Keep track of the cleanups for callee-destructed parameters pushed to the
+  // cleanup stack so that they can be deactivated later.
+  llvm::DenseMap<const ParmVarDecl *, EHScopeStack::stable_iterator>
+      CalleeDestructedParamCleanups;
+
+  // Cleanup stack depth of the RunCleanupsScope that was pushed most recently.
+  EHScopeStack::stable_iterator CurrentCleanupStackDepth =
+      EHScopeStack::stable_end();
+
   /// SizeArguments - If a ParmVarDecl had the pass_object_size attribute, this
   /// will contain a mapping from said ParmVarDecl to its implicit "object_size"
   /// parameter.
Index: lib/CodeGen/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1962,6 +1962,8 @@
                 DtorKind == QualType::DK_nontrivial_c_struct) &&
                "unexpected destructor type");
         pushDestroy(DtorKind, DeclPtr, Ty);
+        CalleeDestructedParamCleanups[cast<ParmVarDecl>(&D)] =
+            EHStack.stable_begin();
       }
     }
   } else {
Index: lib/CodeGen/CGCleanup.cpp
===================================================================
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -1233,8 +1233,10 @@
   EHCleanupScope &Scope = cast<EHCleanupScope>(*EHStack.find(C));
   assert(Scope.isActive() && "double deactivation");
 
-  // If it's the top of the stack, just pop it.
-  if (C == EHStack.stable_begin()) {
+  // If it's the top of the stack, just pop it, but do so only if it belongs
+  // to the current RunCleanupsScope.
+  if (C == EHStack.stable_begin() &&
+      getCurrentCleanupStackDepth().strictlyEncloses(C)) {
     // If it's a normal cleanup, we need to pretend that the
     // fallthrough is unreachable.
     CGBuilderTy::InsertPoint SavedIP = Builder.saveAndClearIP();
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -3059,6 +3059,22 @@
   } else {
     args.add(convertTempToRValue(local, type, loc), type);
   }
+
+  // Deactivate the cleanup for the callee-destructed param that was pushed.
+  if (hasAggregateEvaluationKind(type) &&
+      getContext().isParamDestroyedInCallee(type)) {
+    EHScopeStack::stable_iterator cleanup =
+        CalleeDestructedParamCleanups.lookup(cast<ParmVarDecl>(param));
+    if (cleanup.isValid()) {
+      // This unreachable is a temporary marker which will be removed later.
+      llvm::Instruction *isActive = Builder.CreateUnreachable();
+      args.addArgCleanupDeactivation(cleanup, isActive);
+    } else
+      // A param cleanup should have been pushed unless we are code-generating
+      // a thunk.
+      assert(CurFuncIsThunk &&
+             "cleanup for callee-destructed param not recorded");
+  }
 }
 
 static bool isProvablyNull(llvm::Value *addr) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to